You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "dajac (via GitHub)" <gi...@apache.org> on 2023/02/06 08:44:14 UTC

[GitHub] [kafka] dajac opened a new pull request, #13200: MINOR: Move `__consumer_offsets` records from `core` to `group-coordinator`

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

   This patch moves the current `__consumer_offsets` records from the `core` module to the new `group-coordinator` module.
   
   ### 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] dajac merged pull request #13200: KAFKA-14678: Move `__consumer_offsets` records from `core` to `group-coordinator`

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac merged PR #13200:
URL: https://github.com/apache/kafka/pull/13200


-- 
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 #13200: MINOR: Move `__consumer_offsets` records from `core` to `group-coordinator`

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #13200:
URL: https://github.com/apache/kafka/pull/13200#discussion_r1097389168


##########
checkstyle/import-control.xml:
##########
@@ -341,7 +341,9 @@
   <subpackage name="coordinator">
     <subpackage name="group">
       <allow pkg="org.apache.kafka.common.message" />
+      <allow pkg="org.apache.kafka.common.protocol" />

Review Comment:
   Yes. The generated code needs 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] clolov commented on a diff in pull request #13200: MINOR: Move `__consumer_offsets` records from `core` to `group-coordinator`

Posted by "clolov (via GitHub)" <gi...@apache.org>.
clolov commented on code in PR #13200:
URL: https://github.com/apache/kafka/pull/13200#discussion_r1097379412


##########
checkstyle/import-control.xml:
##########
@@ -341,7 +341,9 @@
   <subpackage name="coordinator">
     <subpackage name="group">
       <allow pkg="org.apache.kafka.common.message" />
+      <allow pkg="org.apache.kafka.common.protocol" />

Review Comment:
   For my curiosity, are these lines needed in the context of this pull request or they are a remnant from a previous change?



##########
build.gradle:
##########
@@ -1266,6 +1272,23 @@ project(':group-coordinator') {
   javadoc {
     enabled = false
   }
+
+  task processMessages(type:JavaExec) {
+    mainClass = "org.apache.kafka.message.MessageGenerator"
+    classpath = configurations.generator
+    args = [ "-p", "org.apache.kafka.coordinator.group.generated",

Review Comment:
   I noticed that sometimes we append `.generated` in similar closures and sometimes we do not. Are we appending `.generated` going forward?



-- 
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 #13200: MINOR: Move `__consumer_offsets` records from `core` to `group-coordinator`

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #13200:
URL: https://github.com/apache/kafka/pull/13200#discussion_r1097390286


##########
build.gradle:
##########
@@ -1266,6 +1272,23 @@ project(':group-coordinator') {
   javadoc {
     enabled = false
   }
+
+  task processMessages(type:JavaExec) {
+    mainClass = "org.apache.kafka.message.MessageGenerator"
+    classpath = configurations.generator
+    args = [ "-p", "org.apache.kafka.coordinator.group.generated",

Review Comment:
   Yeah, I have noticed this as well. Personally, I like having `generated` in the package name because it makes the intent clear in the code. I am also fine if folks prefer to 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 pull request #13200: MINOR: Move `__consumer_offsets` records from `core` to `group-coordinator`

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #13200:
URL: https://github.com/apache/kafka/pull/13200#issuecomment-1419045542

   @mimaison Could you review this one?


-- 
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 pull request #13200: KAFKA-14678: Move `__consumer_offsets` records from `core` to `group-coordinator`

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #13200:
URL: https://github.com/apache/kafka/pull/13200#issuecomment-1419247177

   @mimaison Thanks! Created KAFKA-14678.


-- 
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