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