Skip to content
This repository has been archived by the owner on Mar 17, 2024. It is now read-only.

Add kafka topic blacklist #90

Merged
merged 6 commits into from
Nov 11, 2019
Merged

Add kafka topic blacklist #90

merged 6 commits into from
Nov 11, 2019

Conversation

edrzmr
Copy link
Contributor

@edrzmr edrzmr commented Oct 15, 2019

Closes #70

@lightbend-cla-validator
Copy link

Hi @drrzmr,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

seglo
seglo reviewed Nov 2, 2019
View reviewed changes
Copy link
Owner

@seglo seglo left a comment

Choose a reason for hiding this comment

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

Hi @drrzmr . Thanks a lot for the PR and apologies for the late review. There are a few other places that should be updated with the blacklist reference:

  • charts/kafka-lag-exporter/values.yaml - Add an example config where you see the topic whitelist example (commented out)
  • charts/kafka-lag-exporter/templates/030-ConfigMap.yaml - Can you add the config mapping code.
  • README.md - Add an entry to the "Kafka Cluster Connection Details" table in the "Configuration" section

@edrzmr
Copy link
Contributor Author

edrzmr commented Nov 11, 2019

@seglo I'm not a helm master, but reading ConfigMap template I see something weird, https://github.com/lightbend/kafka-lag-exporter/blob/master/charts/kafka-lag-exporter/templates/030-ConfigMap.yaml#L31-L38.
topicWhitelist are computed in terms of groupWhitelist, it's right? Or a copy/paste bug was introduced on: ad9d39f#diff-6b508968bd8a35d1d9300f1702b60c26R31-L55?

@edrzmr
Copy link
Contributor Author

edrzmr commented Nov 11, 2019

@seglo Add requested updates and 2 minor fixes. Please, lt me known If you think that fixes should not be part of this PR.

@seglo seglo self-requested a review November 11, 2019 10:47
seglo
seglo approved these changes Nov 11, 2019
View reviewed changes
Copy link
Owner

@seglo seglo left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution, and for fixing the bug with the group/topic whitelist.

@seglo seglo merged commit 57b73b3 into seglo:master Nov 11, 2019
@seglo seglo added this to the 0.5.5 milestone Nov 11, 2019
anbarasantr pushed a commit to anbarasantr/kafka-lag-exporter that referenced this pull request Nov 24, 2019
@edrzmr
Add kafka topic blacklist (seglo#90)
63f04b9 
* Add support to topic blacklist

* Add tests for topic blacklist feature

* Update configmap template with topic-blacklist

* Update documentation with topic blacklist info

* fix: Add missing "|" on README.md table

* fix: Remove typo, s/groupWhitelist/topicBlacklist/
@edrzmr edrzmr deleted the feat/topic-blacklist branch May 3, 2020 14:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@seglo seglo seglo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add blacklist of topic names
3 participants
@edrzmr @lightbend-cla-validator @seglo