-
Notifications
You must be signed in to change notification settings - Fork 181
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
Add librdkafka based consumer #367
Conversation
Signed-off-by: Tamas Kornai <[email protected]>
Signed-off-by: Tamas Kornai <[email protected]>
Signed-off-by: Tamas Kornai <[email protected]>
8d6a9d0
to
b225673
Compare
I have just commented on old PR. Check my review: #366 (review) |
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.
Random comments.
Signed-off-by: Tamas Kornai <[email protected]>
Signed-off-by: Tamas Kornai <[email protected]>
Thanks @repeatedly and @robinp-tw for the comments, I tried to address all of them. |
Signed-off-by: Tamas Kornai <[email protected]>
Hi @repeatedly, thanks for your help with this change set. Quick question regarding the next steps. Should I add a disclaimer somewhere about the new input plugin being in a "beta" state? Or is there anything else that I should do so the PR can be merged? |
If you want to put it, there are 2 approaches:
|
Decided to do both. Is there anything else I should take care of? |
Signed-off-by: Tamas Kornai <[email protected]>
7e65adc
to
b65c75d
Compare
Merged. I will release new version soon. |
Hence solving for #350
Only tested locally, not ready for production use yet.
Checking the last commit in this PR shows diffs agains the in_kafka_group implementation which I used as a template.