mimaison commented on code in PR #17507:
URL: https://github.com/apache/kafka/pull/17507#discussion_r1835514323


##########
core/src/test/java/kafka/admin/ConfigCommandTest.java:
##########
@@ -508,100 +426,56 @@ public void testExpectedEntityTypeNames(List<String> 
expectedTypes, List<String>
         assertEquals(createOpts.entityTypes().toSeq(), seq(expectedTypes));
         assertEquals(createOpts.entityNames().toSeq(), seq(expectedNames));
     }
-
-    public void doTestOptionEntityTypeNames(boolean zkConfig) {
-        List<String> connectOpts = zkConfig
-            ? Arrays.asList("--zookeeper", ZK_CONNECT)
-            : Arrays.asList("--bootstrap-server", "localhost:9092");
-
-        // zookeeper config only supports "users" and "brokers" entity type
-        if (!zkConfig) {
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.TOPIC), 
Collections.singletonList("A"), connectOpts, "--entity-type", "topics", 
"--entity-name", "A");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.IP), 
Collections.singletonList("1.2.3.4"), connectOpts, "--entity-name", "1.2.3.4", 
"--entity-type", "ips");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.CLIENT_METRICS),
 Collections.singletonList("A"), connectOpts, "--entity-type", 
"client-metrics", "--entity-name", "A");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.GROUP), 
Collections.singletonList("A"), connectOpts, "--entity-type", "groups", 
"--entity-name", "A");
-            testExpectedEntityTypeNames(Arrays.asList(ConfigType.USER, 
ConfigType.CLIENT), Arrays.asList("A", ""), connectOpts,
-                "--entity-type", "users", "--entity-type", "clients", 
"--entity-name", "A", "--entity-default");
-            testExpectedEntityTypeNames(Arrays.asList(ConfigType.USER, 
ConfigType.CLIENT), Arrays.asList("", "B"), connectOpts,
-                "--entity-default", "--entity-name", "B", "--entity-type", 
"users", "--entity-type", "clients");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.TOPIC), 
Collections.singletonList("A"), connectOpts, "--topic", "A");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.IP), 
Collections.singletonList("1.2.3.4"), connectOpts, "--ip", "1.2.3.4");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.GROUP), 
Collections.singletonList("A"), connectOpts, "--group", "A");
-            testExpectedEntityTypeNames(Arrays.asList(ConfigType.CLIENT, 
ConfigType.USER), Arrays.asList("B", "A"), connectOpts, "--client", "B", 
"--user", "A");
-            testExpectedEntityTypeNames(Arrays.asList(ConfigType.CLIENT, 
ConfigType.USER), Arrays.asList("B", ""), connectOpts, "--client", "B", 
"--user-defaults");
-            testExpectedEntityTypeNames(Arrays.asList(ConfigType.CLIENT, 
ConfigType.USER), Collections.singletonList("A"), connectOpts,
-                "--entity-type", "clients", "--entity-type", "users", 
"--entity-name", "A");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.TOPIC), 
Collections.emptyList(), connectOpts, "--entity-type", "topics");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.IP), 
Collections.emptyList(), connectOpts, "--entity-type", "ips");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.GROUP), 
Collections.emptyList(), connectOpts, "--entity-type", "groups");
-            
testExpectedEntityTypeNames(Collections.singletonList(ConfigType.CLIENT_METRICS),
 Collections.emptyList(), connectOpts, "--entity-type", "client-metrics");
-        }
-
+    @Test

Review Comment:
   Nit: can we add a newline between both tests?



##########
core/src/main/scala/kafka/admin/ConfigCommand.scala:
##########
@@ -952,19 +644,11 @@ object ConfigCommand extends Logging {
       val hasEntityDefault = entityNames.exists(_.isEmpty)
 
       val numConnectOptions = (if (options.has(bootstrapServerOpt)) 1 else 0) +
-        (if (options.has(bootstrapControllerOpt)) 1 else 0) +
-        (if (options.has(zkConnectOpt)) 1 else 0)
+        (if (options.has(bootstrapControllerOpt)) 1 else 0)
       if (numConnectOptions == 0)
-        throw new IllegalArgumentException("One of the required 
--bootstrap-server, --boostrap-controller, or --zookeeper arguments must be 
specified")
+        throw new IllegalArgumentException("One of the required 
--bootstrap-server or --bootstrap-controller arguments must be specified")

Review Comment:
   I think this line is not reachable anymore since we now throw on line 627 is 
both --bootstrap-server and --bootstrap-controller not are present



-- 
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