AndrewJSchofield commented on code in PR #20385:
URL: https://github.com/apache/kafka/pull/20385#discussion_r2314318522
##########
tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java:
##########
@@ -291,26 +297,41 @@ public ConsumerPerfOptions(String[] args) {
.describedAs("size")
.ofType(Integer.class)
.defaultsTo(1024 * 1024);
+ commandPropertiesOpt = parser.accepts("command-property", "Kafka
consumer related configuration properties like client.id. " +
+ "These configs take precedence over those passed via
--command-config or --consumer.config.")
+ .withRequiredArg()
+ .describedAs("prop1=val1,prop2=val2...")
Review Comment:
I do not think the code handles the format as described. For example, if I
use `--command-property client.id=c1,check.crcs=false`, then the client ID is
`"c1,check.crcs=false"`. The tricky part is that `kafka-producer-perf-test`
uses a different argument parsing library and it supports `--command-property
client.id=c1 bootstrap.servers=localhost:9092` with a space used as a
separator. That's very non-standard but it's already there for that tool at
least.
Personally, I think that the easiest is to support `prop=val` only and
permit multiple instances of `--command-property`, so someone can do
`--command-property client.id=c1 --command-property check.crcs=false`. I didn't
think the details would be subtle. wdyt?
The same comment applies to the share consumer perf test also.
--
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]
