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

KAFKA-17956: Remove Admin.listShareGroups #17912

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

Conversation

AndrewJSchofield
Copy link
Collaborator

KIP-1043 introduced Admin.listGroups as the way to list all types of groups. As a result, Admin.listShareGroups has been removed. This PR is the final step of the removal.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added the tools label Nov 22, 2024
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@AndrewJSchofield thanks for this cleanup!

@@ -3855,48 +3855,6 @@ public DeleteConsumerGroupOffsetsResult deleteConsumerGroupOffsets(
return new DeleteConsumerGroupOffsetsResult(future.get(CoordinatorKey.byGroupId(groupId)), partitions);
}

private static final class ListShareGroupsResults {
private final List<Throwable> errors;
private final HashMap<String, ShareGroupListing> listings;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please remove ShareGroupListing as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

* The API of this class is evolving, see {@link Admin} for details.
*/
@InterfaceStability.Evolving
public class ListShareGroupsResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please remove those classes from KIP-932

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. I have aligned KIP-932 with KIP-1043. This means:

  • Removal of Admin.listShareGroups and the associated classes.
  • Replacement of ShareGroupState with the generic GroupState.

@AndrewJSchofield
Copy link
Collaborator Author

Test failure known and fixed by https://issues.apache.org/jira/browse/KAFKA-18078.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants