Skip to content

KAFKA-9519: Deprecating ZK for ConfigCommand#8056

Merged
cmccabe merged 3 commits intoapache:trunkfrom
skaundinya15:KAFKA-9519
Feb 8, 2020
Merged

KAFKA-9519: Deprecating ZK for ConfigCommand#8056
cmccabe merged 3 commits intoapache:trunkfrom
skaundinya15:KAFKA-9519

Conversation

@skaundinya15
Copy link
Copy Markdown
Contributor

@skaundinya15 skaundinya15 commented Feb 6, 2020

As part of KIP-555, direct zookeeper access should be deprecated in the Kafka admin tools. This PR is the first of which to do so. This PR adds warnings for deprecation of the --zookeeper option for the ConfigCommand in kafka-configs.sh.

JIRA: https://issues.apache.org/jira/browse/KAFKA-9519

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@skaundinya15 skaundinya15 requested a review from cmccabe February 6, 2020 21:59
rondagostino
rondagostino reviewed Feb 6, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@rondagostino rondagostino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor suggestion on wording for consistency with other tools.

Comment thread core/src/main/scala/kafka/admin/ConfigCommand.scala
@skaundinya15
Copy link
Copy Markdown
Contributor Author

skaundinya15 commented Feb 6, 2020

Thanks for the initial reviews @rondagostino @cmccabe! Updated the PR with the suggestions and responded to the comments as well. Ready for another round when you are

rondagostino
rondagostino reviewed Feb 7, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@rondagostino rondagostino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chia7712
Copy link
Copy Markdown
Member

chia7712 commented Feb 7, 2020

The zkConnectOpt is deprecated now so should we define bootstrapServerOpt with .requiredUnless("bootstrap-server") ? And then the manual check can be moved as well.

      if (!options.has(bootstrapServerOpt) && !options.has(zkConnectOpt))
        throw new IllegalArgumentException("One of the required --bootstrap-server or --zookeeper arguments must be specified")

@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Feb 7, 2020

ok to test

@skaundinya15
Copy link
Copy Markdown
Contributor Author

skaundinya15 commented Feb 8, 2020 •
edited
Loading

@cmccabe Seems as if there were some test failures, but not sure if they are related to my change - should we do a retest or is there something I have to change?

@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Feb 8, 2020

The zkConnectOpt is deprecated now so should we define bootstrapServerOpt with .requiredUnless("bootstrap-server") ? And then the manual check can be moved as well.

Let's think about that for a follow-up PR.

@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Feb 8, 2020

LGTM

@cmccabe cmccabe merged commit be4a6dd into apache:trunk Feb 8, 2020
@skaundinya15 skaundinya15 deleted the KAFKA-9519 branch February 8, 2020 00:08
asfgit pushed a commit that referenced this pull request Feb 8, 2020
@cmccabe
KAFKA-9519: Deprecate the --zookeeper flag in ConfigCommand (#8056)
be22901 
Reviewers: Colin P. McCabe <cmccabe@apache.org>, Ron Dagostino <rndgstn@gmail.com>
(cherry picked from commit be4a6dd)
stanislavkozlovski pushed a commit to stanislavkozlovski/kafka that referenced this pull request Feb 18, 2020
@stanislavkozlovski
KAFKA-9519: Deprecate the --zookeeper flag in ConfigCommand (apache#8056
6a7151c 
)

Reviewers: Colin P. McCabe <cmccabe@apache.org>, Ron Dagostino <rndgstn@gmail.com>
(cherry picked from commit be4a6dd)
@showuon showuon mentioned this pull request Apr 17, 2020
Merged
3 tasks
ijuma added a commit to ijuma/kafka that referenced this pull request Apr 28, 2020
@ijuma
Merge remote-tracking branch 'apache-github/trunk' into skip-to-struc…
186aad4 
…t-for-generated-requests

* apache-github/trunk: (410 commits)
  KAFKA-8843: KIP-515: Zookeeper TLS support
  MINOR: Add missing quote for malformed line content (apache#8070)
  MINOR: Simplify KafkaProducerTest (apache#8044)
  KAFKA-9507; AdminClient should check for missing committed offsets (apache#8057)
  KAFKA-9519: Deprecate the --zookeeper flag in ConfigCommand (apache#8056)
  KAFKA-9509; Fixing flakiness of MirrorConnectorsIntegrationTest.testReplication (apache#8048)
  HOTFIX: Fix two test failures in JDK11 (apache#8063)
  DOCS - clarify transactionalID and idempotent behavior (apache#7821)
  MINOR: further InternalTopologyBuilder cleanup  (apache#8046)
  MINOR: Add timer for update limit offsets (apache#8047)
  HOTFIX: Fix spotsbug failure in Kafka examples (apache#8051)
  KAFKA-9447: Add new customized EOS model example (apache#8031)
  KAFKA-8164: Add support for retrying failed (apache#8019)
  HOTFIX: checkstyle for newly added unit test
  KAFKA-9261; Client should handle unavailable leader metadata (apache#7770)
  MINOR: Fix typos introduced in KIP-559 (apache#8042)
  MINOR: Fixing null handilg in ValueAndTimestampSerializer (apache#7679)
  KAFKA-9113: Clean up task management and state management (apache#7997)
  MINOR: fix checkstyle issue in ConsumerConfig.java (apache#8038)
  KAFKA-9491; Increment high watermark after full log truncation (apache#8037)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rondagostino rondagostino rondagostino left review comments

@cmccabe cmccabe Awaiting requested review from cmccabe

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@skaundinya15 @chia7712 @cmccabe @rondagostino