KAFKA-17897: Deprecate Admin.listConsumerGroups #19477
Conversation
There was a problem hiding this comment.
@AndrewJSchofield thanks for this patch. a couple of comments are left. PTAL
| * | ||
| * @param options The options to use when listing the consumer groups. | ||
| * @return The ListConsumerGroupsResult. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
@Deprecated(since = "4.1", forRemoval = true)
| * | ||
| * @return The ListConsumerGroupsResult. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
@Deprecated(since = "4.1", forRemoval = true)
| /** | ||
| * Returns the list of protocol types that are requested or empty if no protocol types have been specified. | ||
| */ | ||
| public Set<String> protocolTypes() { |
There was a problem hiding this comment.
I guess there will be a follow-up to use this field?
| @@ -45,6 +57,11 @@ public ListGroupsOptions inGroupStates(Set<GroupState> groupStates) { | |||
| return this; | |||
| } | |||
|
|
|||
| public ListGroupsOptions withProtocolTypes(Set<String> protocolTypes) { | |||
| this.protocolTypes = (protocolTypes == null || protocolTypes.isEmpty()) ? Set.of() : Set.copyOf(protocolTypes); | |||
There was a problem hiding this comment.
Could you please change the Collections.emptySet() to Set.of for the line#56?
| ListConsumerGroupsResult listConsumerGroups(ListConsumerGroupsOptions options); | ||
|
|
||
| /** | ||
| * List the consumer groups available in the cluster with the default options. | ||
| * <p> | ||
| * This is a convenience method for {@link #listConsumerGroups(ListConsumerGroupsOptions)} with default options. | ||
| * See the overload for more details. | ||
| * @deprecated Since 4.1. Use {@link Admin#listGroups(ListGroupsOptions)} instead. |
There was a problem hiding this comment.
Use {@link Admin#listGroups()} instead.
| * Only consumer groups will be returned by listGroups(). | ||
| * This operation sets filters on group type and protocol type which select consumer groups. | ||
| */ | ||
| public ListGroupsOptions forConsumerGroups() { |
There was a problem hiding this comment.
This new API is not in KIP-1043, therefore its code style is probably open for discussion 😃
Given that this API appears to be a helper function for creating a specific query, perhaps we could refactor it as a static method.
public static ListGroupsOptions forConsumerGroups() {
return new ListGroupsOptions()
.withTypes(Set.of(GroupType.CLASSIC, GroupType.CONSUMER))
.withProtocolTypes(Set.of("", ConsumerProtocol.PROTOCOL_TYPE));
}and then the code new ListGroupsOptions().forConsumerGroups() can be replaced by ListGroupsOptions.forConsumerGroups()
The final part of KIP-1043 is to deprecate Admin.listConsumerGroups() in
favour of Admin.listGroups() which works for all group types.