You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/11/07 14:35:56 UTC

[GitHub] [kafka] dajac opened a new pull request, #12827: KAFKA-14363; Add new `coordinator` module (KIP-848)

dajac opened a new pull request, #12827:
URL: https://github.com/apache/kafka/pull/12827

   Introduce new `coordinator` module that will host the future new group coordinator as part of KIP-848.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] ijuma commented on a diff in pull request #12827: KAFKA-14363; Add new `group-coordinator` module (KIP-848)

Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #12827:
URL: https://github.com/apache/kafka/pull/12827#discussion_r1016750711


##########
build.gradle:
##########
@@ -1222,6 +1225,41 @@ project(':metadata') {
   }
 }
 
+project(':group-coordinator') {
+  archivesBaseName = "kafka-group-coordinator"
+
+  dependencies {
+    implementation project(':server-common')
+    implementation project(':clients')
+    compileOnly libs.log4j

Review Comment:
   Do we need this?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] dajac commented on a diff in pull request #12827: KAFKA-14363; Add new `group-coordinator` module (KIP-848)

Posted by GitBox <gi...@apache.org>.
dajac commented on code in PR #12827:
URL: https://github.com/apache/kafka/pull/12827#discussion_r1016794520


##########
build.gradle:
##########
@@ -1222,6 +1225,41 @@ project(':metadata') {
   }
 }
 
+project(':group-coordinator') {
+  archivesBaseName = "kafka-group-coordinator"
+
+  dependencies {
+    implementation project(':server-common')
+    implementation project(':clients')
+    compileOnly libs.log4j
+    testImplementation libs.junitJupiter
+    testImplementation libs.jqwik
+    testImplementation libs.hamcrest
+    testImplementation libs.mockitoCore
+    testImplementation libs.mockitoInline

Review Comment:
   I removed all the unnecessary ones and I kept the minimal that I need for now.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] dajac merged pull request #12827: KAFKA-14363; Add new `group-coordinator` module (KIP-848)

Posted by GitBox <gi...@apache.org>.
dajac merged PR #12827:
URL: https://github.com/apache/kafka/pull/12827


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] ijuma commented on a diff in pull request #12827: KAFKA-14363; Add new `group-coordinator` module (KIP-848)

Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #12827:
URL: https://github.com/apache/kafka/pull/12827#discussion_r1016751779


##########
build.gradle:
##########
@@ -1222,6 +1225,41 @@ project(':metadata') {
   }
 }
 
+project(':group-coordinator') {
+  archivesBaseName = "kafka-group-coordinator"
+
+  dependencies {
+    implementation project(':server-common')
+    implementation project(':clients')
+    compileOnly libs.log4j
+    testImplementation libs.junitJupiter
+    testImplementation libs.jqwik
+    testImplementation libs.hamcrest
+    testImplementation libs.mockitoCore
+    testImplementation libs.mockitoInline

Review Comment:
   Do we need all these test dependencies? I'd consider removing `mockitoInline` and `hamcrest`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] dajac commented on a diff in pull request #12827: KAFKA-14363; Add new `group-coordinator` module (KIP-848)

Posted by GitBox <gi...@apache.org>.
dajac commented on code in PR #12827:
URL: https://github.com/apache/kafka/pull/12827#discussion_r1016821182


##########
build.gradle:
##########
@@ -1222,6 +1225,42 @@ project(':metadata') {
   }
 }
 
+project(':group-coordinator') {
+  archivesBaseName = "kafka-group-coordinator"
+
+  dependencies {
+    implementation project(':server-common')
+    implementation project(':clients')
+    implementation libs.slf4jApi
+
+    testImplementation project(':clients')

Review Comment:
   You're right. Gradle does not complain without them. I removed them.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] ijuma commented on a diff in pull request #12827: KAFKA-14363; Add new `group-coordinator` module (KIP-848)

Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #12827:
URL: https://github.com/apache/kafka/pull/12827#discussion_r1016813429


##########
build.gradle:
##########
@@ -1222,6 +1225,42 @@ project(':metadata') {
   }
 }
 
+project(':group-coordinator') {
+  archivesBaseName = "kafka-group-coordinator"
+
+  dependencies {
+    implementation project(':server-common')
+    implementation project(':clients')
+    implementation libs.slf4jApi
+
+    testImplementation project(':clients')

Review Comment:
   Isn't this redundant given that we already have an implementation dependency on clients? Or does gradle complain if you remove it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] dajac commented on a diff in pull request #12827: KAFKA-14363; Add new `group-coordinator` module (KIP-848)

Posted by GitBox <gi...@apache.org>.
dajac commented on code in PR #12827:
URL: https://github.com/apache/kafka/pull/12827#discussion_r1016794025


##########
build.gradle:
##########
@@ -1222,6 +1225,41 @@ project(':metadata') {
   }
 }
 
+project(':group-coordinator') {
+  archivesBaseName = "kafka-group-coordinator"
+
+  dependencies {
+    implementation project(':server-common')
+    implementation project(':clients')
+    compileOnly libs.log4j

Review Comment:
   Likely not. I fixed this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org