You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "nizhikov (via GitHub)" <gi...@apache.org> on 2024/05/13 12:20:21 UTC
[PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
nizhikov opened a new pull request, #15930:
URL: https://github.com/apache/kafka/pull/15930
This is third part of https://github.com/apache/kafka/pull/15839 refactoring.
PR intention - split big PR in parts to simplify review.
PR contains tests of ConfigCommandTest rewritten in java.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on PR #15930:
URL: https://github.com/apache/kafka/pull/15930#issuecomment-2112395461
CI looks Ok for me
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on PR #15930:
URL: https://github.com/apache/kafka/pull/15930#issuecomment-2107875375
Added tests passed. PR ready for review.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #15930:
URL: https://github.com/apache/kafka/pull/15930#issuecomment-2114777884
@nizhikov nice patch!
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #15930:
URL: https://github.com/apache/kafka/pull/15930#discussion_r1601032507
##########
core/src/test/java/kafka/admin/ConfigCommandUnitTest.java:
##########
@@ -878,6 +886,486 @@ public void shouldNotDescribeUserScramCredentialsWithEntityDefaultUsingBootstrap
verifyUserScramCredentialsNotDescribed(defaultUserOpt);
}
+ @Test
+ public void shouldAddTopicConfigUsingZookeeper() {
+ ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(toArray("--zookeeper", ZK_CONNECT,
+ "--entity-name", "my-topic",
+ "--entity-type", "topics",
+ "--alter",
+ "--add-config", "a=b,c=d"));
+
+ KafkaZkClient zkClient = mock(KafkaZkClient.class);
+ when(zkClient.getEntityConfigs(anyString(), anyString())).thenReturn(new Properties());
+
+ ConfigCommand.alterConfigWithZk(null, createOpts, new AdminZkClient(zkClient, scala.None$.empty()) {
+ @Override
+ public void changeTopicConfig(String topic, Properties configChange) {
+ assertEquals("my-topic", topic);
+ assertEquals("b", configChange.get("a"));
+ assertEquals("d", configChange.get("c"));
+ }
+ });
+ }
+
+ @Test
Review Comment:
How about using `ValueSource`?
```java
@Test
@ValueSource(booleans = {true, false})
public void shouldAlterTopicConfig(boolean file) {
```
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on PR #15930:
URL: https://github.com/apache/kafka/pull/15930#issuecomment-2114875511
@chia7712 Thanks for the review and merge.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #15930:
URL: https://github.com/apache/kafka/pull/15930
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #15930:
URL: https://github.com/apache/kafka/pull/15930#discussion_r1601121898
##########
core/src/test/java/kafka/admin/ConfigCommandUnitTest.java:
##########
@@ -878,6 +886,486 @@ public void shouldNotDescribeUserScramCredentialsWithEntityDefaultUsingBootstrap
verifyUserScramCredentialsNotDescribed(defaultUserOpt);
}
+ @Test
+ public void shouldAddTopicConfigUsingZookeeper() {
+ ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(toArray("--zookeeper", ZK_CONNECT,
+ "--entity-name", "my-topic",
+ "--entity-type", "topics",
+ "--alter",
+ "--add-config", "a=b,c=d"));
+
+ KafkaZkClient zkClient = mock(KafkaZkClient.class);
+ when(zkClient.getEntityConfigs(anyString(), anyString())).thenReturn(new Properties());
+
+ ConfigCommand.alterConfigWithZk(null, createOpts, new AdminZkClient(zkClient, scala.None$.empty()) {
+ @Override
+ public void changeTopicConfig(String topic, Properties configChange) {
+ assertEquals("my-topic", topic);
+ assertEquals("b", configChange.get("a"));
+ assertEquals("d", configChange.get("c"));
+ }
+ });
+ }
+
+ @Test
Review Comment:
Done.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14588 [3/N] ConfigCommandTest rewritten in java [kafka]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on PR #15930:
URL: https://github.com/apache/kafka/pull/15930#issuecomment-2107461043
Hello @chia7712
I opened PR with third part of `ConfigCommandTest` refactoring.
Can you, please, take a look?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org