[kafka] branch trunk updated: KAFKA-13396: Allow create topic without partition/replicaFactor (#11429)

[ij...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ijuma pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 43bcc56  KAFKA-13396: Allow create topic without partition/replicaFactor (#11429)
43bcc56 is described below

commit 43bcc5682da82a602a4c0a000dc7433d0507b450
Author: Luke Chen <sh...@gmail.com>
AuthorDate: Thu Nov 4 21:44:05 2021 +0800

    KAFKA-13396: Allow create topic without partition/replicaFactor (#11429)
    
    [KIP-464](https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic) (PR: https://github.com/apache/kafka/pull/6728)
    made it possible to create topics without passing partition count and/or replica factor when using
    the admin client. We incorrectly disallowed this via https://github.com/apache/kafka/pull/10457 while
    trying to ensure validation was consistent between ZK and the admin client (in this case the
    inconsistency was intentional).
    
    Fix this regression and add tests for the command lines in quick start (i.e. create topic and describe
    topic) to make sure it won't be broken in the future.
    
    Reviewers: Lee Dongjin <do...@apache.org>, Ismael Juma <is...@juma.me.uk>
---
 core/src/main/scala/kafka/admin/TopicCommand.scala |  2 -
 .../scala/unit/kafka/admin/TopicCommandTest.scala  | 47 +++++++++++++++-------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/core/src/main/scala/kafka/admin/TopicCommand.scala b/core/src/main/scala/kafka/admin/TopicCommand.scala
index f7b1c87..5e7d98c 100755
--- a/core/src/main/scala/kafka/admin/TopicCommand.scala
+++ b/core/src/main/scala/kafka/admin/TopicCommand.scala
@@ -625,8 +625,6 @@ object TopicCommand extends Logging {
       }
       if (!has(listOpt) && !has(describeOpt))
         CommandLineUtils.checkRequiredArgs(parser, options, topicOpt)
-      if (has(createOpt) && !has(replicaAssignmentOpt))
-        CommandLineUtils.checkRequiredArgs(parser, options, partitionsOpt, replicationFactorOpt)
       if (has(alterOpt)) {
         CommandLineUtils.checkInvalidArgsSet(parser, options, Set(bootstrapServerOpt, configOpt), Set(alterOpt),
         Some(kafkaConfigsCanAlterTopicConfigsViaBootstrapServer))
diff --git a/core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala b/core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala
index f34e154..9586cf5 100644
--- a/core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala
+++ b/core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala
@@ -82,23 +82,23 @@ class TopicCommandTest {
   }
 
   @Test
-  def testCreateWithPartitionCountWithoutReplicationFactor(): Unit = {
-    assertCheckArgsExitCode(1,
-      new TopicCommandOptions(
-        Array("--bootstrap-server", brokerList,
-          "--create",
-          "--partitions", "2",
-          "--topic", topicName)))
+  def testCreateWithPartitionCountWithoutReplicationFactorShouldSucceed(): Unit = {
+    val opts = new TopicCommandOptions(
+      Array("--bootstrap-server", brokerList,
+        "--create",
+        "--partitions", "2",
+        "--topic", topicName))
+    opts.checkArgs()
   }
 
   @Test
-  def testCreateWithReplicationFactorWithoutPartitionCount(): Unit = {
-    assertCheckArgsExitCode(1,
-      new TopicCommandOptions(
-        Array("--bootstrap-server", brokerList,
-          "--create",
-          "--replication-factor", "3",
-          "--topic", topicName)))
+  def testCreateWithReplicationFactorWithoutPartitionCountShouldSucceed(): Unit = {
+    val opts = new TopicCommandOptions(
+      Array("--bootstrap-server", brokerList,
+        "--create",
+        "--replication-factor", "3",
+        "--topic", topicName))
+    opts.checkArgs()
   }
 
   @Test
@@ -124,6 +124,25 @@ class TopicCommandTest {
   }
 
   @Test
+  def testCreateWithoutPartitionCountAndReplicationFactorShouldSucceed(): Unit = {
+    val opts = new TopicCommandOptions(
+      Array("--bootstrap-server", brokerList,
+        "--create",
+        "--topic", topicName))
+    opts.checkArgs()
+  }
+
+  @Test
+  def testDescribeShouldSucceed(): Unit = {
+    val opts = new TopicCommandOptions(
+      Array("--bootstrap-server", brokerList,
+        "--describe",
+        "--topic", topicName))
+    opts.checkArgs()
+  }
+
+
+  @Test
   def testParseAssignmentDuplicateEntries(): Unit = {
     assertThrows(classOf[AdminCommandFailedException], () => TopicCommand.parseReplicaAssignment("5:5"))
   }