Skip to content

Conversation

DL1231
Copy link
Contributor

@DL1231 DL1231 commented Sep 19, 2025

The metrics attribute in StreamsGroup is not used anymore. This
patch removes it.

Reviewers: Ken Huang [email protected], Lucas Brutschy
[email protected], Chia-Ping Tsai [email protected]

@github-actions github-actions bot added triage PRs from the community group-coordinator small Small PRs labels Sep 19, 2025
Copy link
Collaborator

@m1a2st m1a2st left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Copy link
Member

@lucasbru lucasbru left a comment

Choose a reason for hiding this comment

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

LGTM

@lucasbru lucasbru requested a review from Copilot September 19, 2025 08:02
Copy link
Contributor

@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

Removes the unused metrics dependency from StreamsGroup by dropping the metrics field and constructor parameter, and updating all visible instantiations accordingly.

  • Removed GroupCoordinatorMetricsShard field and constructor argument from StreamsGroup.
  • Updated all StreamsGroup constructions in tests and GroupMetadataManager to use the new signature.
  • Cleaned up now-unused imports and test scaffolding (mocks and TopicPartition usage).

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
StreamsGroupTest.java Updated all StreamsGroup instantiations to new constructor; removed related imports and mock setup.
StreamsGroup.java Removed metrics field and constructor parameter plus null check.
GroupMetadataManager.java Adjusted StreamsGroup instantiations to new constructor signature.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 214 to 218
public StreamsGroup(
LogContext logContext,
SnapshotRegistry snapshotRegistry,
String groupId,
GroupCoordinatorMetricsShard metrics
String groupId
) {
Copy link
Preview

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The removal of the GroupCoordinatorMetricsShard constructor parameter is a breaking change to this public constructor. If StreamsGroup is intended for external (non-internal) use, consider a deprecation cycle or documenting the change in release notes; otherwise, mark the class or constructor clearly as internal to avoid unintended API breakage.

Copilot uses AI. Check for mistakes.

@chia7712 chia7712 merged commit cfa0b41 into apache:trunk Sep 19, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants