justinrlee commented on code in PR #12797:
URL: https://github.com/apache/kafka/pull/12797#discussion_r1042238071
##########
core/src/main/scala/kafka/admin/BrokerApiVersionsCommand.scala:
##########
@@ -95,8 +96,6 @@ object BrokerApiVersionsCommand {
def checkArgs(): Unit = {
CommandLineUtils.printHelpAndExitIfNeeded(this, "This tool helps to
retrieve broker version information.")
- // check required args
- CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt)
Review Comment:
Hey @dengziming thanks for the comment. This is sort of addressed in the PR
comment. Basically, if the user doesn't pass bootstrap server in either arg or
property file, you get an exit code of 1 and fairly intuitive error `Missing
required configuration "bootstrap.servers" which has no default value.`.
I was thinking this would be sufficient, but I can look at adding explicit
error handling if you think that's better. Was trying to keep this a relatively
small PR.
> For example, specifying ONLY the property file without bootstrap.servers
specified will result in output that looks like this, which is fairly intuitive
to address:
```
./kafka_2.13-3.4.0-SNAPSHOT/bin/kafka-broker-api-versions.sh
--command-config client-no-bootstrap.properties
Exception in thread "main" org.apache.kafka.common.config.ConfigException:
Missing required configuration "bootstrap.servers" which has no default value.
at
org.apache.kafka.common.config.ConfigDef.parseValue(ConfigDef.java:493)
at org.apache.kafka.common.config.ConfigDef.parse(ConfigDef.java:483)
at
org.apache.kafka.common.config.AbstractConfig.<init>(AbstractConfig.java:113)
at
org.apache.kafka.common.config.AbstractConfig.<init>(AbstractConfig.java:146)
at
kafka.admin.BrokerApiVersionsCommand$AdminClient$AdminConfig.<init>(BrokerApiVersionsCommand.scala:265)
at
kafka.admin.BrokerApiVersionsCommand$AdminClient$.create(BrokerApiVersionsCommand.scala:269)
at
kafka.admin.BrokerApiVersionsCommand$AdminClient$.create(BrokerApiVersionsCommand.scala:267)
at
kafka.admin.BrokerApiVersionsCommand$.createAdminClient(BrokerApiVersionsCommand.scala:79)
at
kafka.admin.BrokerApiVersionsCommand$.execute(BrokerApiVersionsCommand.scala:60)
at
kafka.admin.BrokerApiVersionsCommand$.main(BrokerApiVersionsCommand.scala:55)
at
kafka.admin.BrokerApiVersionsCommand.main(BrokerApiVersionsCommand.scala)
```
--
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]
