Skip to content

KAFKA-6627: Prevent config default values overriding ones specified through --producer-property on command line.#6084

Merged
harshach merged 2 commits intoapache:trunkfrom
likan999:dev
Jan 11, 2019
Merged

KAFKA-6627: Prevent config default values overriding ones specified through --producer-property on command line.#6084
harshach merged 2 commits intoapache:trunkfrom
likan999:dev

Conversation

@likan999
Copy link
Copy Markdown
Contributor

@likan999 likan999 commented Jan 3, 2019

In ConsoleProducer, extraProducerProps (options specified in --producer-property) are applied first, then overriden (thus ignored) by the values returned by command line parser (default values are used for ones not specified from command line). This patch fixes it so that it doesn't override the existing value set by --producer-property if it is not explicitly specified.

The contribution is my original work and I license the work to the project under the project's open source license.

Signed-off-by: Kan Li likan@uber.com

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Testing strategy:
For producer, print out acks value in KafkaProducer.configureAcks, start producer using --producer-property aks=all, then observe the difference with and without the patch.
For consumer, print out isolationLevel in KafkaConsumer, start consumer using --consumer-property isolation.level=read_committed, then observe the difference with and without the patch.

Committer Checklist (excluded from commit message)

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

@harshach
Copy link
Copy Markdown

harshach commented Jan 4, 2019

@likan999 looks good. Can you also check if we should apply these changes to console-consumer as well?

Kan Li added 2 commits January 4, 2019 12:59
…hrough --producer-property on command line.

In Console{Producer,Consumer}, extraProducerProps (options specified in
--producer-property) is applied first, then overriden unconditionally,
even if the value is not specified explicitly (and default value is
used). This patch fixes it so that it doesn't override the existing
value set by --producer-property if it is not explicitly specified.

The contribution is my original work and I license the work to the
project under the project's open source license.

Signed-off-by: Kan Li <likan@uber.com>
@likan999
Copy link
Copy Markdown
Contributor Author

likan999 commented Jan 4, 2019

Moved the method into CommandLineUtils so that it doesn't introduce a new file.

Also tested it against consumer and without the diff --consumer-property isolation.level=read_committed doesn't work, and works with the diff.

Consumer is kind of different to producer as the command line flags don't have many overlap with the ones specified through --consumer-property. isolation.level is one of a few overlaps, and other overlaps (group_id, e.g.) are handled specifically.

@likan999
Copy link
Copy Markdown
Contributor Author

likan999 commented Jan 4, 2019

@harshach could you TAL? Thanks.

Comment thread core/src/main/scala/kafka/utils/ConfigUtils.scala Outdated
@harshach harshach merged commit 694da1a into apache:trunk Jan 11, 2019
hachikuji added a commit that referenced this pull request Jan 11, 2019
This was broken by #6084. The syntax works with Scala 2.12, but not 2.11.

Reviewers: Colin Patrick McCabe <colin@cmccabe.xyz>
@hachikuji
Copy link
Copy Markdown
Contributor

@likan999 @harshach Note the needed fix above. It's important to check build failures. Thanks!

pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…hrough --producer-property on command line. (apache#6084)

* KAFKA-6627: Prevent config default values overriding ones specified through --producer-property on command line.

In Console{Producer,Consumer}, extraProducerProps (options specified in
--producer-property) is applied first, then overriden unconditionally,
even if the value is not specified explicitly (and default value is
used). This patch fixes it so that it doesn't override the existing
value set by --producer-property if it is not explicitly specified.

The contribution is my original work and I license the work to the
project under the project's open source license.

Reviewers: Sriharsha Chintalapani <sriharsha@apache.org>
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
This was broken by apache#6084. The syntax works with Scala 2.12, but not 2.11.

Reviewers: Colin Patrick McCabe <colin@cmccabe.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants