Skip to content
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

Checking if partitions list is empty before allocating a new list #985

Conversation

alexandredantas
Copy link
Contributor

This check makes it possible to list all partitions assigned to a consumer group. In librdkafka, the method rd_kafka_ListConsumerGroupOffsets expects the partition to be NULL or having some content. However, in confluent-kafka-go a list is always created

Copy link
Contributor

@milindl milindl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for submitting this PR!

Please also add unit tests to adminapi_test.go, one test case for nil topic partition list, which should fail in the unit test with "context deadline exceeded", and one test with 0-sized topic partition list, which should fail with "NULL or non-empty topic partition list must be passed".

Please let me know if you require any help with the changes.

kafka/adminapi.go Outdated Show resolved Hide resolved
@frankgrimes97
Copy link

@milindl I believe the requested changes have been addressed... are any further changes required before this can be reviewed, approved and merged? Thanks!

@milindl
Copy link
Contributor

milindl commented Jun 5, 2023

This looks good to me, mostly.
I created an internal branch with this change, and raised a PR, so that our CI can run and validate it once before we can merge it. #1007. I've also made some changes to the tests, mostly to the integration tests, and added the CHANGELOG.md entry.

@frankgrimes97
Copy link

@milindl Any update on this?

@milindl
Copy link
Contributor

milindl commented Jun 28, 2023

The internal PR is merged. Thanks for this! It'll be released with 2.2.0

@milindl milindl closed this Jun 28, 2023
@alexandredantas alexandredantas deleted the bugfix/list-consumer-groups-with-empty-list branch June 29, 2023 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants