AndrewJSchofield commented on code in PR #16887:
URL: https://github.com/apache/kafka/pull/16887#discussion_r1718343472
##########
core/src/main/scala/kafka/admin/ConfigCommand.scala:
##########
@@ -562,6 +543,8 @@ object ConfigCommand extends Logging {
adminClient.describeCluster(new
DescribeClusterOptions()).nodes().get().asScala.map(_.idString).toSeq :+
BrokerDefaultEntityName
case ConfigType.CLIENT_METRICS =>
adminClient.listClientMetricsResources().all().get().asScala.map(_.name).toSeq
+ case ConfigType.GROUP =>
+ adminClient.listConsumerGroups(new
ListConsumerGroupsOptions().withTypes(Seq(GroupType.CONSUMER).toSet.asJava)).all.get.asScala.map(_.groupId).toSeq
Review Comment:
This is interesting. Because you've used `withTypes(GroupType.CONSUMER)`,
this only lists out the consumers which are using the new consumer group
protocol. So, I can set configs for other consumers, but `--describe` without a
group name will not show them.
Really, the solution here I think is in KIP-1043 which adds
`adminClient.listGroups()` so that all groups regardless of type can be listed.
Once that KIP is approved, I'll improve this area. For now, I think
`Seq(GroupType.CONSUMER, GroupType.CLASSIC)` is probably the best you can do.
That does at least list out the configs for all new and classic consumer groups.
##########
core/src/main/scala/kafka/admin/ConfigCommand.scala:
##########
@@ -57,6 +58,7 @@ import scala.collection._
* <li> broker-logger: --broker-logger <broker-id> OR --entity-type
broker-loggers --entity-name <broker-id>
* <li> ip: --ip <ip> OR --entity-type ips --entity-name <ip>
* <li> client-metrics: --client-metrics <name> OR --entity-type
client-metrics --entity-name <name>
+ * <li> group: --group <group> OR --entiry-type groups --entity-name
<group>
Review Comment:
Also, you didn't add the `--group` option. I think you should either add
that option, or change this comment to remove `--group`.
I've noticed that I didn't implement `--client-metrics` when I did
`--entity-type client-metrics`. I'll open a JIRA to fix that.
##########
core/src/main/scala/kafka/admin/ConfigCommand.scala:
##########
@@ -57,6 +58,7 @@ import scala.collection._
* <li> broker-logger: --broker-logger <broker-id> OR --entity-type
broker-loggers --entity-name <broker-id>
* <li> ip: --ip <ip> OR --entity-type ips --entity-name <ip>
* <li> client-metrics: --client-metrics <name> OR --entity-type
client-metrics --entity-name <name>
+ * <li> group: --group <group> OR --entiry-type groups --entity-name
<group>
Review Comment:
nit: "entiry" should be "entity"
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
