Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle empty topics and partitions #3807

Merged
merged 1 commit into from
May 27, 2019
Merged

Handle empty topics and partitions #3807

merged 1 commit into from
May 27, 2019

Conversation

therve
Copy link
Contributor

@therve therve commented May 24, 2019

Using None as a default value for everything doesn't work with Agent
6. We make it work by instead passing empty dict and list when we want
all elements.

Using `None` as a default value for everything doesn't work with Agent
6. We make it work by instead passing empty dict and list when we want
all elements.
@therve therve requested a review from a team as a code owner May 24, 2019 16:38
@codecov
Copy link

codecov bot commented May 24, 2019

Codecov Report

Merging #3807 into master will decrease coverage by 5.65%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3807      +/-   ##
==========================================
- Coverage   86.57%   80.92%   -5.66%     
==========================================
  Files         730        8     -722     
  Lines       37993      540   -37453     
  Branches     4542      113    -4429     
==========================================
- Hits        32893      437   -32456     
+ Misses       3861       72    -3789     
+ Partials     1239       31    -1208

@hithwen
Copy link
Contributor

hithwen commented May 27, 2019

What happens with people who already have empty vaues on their cfg? Should this be a major?

@dabcoder
Copy link
Contributor

What happens with people who already have empty vaues on their cfg? Should this be a major?

Currently testing this, existing configurations with empty values won't return any metrics, without any errors.

@therve therve merged commit 88645d2 into master May 27, 2019
@therve therve deleted the therve/kafka-defaults branch May 27, 2019 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants