Skip to content

MINOR: Finished exposing the broker config#2350

Closed
enothereska wants to merge 8 commits intoapache:trunkfrom
enothereska:minor-broker-level-config
Closed

MINOR: Finished exposing the broker config#2350
enothereska wants to merge 8 commits intoapache:trunkfrom
enothereska:minor-broker-level-config

Conversation

@enothereska
Copy link
Copy Markdown
Contributor

This was left over from KIP-104. Thanks to @ijuma for pointing out.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/784/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/786/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/784/
Test PASSed (JDK 7 and Scala 2.10).

@enothereska
Copy link
Copy Markdown
Contributor Author

@ijuma have a look when you can, thanks.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 13, 2017

Thanks @enothereska, can we add a simple unit test? LGTM otherwise.

@enothereska
Copy link
Copy Markdown
Contributor Author

@ijuma I added a simple check, still thinking of what more to add at this level since there is testing at the sensor level.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/843/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/845/
Test FAILed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/848/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/850/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/848/
Test FAILed (JDK 7 and Scala 2.10).

@enothereska
Copy link
Copy Markdown
Contributor Author

Known failure being worked on: org.apache.kafka.streams.integration.QueryableStateIntegrationTest.shouldNotMakeStoreAvailableUntilAllStoresAvailable[1]

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/843/
Test FAILed (JDK 7 and Scala 2.10).

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 16, 2017

@enothereska, the test I had in mind is to verify that MetricConfig had the right recording level if the broker config is set. That is what has changed in this PR. Before this PR, the information was not propagated to that class. I agree that the sensor tests are the right place to verify that the recording level is used in the right way.

@enothereska
Copy link
Copy Markdown
Contributor Author

Thanks @ijuma I added a test as you suggested.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/930/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/928/
Test FAILed (JDK 7 and Scala 2.10).

@enothereska
Copy link
Copy Markdown
Contributor Author

enothereska commented Jan 17, 2017

Known issue pure virtual method called not related to PR.
Known streams issues not related to PR:
org.apache.kafka.streams.integration.KStreamRepartitionJoinTest.shouldCorrectlyRepartitionOnJoinOperations[0]
org.apache.kafka.streams.integration.KStreamRepartitionJoinTest.shouldCorrectlyRepartitionOnJoinOperations[1]
org.apache.kafka.streams.integration.QueryableStateIntegrationTest.queryOnRebalance[0]

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/928/
Test FAILed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 18, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/980/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 18, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/978/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 18, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/978/
Test FAILed (JDK 8 and Scala 2.12).

@enothereska
Copy link
Copy Markdown
Contributor Author

Needs to go to 0.10.2 as well please.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 23, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1107/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 23, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1105/
Test FAILed (JDK 8 and Scala 2.12).

Copy link
Copy Markdown
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, LGTM. Merging to trunk and 0.10.2 (as it's a follow-up to KIP-105).

asfgit pushed a commit that referenced this pull request Jan 23, 2017
This is a KIP-104/105 follow-up. Thanks to ijuma for pointing out.

Author: Eno Thereska <eno.thereska@gmail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes #2350 from enothereska/minor-broker-level-config

(cherry picked from commit 1eb1e2f)
Signed-off-by: Ismael Juma <ismael@juma.me.uk>
@asfgit asfgit closed this in 1eb1e2f Jan 23, 2017
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 23, 2017

For the record, I did some minor style and test clean-ups before merging.

@enothereska
Copy link
Copy Markdown
Contributor Author

Thanks @ijuma

@enothereska enothereska deleted the minor-broker-level-config branch January 23, 2017 15:21
@asfbot
Copy link
Copy Markdown

asfbot commented Jan 23, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1105/
Test PASSed (JDK 7 and Scala 2.10).

soenkeliebau pushed a commit to soenkeliebau/kafka that referenced this pull request Feb 7, 2017
This is a KIP-104/105 follow-up. Thanks to ijuma for pointing out.

Author: Eno Thereska <eno.thereska@gmail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes apache#2350 from enothereska/minor-broker-level-config
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