Skip to content

[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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

PratRanj07
Copy link
Contributor

This PR intends to add two new field type and target assignment for DescribeConsumerGroup Response.

@Copilot Copilot AI review requested due to automatic review settings May 26, 2025 10:18
@PratRanj07 PratRanj07 requested review from a team as code owners May 26, 2025 10:18
Copy link

@Copilot Copilot AI left a 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 include targetAssignment and group type.
  • 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 added targetAssignment. Add assertions to verify group.targetAssignment is returned correctly when present.
type: ConsumerGroupTypes.CLASSIC,

examples/kafkajs/admin/describe-groups.js:75

  • The example logs the new type field but omits targetAssignment. Consider adding a line like console.log(\tTarget assignment: ${JSON.stringify(group.targetAssignment)}); to illustrate its usage.
console.log(`	Type: ${group.type}`);

* @enum {number}
* @readonly
* @memberof KafkaJS
* @see RdKafka.ConsumerGroupTypes
Copy link
Preview

Copilot AI May 26, 2025

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.

Suggested change
* @see RdKafka.ConsumerGroupTypes
* @see RdKafka.AdminClient.ConsumerGroupTypes

Copilot uses AI. Check for mistakes.

@sonarqube-confluent
Copy link

Passed

Analysis Details

0 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 0 Code Smells

Coverage and Duplications

  • Coverage 100.00% Coverage (71.80% Estimated after merge)
  • Duplications No duplication information (2.00% Estimated after merge)

Project ID: confluent-kafka-javascript

View in SonarQube

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.

1 participant