Skip to content

KAFKA-8426; Fix for keeping the ConfigProvider configs consistent with KIP-297#6750

Merged
rajinisivaram merged 3 commits into
apache:trunkfrom
tadsul:KIP421_bug
Jun 3, 2019
Merged

KAFKA-8426; Fix for keeping the ConfigProvider configs consistent with KIP-297#6750
rajinisivaram merged 3 commits into
apache:trunkfrom
tadsul:KIP421_bug

Conversation

@tadsul

@tadsul tadsul commented May 17, 2019

Copy link
Copy Markdown
Contributor

According to KIP-297 a parameter is passed to ConfigProvider with syntax "config.providers.{name}.param.{param-name}". Currently AbstractConfig allows parameters of the format "config.providers.{name}.{param-name}". With this fix AbstractConfig will be consistent with KIP-297 syntax.

According to KIP-297 a parameter is passed to ConfigProvider with syntax "config.providers.{name}.param.{param-name}". Currently AbstractConfig allows parameters of the format "config.providers.{name}.{param-name}". With this fix AbstractCongig will be consistent with KIP-297 syntax.

@rayokota rayokota left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java Outdated
@rajinisivaram

Copy link
Copy Markdown
Contributor

@tadsul Can we create a JIRA and set 2.3.0 as fix version to make sure it gets merged into 2.3.0 before thre release? Also include the JIRA in the PR title.

@rajinisivaram rajinisivaram changed the title Fix for keeping the ConfigProvider configs consistent with KIP-297 KAFKA-8426; Fix for keeping the ConfigProvider configs consistent with KIP-297 May 29, 2019

@rajinisivaram rajinisivaram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tadsul Thanks for the updates, left a few minor comments.

Comment thread clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java Outdated
@tadsul

tadsul commented May 29, 2019

Copy link
Copy Markdown
Contributor Author

rerun the test

@rajinisivaram rajinisivaram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tadsul Thanks for the updates, LGTM. Merging to trunk and 2.3.

@rajinisivaram
rajinisivaram merged commit b042b36 into apache:trunk Jun 3, 2019
rajinisivaram pushed a commit that referenced this pull request Jun 3, 2019
…h KIP-297 (#6750)

According to KIP-297 a parameter is passed to ConfigProvider with syntax "config.providers.{name}.param.{param-name}". Currently AbstractConfig allows parameters of the format "config.providers.{name}.{param-name}". With this fix AbstractConfig will be consistent with KIP-297 syntax.

Reviewers: Robert Yokota <rayokota@gmail.com>, Rajini Sivaram <rajinisivaram@googlemail.com>
omkreddy added a commit to confluentinc/kafka that referenced this pull request Jun 4, 2019
* AK_REPO/2.3:
  KAFKA-8155: Add 2.2.0 release to system tests (apache#6597)
  KAFKA-8404: Add HttpHeader to RestClient HTTP Request and Connector REST API (apache#6791)
  KAFKA-8473: Adjust Connect system tests for incremental cooperative rebalancing (apache#6872)
  KAFKA-8475: Temporarily restore SslFactory.sslContext() helper
  KAFKA-8449: Restart tasks on reconfiguration under incremental cooperative rebalancing (apache#6850)
  KAFKA-8426; Fix for keeping the ConfigProvider configs consistent with KIP-297 (apache#6750)
  KAFKA-8425: Fix for correctly handling immutable maps (KIP-421 bug) (apache#6795)
  MINOR: Reordering the props modification with configs construction
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…h KIP-297 (apache#6750)

According to KIP-297 a parameter is passed to ConfigProvider with syntax "config.providers.{name}.param.{param-name}". Currently AbstractConfig allows parameters of the format "config.providers.{name}.{param-name}". With this fix AbstractConfig will be consistent with KIP-297 syntax.

Reviewers: Robert Yokota <rayokota@gmail.com>, Rajini Sivaram <rajinisivaram@googlemail.com>
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