-
Notifications
You must be signed in to change notification settings - Fork 901
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
Added logging parameter for Avro(Consumer|Producer). #699
Conversation
It looks like @venthur hasn't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here. Once you've signed reply with Appreciation of efforts, clabot |
[clabot:check] |
@confluentinc It looks like @venthur just signed our Contributor License Agreement. 👍 Always at your service, clabot |
Any updates on this one? |
@rnpridgeon suggests a workaround here. That being said, it looks like this PR might be ready for review @rnpridgeon? |
while f.cnt == 0: | ||
kc.poll(timeout=0.5) | ||
|
||
print('%s: %d log messages seen' % (f.name, f.cnt)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should add a check that the logger actually saw some logs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't that implied by the f.cnt > 0
after the while
loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, you're right!
The patch introduces **kwargs and just passes them to the underlying super calls. Tests are included. Closes: confluentinc#698
406ab96
to
cfb9794
Compare
Thank you for this! |
I've rebased this branch against current master. I think the failing tests are unrelated to this merge request. |
The patch introduces **kwargs and just passes them to the underlying
super calls.
Tests are included.
Closes: #698