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


Reply via email to