-
Notifications
You must be signed in to change notification settings - Fork 14.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MINOR: Replace enum name with state name when parsing ConsumerGroupState
#18315
base: trunk
Are you sure you want to change the base?
Conversation
Can we add a test for this please? |
Also, is this also broken in the 4.0 branch? If so, let's note that a cherry-pick to 4.0 is also required. |
it's also broken. I'll open a cherry-pick pr once this is merged. https://github.com/apache/kafka/blob/4.0/clients/src/main/java/org/apache/kafka/clients/admin/ConsumerGroupDescription.java#L187 |
@dongnuo123 looks like the import order needs to be fixed. |
@jolshan Could you trigger the build again? |
@dongnuo123 nice find. We need to replace ".name()" by ".toString" as well: kafka/clients/src/main/java/org/apache/kafka/clients/admin/ConsumerGroupListing.java Line 140 in 4567f39
kafka/clients/src/main/java/org/apache/kafka/clients/admin/ListConsumerGroupsOptions.java Line 87 in 4567f39
kafka/clients/src/main/java/org/apache/kafka/clients/admin/ListConsumerGroupsOptions.java Line 87 in 4567f39
kafka/clients/src/main/java/org/apache/kafka/clients/admin/ConsumerGroupListing.java Line 56 in 4567f39
kafka/clients/src/main/java/org/apache/kafka/clients/admin/ConsumerGroupListing.java Line 75 in 4567f39
|
I notice there is something make our CI hanging bit I think it is not related this PR. |
ConsumerGroupDescription.state()
ConsumerGroupState
groupState.name()
returns the name of the enum whileConsumerGroupState#parse
uses the state name.Committer Checklist (excluded from commit message)