You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "AndrewJSchofield (via GitHub)" <gi...@apache.org> on 2024/12/06 17:54:26 UTC
[PR] KAFKA-17548: Improve group listing integration test [kafka]
AndrewJSchofield opened a new pull request, #18081:
URL: https://github.com/apache/kafka/pull/18081
### 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
Re: [PR] KAFKA-17548: Improve group listing integration test [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #18081:
URL: https://github.com/apache/kafka/pull/18081#discussion_r1874419762
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2147,15 +2147,17 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
TestUtils.waitUntilTrue(() => {
val matching = client.listConsumerGroups.all.get.asScala.filter(group =>
group.groupId == testGroupId &&
- group.state.get == ConsumerGroupState.STABLE)
+ group.state.get == ConsumerGroupState.STABLE &&
+ group.groupState.get == GroupState.STABLE)
Review Comment:
Just a query: Why do we need to check `group.state.get` && `group.groupState.get` but just not `group.groupState.get` as the other method is deprecated and uses groupState itself internally?
--
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
Re: [PR] KAFKA-17548: Improve group listing integration test [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18081:
URL: https://github.com/apache/kafka/pull/18081#discussion_r1874495627
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2147,15 +2147,17 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
TestUtils.waitUntilTrue(() => {
val matching = client.listConsumerGroups.all.get.asScala.filter(group =>
group.groupId == testGroupId &&
- group.state.get == ConsumerGroupState.STABLE)
+ group.state.get == ConsumerGroupState.STABLE &&
+ group.groupState.get == GroupState.STABLE)
Review Comment:
I want to ensure that they give the same result and continue to give the same result in the future. I know it should be obvious that they do, but a lot of testing is like that. In 5.0, we'll remove `ConsumerGroupState` entirely.
--
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
Re: [PR] KAFKA-17548: Improve group listing integration test [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #18081:
URL: https://github.com/apache/kafka/pull/18081#discussion_r1874516370
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2147,15 +2147,17 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
TestUtils.waitUntilTrue(() => {
val matching = client.listConsumerGroups.all.get.asScala.filter(group =>
group.groupId == testGroupId &&
- group.state.get == ConsumerGroupState.STABLE)
+ group.state.get == ConsumerGroupState.STABLE &&
+ group.groupState.get == GroupState.STABLE)
Review Comment:
Thanks for explaining.
--
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
Re: [PR] KAFKA-17548: Improve group listing integration test [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield merged PR #18081:
URL: https://github.com/apache/kafka/pull/18081
--
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