KAFKA-13396: allow create topic without partition/replicaFactor#11429
Merged
Conversation
showuon
commented
Oct 24, 2021
| if (!has(listOpt) && !has(describeOpt)) | ||
| CommandLineUtils.checkRequiredArgs(parser, options, topicOpt) | ||
| if (has(createOpt) && !has(replicaAssignmentOpt)) | ||
| CommandLineUtils.checkRequiredArgs(parser, options, partitionsOpt, replicationFactorOpt) |
Member
Author
There was a problem hiding this comment.
remove them since it is only checked for zookeeper option case before.
showuon
commented
Oct 24, 2021
Comment on lines
+126
to
+142
| @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() | ||
| } |
Member
Author
There was a problem hiding this comment.
Make sure the 2 command lines from quick start will pass successfully.
Contributor
There was a problem hiding this comment.
I love this case-by-case approach + it also works like a charm. 😃
Member
Author
|
@ijuma , please help check this PR when available. Thank you. |
Member
Author
|
@ijuma , please help check it when available. This is a regression issue, so I think we should put it v3.1.0. Thank you. |
Member
|
@showuon we should have this in 3.0.1 too, right? |
Member
Author
|
Yes, cherry-pick back to v3.0.1, too, please. Thank you! |
ijuma
pushed a commit
that referenced
this pull request
Nov 4, 2021
[KIP-464](https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic) (PR: #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 #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 <dongjin@apache.org>, Ismael Juma <ismael@juma.me.uk>
Member
|
@ijuma Should we pick this one to the 3.1 branch as well? |
Member
|
@dajac Makes sense, pushed to trunk, 3.1 and 3.0. |
ijuma
pushed a commit
that referenced
this pull request
Nov 4, 2021
[KIP-464](https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic) (PR: #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 #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 <dongjin@apache.org>, Ismael Juma <ismael@juma.me.uk>
stanislavkozlovski
added a commit
to stanislavkozlovski/kafka
that referenced
this pull request
Nov 11, 2021
…ntegration-11-nov * ak/trunk: (15 commits) KAFKA-13429: ignore bin on new modules (apache#11415) KAFKA-12648: introduce TopologyConfig and TaskConfig for topology-level overrides (apache#11272) KAFKA-12487: Add support for cooperative consumer protocol with sink connectors (apache#10563) MINOR: Log client disconnect events at INFO level (apache#11449) MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order (apache#11403) KAFKA-13417; Ensure dynamic reconfigurations set old config properly (apache#11448) MINOR: Adding a constant to denote UNKNOWN leader in LeaderAndEpoch (apache#11477) KAFKA-10543: Convert KTable joins to new PAPI (apache#11412) KAFKA-12226: Commit source task offsets without blocking on batch delivery (apache#11323) KAFKA-13396: Allow create topic without partition/replicaFactor (apache#11429) ...
3 tasks
xdgrulez
pushed a commit
to xdgrulez/kafka
that referenced
this pull request
Dec 22, 2021
…he#11429) [KIP-464](https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic) (PR: apache#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 apache#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 <dongjin@apache.org>, Ismael Juma <ismael@juma.me.uk>
lmr3796
pushed a commit
to lmr3796/kafka
that referenced
this pull request
Jun 2, 2022
…he#11429) [KIP-464](https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic) (PR: apache#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 apache#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 <dongjin@apache.org>, Ismael Juma <ismael@juma.me.uk>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In https://github.com/apache/kafka/pull/10457/files#r635196307, the reviewer asked if there's a bug that we only check for zookeeper case, I didn't do a well check and directly confirm it. Now, after checking the change history, I found the change is because the KIP-464 (PR: #6728) added default values for partition count and replica factor for createTopic in AdminClient. So it didn't apply for zookeeper option case.
Fix this bug 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.
Committer Checklist (excluded from commit message)