rondagostino commented on a change in pull request #10811: URL: https://github.com/apache/kafka/pull/10811#discussion_r667389229
########## File path: core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala ########## @@ -133,13 +183,13 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging { } @Test - def shouldParseArgumentsForIpEntityType(): Unit = { - testArgumentParse("ips", zkConfig = false) + def shouldParseArgumentsForIpEntityTypeUsingZookeeper(): Unit = { Review comment: s/shouldParse/shouldFailParse/ ########## File path: core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala ########## @@ -118,7 +168,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging { } @Test - def shouldParseArgumentsForBrokersEntityTypeUsingZookeeper(): Unit = { + def shouldFailParseArgumentsForBrokersEntityTypeUsingZookeeper(): Unit = { Review comment: I think this name change is incorrect -- it still parses correctly ########## File path: core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala ########## @@ -1399,6 +1454,11 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging { ConfigCommand.alterConfigWithZk(null, del512, CredentialChange("userB", Set(), 4096)) } + @Test + def testQuotaConfigEntityUsingZookeeper(): Unit = { Review comment: s/testQuotaConfigEntityUsingZookeeper/testQuotaConfigEntityUsingZookeeperNotAllowed/ ########## File path: docs/upgrade.html ########## @@ -46,8 +46,10 @@ <h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3 <code>AclBindingFilter</code>.</li> <li>The <code>Admin.electedPreferredLeaders()</code> methods were removed. Please use <code>Admin.electLeaders</code> instead.</li> <li>The <code>kafka-preferred-replica-election</code> command line tool was removed. Please use <code>kafka-leader-election</code> instead.</li> - <li>The <code>--zookeeper</code> option was removed from the <code>bin/kafka-topics.sh</code> and <code>bin/kafka-reassign-partitions.sh</code> command line tools. + <li>The <code>--zookeeper</code> option was removed from the <code>kafka-topics</code> and <code>kafka-reassign-partitions</code> command line tools. Please use <code>--bootstrap-server</code> instead.</li> + <li>In <code>kafka-configs</code> command line tool, the <code>--zookeeper</code> option only supported for <a href="#security_sasl_scram_credentials">SCRAM Credentials configuration</a> + and <a href="#dynamicbrokerconfigs">update password config before broker started</a>.</li> Review comment: ```suggestion and <a href="#dynamicbrokerconfigs">updating dynamic broker configs before brokers are started</a>.</li> ``` ########## File path: docs/upgrade.html ########## @@ -46,8 +46,10 @@ <h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3 <code>AclBindingFilter</code>.</li> <li>The <code>Admin.electedPreferredLeaders()</code> methods were removed. Please use <code>Admin.electLeaders</code> instead.</li> <li>The <code>kafka-preferred-replica-election</code> command line tool was removed. Please use <code>kafka-leader-election</code> instead.</li> - <li>The <code>--zookeeper</code> option was removed from the <code>bin/kafka-topics.sh</code> and <code>bin/kafka-reassign-partitions.sh</code> command line tools. + <li>The <code>--zookeeper</code> option was removed from the <code>kafka-topics</code> and <code>kafka-reassign-partitions</code> command line tools. Please use <code>--bootstrap-server</code> instead.</li> + <li>In <code>kafka-configs</code> command line tool, the <code>--zookeeper</code> option only supported for <a href="#security_sasl_scram_credentials">SCRAM Credentials configuration</a> Review comment: s/In/In the/ s/option only supported/option is only supported/ ########## File path: core/src/main/scala/kafka/admin/ConfigCommand.scala ########## @@ -731,10 +726,11 @@ object ConfigCommand extends Config { class ConfigCommandOptions(args: Array[String]) extends CommandDefaultOptions(args) { val zkConnectOpt = parser.accepts("zookeeper", "DEPRECATED. The connection string for the zookeeper connection in the form host:port. " + - "Multiple URLS can be given to allow fail-over. Replaced by --bootstrap-server, REQUIRED unless --bootstrap-server is given.") - .withRequiredArg - .describedAs("urls") - .ofType(classOf[String]) + "Multiple URLS can be given to allow fail-over. Replaced by --bootstrap-server except configuring SCRAM credentials for user or " + Review comment: s/except configuring SCRAM credentials for user or /except for when configuring SCRAM credentials for users or / ########## File path: core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala ########## @@ -109,7 +159,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging { @Test def shouldParseArgumentsForTopicsEntityTypeUsingZookeeper(): Unit = { Review comment: s/shouldParse/shouldFailParse/ ########## File path: core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala ########## @@ -1474,13 +1534,13 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging { } @Test - def testQuotaConfigEntityUsingZookeeper(): Unit = { - doTestQuotaConfigEntity(zkConfig = true) + def testQuotaConfigEntity(): Unit = { + doTestQuotaConfigEntity(zkConfig = false) } @Test - def testQuotaConfigEntity(): Unit = { - doTestQuotaConfigEntity(zkConfig = false) + def testUserClientQuotaOptsUsingZookeeper(): Unit = { Review comment: s/testUserClientQuotaOptsUsingZookeeper/testUserClientQuotaOptsUsingZookeeperNotAllowed/ ########## File path: core/src/main/scala/kafka/admin/ConfigCommand.scala ########## @@ -731,10 +726,11 @@ object ConfigCommand extends Config { class ConfigCommandOptions(args: Array[String]) extends CommandDefaultOptions(args) { val zkConnectOpt = parser.accepts("zookeeper", "DEPRECATED. The connection string for the zookeeper connection in the form host:port. " + - "Multiple URLS can be given to allow fail-over. Replaced by --bootstrap-server, REQUIRED unless --bootstrap-server is given.") - .withRequiredArg - .describedAs("urls") - .ofType(classOf[String]) + "Multiple URLS can be given to allow fail-over. Replaced by --bootstrap-server except configuring SCRAM credentials for user or " + + "password configs before starting brokers. REQUIRED unless --bootstrap-server is given.") Review comment: s/password configs before starting brokers/dynamic broker configs before starting brokers/ -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org