chia7712 commented on code in PR #19477:
URL: https://github.com/apache/kafka/pull/19477#discussion_r2047281878
##########
clients/src/main/java/org/apache/kafka/clients/admin/ListGroupsOptions.java:
##########
@@ -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);
Review Comment:
Could you please change the `Collections.emptySet()` to `Set.of` for the
line#56?
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -879,20 +879,24 @@ default DescribeConsumerGroupsResult
describeConsumerGroups(Collection<String> g
/**
* List the consumer groups available in the cluster.
+ * @deprecated Since 4.1. Use {@link Admin#listGroups(ListGroupsOptions)}
instead.
*
* @param options The options to use when listing the consumer groups.
* @return The ListConsumerGroupsResult.
*/
+ @Deprecated
Review Comment:
`@Deprecated(since = "4.1", forRemoval = true)`
##########
clients/src/main/java/org/apache/kafka/clients/admin/ListGroupsOptions.java:
##########
@@ -32,8 +33,19 @@
@InterfaceStability.Evolving
public class ListGroupsOptions extends AbstractOptions<ListGroupsOptions> {
- private Set<GroupState> groupStates = Collections.emptySet();
- private Set<GroupType> types = Collections.emptySet();
+ private Set<GroupState> groupStates = Set.of();
+ private Set<GroupType> types = Set.of();
+ private Set<String> protocolTypes = Set.of();
+
+ /**
+ * 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() {
Review Comment:
This new API is not in KIP-1043, therefore its code style is probably open
for discussion :smiley:
Given that this API appears to be a helper function for creating a specific
query, perhaps we could refactor it as a static method.
```java
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()`
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -879,20 +879,24 @@ default DescribeConsumerGroupsResult
describeConsumerGroups(Collection<String> g
/**
* List the consumer groups available in the cluster.
+ * @deprecated Since 4.1. Use {@link Admin#listGroups(ListGroupsOptions)}
instead.
*
* @param options The options to use when listing the consumer groups.
* @return The ListConsumerGroupsResult.
*/
+ @Deprecated
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.
Review Comment:
`Use {@link Admin#listGroups()} instead.`
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -879,20 +879,24 @@ default DescribeConsumerGroupsResult
describeConsumerGroups(Collection<String> g
/**
* List the consumer groups available in the cluster.
+ * @deprecated Since 4.1. Use {@link Admin#listGroups(ListGroupsOptions)}
instead.
*
* @param options The options to use when listing the consumer groups.
* @return The ListConsumerGroupsResult.
*/
+ @Deprecated
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.
*
* @return The ListConsumerGroupsResult.
*/
+ @Deprecated
Review Comment:
`@Deprecated(since = "4.1", forRemoval = true)`
##########
clients/src/main/java/org/apache/kafka/clients/admin/ListGroupsOptions.java:
##########
@@ -61,6 +78,13 @@ public Set<GroupState> groupStates() {
return groupStates;
}
+ /**
+ * Returns the list of protocol types that are requested or empty if no
protocol types have been specified.
+ */
+ public Set<String> protocolTypes() {
Review Comment:
I guess there will be a follow-up to use this field?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
