You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/06 07:36:02 UTC
[GitHub] [kafka] showuon commented on a change in pull request #10827: KAFKA-12899: Support --bootstrap-server in ReplicaVerificationTool
showuon commented on a change in pull request #10827:
URL: https://github.com/apache/kafka/pull/10827#discussion_r646089490
##########
File path: core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala
##########
@@ -116,7 +121,13 @@ object ReplicaVerificationTool extends Logging {
if (options.has(versionOpt)) {
CommandLineUtils.printVersionAndDie()
}
- CommandLineUtils.checkRequiredArgs(parser, options, brokerListOpt)
+
+ val effectiveBrokerListOpt = if (options.has(bootstrapServerOpt))
+ bootstrapServerOpt
+ else
+ brokerListOpt
Review comment:
Is it possible that user provided both `bootstrapServerOpt` and `brokerListOpt` at the same time?
##########
File path: core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala
##########
@@ -116,7 +121,13 @@ object ReplicaVerificationTool extends Logging {
if (options.has(versionOpt)) {
CommandLineUtils.printVersionAndDie()
}
- CommandLineUtils.checkRequiredArgs(parser, options, brokerListOpt)
+
+ val effectiveBrokerListOpt = if (options.has(bootstrapServerOpt))
+ bootstrapServerOpt
+ else
+ brokerListOpt
+
+ CommandLineUtils.checkRequiredArgs(parser, options, effectiveBrokerListOpt)
val regex = options.valueOf(topicWhiteListOpt)
val topicWhiteListFiler = new IncludeList(regex)
Review comment:
typo: `topicWhiteListFiler` ->‵topicWhiteListFilter`
##########
File path: core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala
##########
@@ -75,9 +75,14 @@ object ReplicaVerificationTool extends Logging {
def main(args: Array[String]): Unit = {
val parser = new OptionParser(false)
- val brokerListOpt = parser.accepts("broker-list", "REQUIRED: The list of hostname and port of the server to connect to.")
+ val brokerListOpt = parser.accepts("broker-list", "DEPRECATED, use --bootstrap-server instead; ignored if --bootstrap-server is specified. The list of hostname and port of the server to connect to.")
.withRequiredArg
- .describedAs("hostname:port,...,hostname:port")
+ .describedAs("HOST1:PORT1,...,HOST3:PORT3")
+ .ofType(classOf[String])
+ val bootstrapServerOpt = parser.accepts("bootstrap-server", "REQUIRED. The list of hostname and port of the server to connect to.")
Review comment:
add tests +1
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org