kirktrue commented on code in PR #16951:
URL: https://github.com/apache/kafka/pull/16951#discussion_r1728053002
##########
tools/src/main/java/org/apache/kafka/tools/VerifiableConsumer.java:
##########
@@ -529,6 +530,24 @@ private static ArgumentParser argParser() {
.metavar("TOPIC")
.help("Consumes messages from this topic.");
+ parser.addArgument("--group-protocol")
+ .action(store())
+ .required(false)
+ .type(String.class)
+ .setDefault(ConsumerConfig.DEFAULT_GROUP_PROTOCOL)
Review Comment:
Hi @AndrewJSchofield 👋
> I believe the idea is that `DEFAULT_GROUP_PROTOCOL` changes to "CONSUMER"
at an AK major release boundary. That would have the side-effect that the
behaviour of this code will change at that boundary, probably unexpectedly.
As of 3.8 `DEFAULT_GROUP_PROTOCOL` is used to determine which implementation
of the `KafkaConsumerDelegate` to instantiate and use. As you point out, in 4.0
`DEFAULT_GROUP_PROTOCOL` switches from `CLASSIC` to `CONSUMER`. Won't that
change affect users in a **much** more substantial way?
> I suggest that you set the default to one of the concrete values, rather
than the config default, and then make a conscious change in this case to move
it forwards to the new value. For example, you could use a default of "CLASSIC"
for releases less than 4.0, and "CONSUMER" for releases 4.0 and later.
Given that the core client defaults to `DEFAULT_GROUP_PROTOCOL`, wouldn't we
want this tool to behave likewise?
Sorry, I'm very tired, so I'm probably missing your point entirely 😫
--
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]