KAFKA-9945: TopicCommand should support --if-exists and --if-not-exists when --bootstrap-server is used #8737
Conversation
…ts when --bootstrap-server is used - Fixed command line arg checking - Added unit test cases
core/src/test/scala/unit/kafka/admin/TopicCommandWithAdminClientTest.scala
Outdated
Show resolved
Hide resolved
- Avoid unnecessary RPC to server, if effective topic list is empty - Efficient use of AdminClient#listPartitionReassignments() - Fix if check style
|
https://issues.apache.org/jira/browse/KAFKA-10071 Opened this to track the follow ups.. @cmccabe let me know if you have further comments on the PR. |
| topicDesc.printDescription() | ||
| ensureTopicExists(topics, opts.topic, !opts.ifExists) | ||
|
|
||
| if (topics.nonEmpty) { |
There was a problem hiding this comment.
It's not necessary to check this. AdminClient handles being passed empty lists or sets of topics.
There was a problem hiding this comment.
val liveBrokers = adminClient.describeCluster().nodes().get().asScala.map(_.id())
This line still makes an RPC call.
|
Thanks for working on this, @vinothchandar. I think it's almost ready to go. I left two comments. |
There was a problem hiding this comment.
I have removed the topics empty check from places, where no extra RPC call would be made. And also changed the tests to not catch-fail, given Kafka is not using the pattern widely.. (personally, I still see value in failing the test explicitly.. Thats why stuff like [assertDoesNotThrow](http://junit.org/junit5/docs/5.3.0/api/org/junit/jupiter/api/Assertions.html#assertDoesNotThrow(org.junit.ju…) was introduced. or alternatively we could have expected a none exception)
Hope this addresses all the comments..
|
LGTM |
|
Thanks @vinothchandar ! |
* apache-github/2.6: (32 commits) KAFKA-10083: fix failed testReassignmentWithRandomSubscriptionsAndChanges tests (apache#8786) KAFKA-9945: TopicCommand should support --if-exists and --if-not-exists when --bootstrap-server is used (apache#8737) KAFKA-9320: Enable TLSv1.3 by default (KIP-573) (apache#8695) KAFKA-10082: Fix the failed testMultiConsumerStickyAssignment (apache#8777) MINOR: Remove unused variable to fix spotBugs failure (apache#8779) MINOR: ChangelogReader should poll for duration 0 for standby restore (apache#8773) KAFKA-10030: Allow fetching a key from a single partition (apache#8706) Kafka-10064 Add documentation for KIP-571 (apache#8760) MINOR: Code cleanup and assertion message fixes in Connect integration tests (apache#8750) KAFKA-9987: optimize sticky assignment algorithm for same-subscription case (apache#8668) KAFKA-9392; Clarify deleteAcls javadoc and add test for create/delete timing (apache#7956) KAFKA-10074: Improve performance of `matchingAcls` (apache#8769) KAFKA-9494; Include additional metadata information in DescribeConfig response (KIP-569) (apache#8723) KAFKA-10056; Ensure consumer metadata contains new topics on subscription change (apache#8739) KAFKA-10029; Don't update completedReceives when channels are closed to avoid ConcurrentModificationException (apache#8705) KAFKA-10061; Fix flaky `ReassignPartitionsIntegrationTest.testCancellation` (apache#8749) KAFKA-9130; KIP-518 Allow listing consumer groups per state (apache#8238) KAFKA-9501: convert between active and standby without closing stores (apache#8248) MINOR: Relax Percentiles test (apache#8748) MINOR: regression test for task assignor config (apache#8743) ...
Committer Checklist (excluded from commit message)