-
Notifications
You must be signed in to change notification settings - Fork 18
[KIP-848]: Structure change for DescribeConsumerGroups #327
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
base: dev_kip848Testing
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds two new fields (type
and targetAssignment
) to the DescribeConsumerGroup response, updates native bindings, JS exports, tests, and examples.
- Extend C++ binding (
common.cc
) to includetargetAssignment
and grouptype
. - Expose and document
ConsumerGroupTypes
in JS library modules. - Update tests and examples to consume the new
type
field.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
test/promisified/admin/describe_groups.spec.js | Import and assert the new type field |
src/common.cc | Add targetAssignment and type properties to V8 objects |
lib/rdkafka.js | Export ConsumerGroupTypes |
lib/kafkajs/_kafka.js | Include ConsumerGroupTypes in client exports |
lib/kafkajs/_admin.js | Document and expose ConsumerGroupTypes enum |
lib/admin.js | Define and freeze ConsumerGroupTypes enum |
examples/kafkajs/admin/describe-groups.js | Log the new type field in sample output |
Comments suppressed due to low confidence (2)
test/promisified/admin/describe_groups.spec.js:89
- Tests cover the new
type
field but don’t assert the newly addedtargetAssignment
. Add assertions to verifygroup.targetAssignment
is returned correctly when present.
type: ConsumerGroupTypes.CLASSIC,
examples/kafkajs/admin/describe-groups.js:75
- The example logs the new
type
field but omitstargetAssignment
. Consider adding a line likeconsole.log(
\tTarget assignment: ${JSON.stringify(group.targetAssignment)});
to illustrate its usage.
console.log(` Type: ${group.type}`);
* @enum {number} | ||
* @readonly | ||
* @memberof KafkaJS | ||
* @see RdKafka.ConsumerGroupTypes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @see
tag references RdKafka.ConsumerGroupTypes
but the enum is on RdKafka.AdminClient.ConsumerGroupTypes
. Update the annotation to match the actual export path.
* @see RdKafka.ConsumerGroupTypes | |
* @see RdKafka.AdminClient.ConsumerGroupTypes |
Copilot uses AI. Check for mistakes.
This comment has been minimized.
This comment has been minimized.
7c59a2f
to
f7e33a5
Compare
This PR intends to add two new field type and target assignment for DescribeConsumerGroup Response.