Skip to content

KAFKA-10168: fix StreamsConfig parameter name variable#8865

Merged
mjsax merged 3 commits intoapache:trunkfrom
mjsax:minor-fix-stream-config-topology-optimization
Jun 20, 2020
Merged

KAFKA-10168: fix StreamsConfig parameter name variable#8865
mjsax merged 3 commits intoapache:trunkfrom
mjsax:minor-fix-stream-config-topology-optimization

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Jun 13, 2020 •
edited
Loading

Rename TOPOLOGY_OPTIMIZATION to TOPOLOGY_OPTIMIZATION_CONFIG.

All variables in StreamConfig end with _CONFIG. We should align the variable name accordingly. This is technically a public API change. Happy to do a quick KIP for it, if we think we need one.

Cf. https://cwiki.apache.org/confluence/display/KAFKA/KIP-626%3A+Rename+StreamsConfig+config+variable+name

\cc @guozhangwang @vvcephei

@mjsax mjsax added the streams label Jun 13, 2020
abbccdda
abbccdda reviewed Jun 15, 2020
View reviewed changes
Copy link
Copy Markdown

@abbccdda abbccdda left a comment

Choose a reason for hiding this comment

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

I'm inclined to get a small KIP for this. Besides, could we have some style check tool to enforce this norm for later configuration addition as well?

@vvcephei
Copy link
Copy Markdown
Contributor

vvcephei commented Jun 15, 2020

Looks like a compile warning:

00:07:57.788 > Task :streams:compileJava
00:07:57.788 /home/jenkins/jenkins-slave/workspace/kafka-pr-jdk11-scala2.13/streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java:530: warning: [dep-ann] deprecated item is not annotated with @Deprecated
00:07:57.788     public static final String TOPOLOGY_OPTIMIZATION = "topology.optimization";
00:07:57.789                                ^
00:07:57.789 error: warnings found and -Werror specified

I'm +1 on this change, and I think it would be fine to just amend KIP-295.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Jun 16, 2020

Not sure if changing KIP-295 would be a good call. We made a mistake and can be honest about it. I'll do a quick KIP.

@mjsax mjsax force-pushed the minor-fix-stream-config-topology-optimization branch from e4204d7 to 1c7292d Compare June 16, 2020 02:24
@mjsax mjsax changed the title MINOR: fix StreamsConfig parameter name variable KAFKA-10168: fix StreamsConfig parameter name variable Jun 16, 2020
@mjsax mjsax added the kip Requires or implements a KIP label Jun 19, 2020
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Jun 19, 2020

KIP was accepted.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Jun 19, 2020

Retest this please.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Jun 19, 2020

@vvcephei As you are +1, I am going to merge after Jenkins passed.

@mjsax mjsax merged commit 712cc5d into apache:trunk Jun 20, 2020
@mjsax mjsax deleted the minor-fix-stream-config-topology-optimization branch June 20, 2020 00:41
Kvicii pushed a commit to Kvicii/kafka that referenced this pull request Jun 21, 2020
Merge branch 'trunk' of github.com:apache/kafka into trunk-read
137d186 
* 'trunk' of github.com:apache/kafka:
  KAFKA-10168: fix StreamsConfig parameter name variable (apache#8865)
  MINOR: code cleanup for inconsistent naming (apache#8871)
  KAFKA-10138: Prefer --bootstrap-server for reassign_partitions command in ducktape tests (apache#8898)
  KAFKA-10185: Restoration info logging (apache#8896)
  KAFKA-9891: add integration tests for EOS and StandbyTask (apache#8890)
  MINOR: Reduce build time by gating test coverage plugins behind a flag (apache#8899)
  KAFKA-10141; Add more detail to log segment delete messages (apache#8850)
  KAFKA-10113; Specify fetch offsets correctly in `LogTruncationException` (apache#8822)
  KAFKA-10167: use the admin client to read end-offset (apache#8876)
  MINOR: Upgrade ducktape to 0.7.8 (apache#8879)
  KAFKA-10123; Fix incorrect value for AWAIT_RESET#hasPosition (apache#8841)
  KAFKA-9896: fix flaky StandbyTaskEOSIntegrationTest (apache#8883)
  MINOR: clean up unused checkstyle suppressions for Streams (apache#8861)
  MINOR: reuse toConfigObject(Map) to generate Config (apache#8889)
  MINOR: Upgrade jetty to 9.4.27.v20200227 and jersey to 2.31 (apache#8859)
  MINOR: Fix flaky HighAvailabilityTaskAssignorIntegrationTest (apache#8884)
  KAFKA-10147 MockAdminClient#describeConfigs(Collection<ConfigResource>) is unable to handle broker resource (apache#8853)
  KAFKA-10165: Remove Percentiles from e2e metrics (apache#8882)

# Conflicts:
#	core/src/main/scala/kafka/log/Log.scala
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abbccdda abbccdda abbccdda left review comments

Assignees

No one assigned

Labels

kip Requires or implements a KIP streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mjsax @vvcephei @abbccdda