Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-10057 optimize class ConfigCommand method alterConfig parameters #8742

Open
wants to merge 2 commits into
base: 1.1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/src/main/scala/kafka/admin/ConfigCommand.scala
Expand Up @@ -97,15 +97,15 @@ object ConfigCommand extends Config {
val adminZkClient = new AdminZkClient(zkClient)
try {
if (opts.options.has(opts.alterOpt))
alterConfig(zkClient, opts, adminZkClient)
alterConfig(opts, adminZkClient)
else if (opts.options.has(opts.describeOpt))
describeConfig(zkClient, opts, adminZkClient)
} finally {
zkClient.close()
}
}

private[admin] def alterConfig(zkClient: KafkaZkClient, opts: ConfigCommandOptions, adminZkClient: AdminZkClient) {
private[admin] def alterConfig(opts: ConfigCommandOptions, adminZkClient: AdminZkClient) {
val configsToBeAdded = parseConfigsToBeAdded(opts)
val configsToBeDeleted = parseConfigsToBeDeleted(opts)
val entity = parseEntity(opts)
Expand Down
30 changes: 15 additions & 15 deletions core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala
Expand Up @@ -143,7 +143,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
def shouldFailIfUnrecognisedEntityType(): Unit = {
val createOpts = new ConfigCommandOptions(Array("--zookeeper", zkConnect,
"--entity-name", "client", "--entity-type", "not-recognised", "--alter", "--add-config", "a=b,c=d"))
ConfigCommand.alterConfig(null, createOpts, new DummyAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new DummyAdminZkClient(zkClient))
}

@Test
Expand All @@ -162,7 +162,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
}
}

ConfigCommand.alterConfig(null, createOpts, new TestAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new TestAdminZkClient(zkClient))
}

@Test
Expand All @@ -181,7 +181,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
}
}

ConfigCommand.alterConfig(null, createOpts, new TestAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new TestAdminZkClient(zkClient))
}

@Test
Expand All @@ -200,7 +200,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
}
}

ConfigCommand.alterConfig(null, alterOpts, new TestAdminZkClient(zkClient))
ConfigCommand.alterConfig(alterOpts, new TestAdminZkClient(zkClient))
}

@Test
Expand Down Expand Up @@ -279,7 +279,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
override def changeTopicConfig(topic: String, configs: Properties): Unit = {}
}

ConfigCommand.alterConfig(null, createOpts, new TestAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new TestAdminZkClient(zkClient))
}

@Test (expected = classOf[IllegalArgumentException])
Expand All @@ -289,7 +289,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
"--entity-type", "brokers",
"--alter",
"--add-config", "leader.replication.throttled.rate=10"))
ConfigCommand.alterConfig(null, createOpts, new DummyAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new DummyAdminZkClient(zkClient))
}

@Test (expected = classOf[IllegalArgumentException])
Expand All @@ -299,7 +299,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
"--entity-type", "brokers",
"--alter",
"--add-config", "message.max.size=100000"))
ConfigCommand.alterConfig(null, createOpts, new DummyAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new DummyAdminZkClient(zkClient))
}

@Test (expected = classOf[IllegalArgumentException])
Expand All @@ -309,7 +309,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
"--entity-type", "brokers",
"--alter",
"--add-config", "a="))
ConfigCommand.alterConfig(null, createOpts, new DummyAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new DummyAdminZkClient(zkClient))
}

@Test (expected = classOf[IllegalArgumentException])
Expand All @@ -319,7 +319,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
"--entity-type", "brokers",
"--alter",
"--add-config", "a=[b,c,d=e"))
ConfigCommand.alterConfig(null, createOpts, new DummyAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new DummyAdminZkClient(zkClient))
}

@Test (expected = classOf[InvalidConfigException])
Expand All @@ -329,7 +329,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
"--entity-type", "topics",
"--alter",
"--delete-config", "missing_config1, missing_config2"))
ConfigCommand.alterConfig(null, createOpts, new DummyAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new DummyAdminZkClient(zkClient))
}

@Test
Expand All @@ -355,7 +355,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
}
}

ConfigCommand.alterConfig(null, createOpts, new TestAdminZkClient(zkClient))
ConfigCommand.alterConfig(createOpts, new TestAdminZkClient(zkClient))
}

@Test
Expand Down Expand Up @@ -393,14 +393,14 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
}
}
val optsA = createOpts("userA", "SCRAM-SHA-256=[iterations=8192,password=abc, def]")
ConfigCommand.alterConfig(null, optsA, CredentialChange("userA", Set("SCRAM-SHA-256"), 8192))
ConfigCommand.alterConfig(optsA, CredentialChange("userA", Set("SCRAM-SHA-256"), 8192))
val optsB = createOpts("userB", "SCRAM-SHA-256=[iterations=4096,password=abc, def],SCRAM-SHA-512=[password=1234=abc]")
ConfigCommand.alterConfig(null, optsB, CredentialChange("userB", Set("SCRAM-SHA-256", "SCRAM-SHA-512"), 4096))
ConfigCommand.alterConfig(optsB, CredentialChange("userB", Set("SCRAM-SHA-256", "SCRAM-SHA-512"), 4096))

val del256 = deleteOpts("userB", "SCRAM-SHA-256")
ConfigCommand.alterConfig(null, del256, CredentialChange("userB", Set("SCRAM-SHA-512"), 4096))
ConfigCommand.alterConfig(del256, CredentialChange("userB", Set("SCRAM-SHA-512"), 4096))
val del512 = deleteOpts("userB", "SCRAM-SHA-512")
ConfigCommand.alterConfig(null, del512, CredentialChange("userB", Set(), 4096))
ConfigCommand.alterConfig(del512, CredentialChange("userB", Set(), 4096))
}

@Test
Expand Down