-
Notifications
You must be signed in to change notification settings - Fork 180
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
out_rdkafka2: add patch for new version of rdkafka #505
Conversation
e3fa121
to
12d1f2e
Compare
I just see #500, and also, waterdrop encapsulation for rdkafka is just for producer side. |
This PR has been automatically marked as stale because it has been open 90 days with no activity. Remove stale label or comment or this PR will be closed in 30 days |
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.
Sorry for my late response, I overlooked this.
It looks good, but need to update for rdkafka-ruby v0.16.
Hey @kenhys, I'm now considering to revert #513 then apply this with some further improvements. In addition, we may need to feedback this timeout patch to upstream to avoid future monkey patching. What do you think about it?
I'm not sure yet but I agreed with reverting #513 to improve it. |
09f3f76
to
8b78582
Compare
In this approach, the border case should also follow in CI. |
Reverted #513. |
Signed-off-by: Thomas Tych <[email protected]> Signed-off-by: Kentaro Hayashi <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
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.
LGTM
Thanks for your contribution! @ttych |
All PR are failing now because of rdkafka gem update.
The current code works with rdkafka 0.12.0,
and in 0.14.0 (there is no 0.13.0), the
Rdkafka::Producer
implementation has changed,no more
@client
andRdkafka::Producer::Client
,now there is a
@native_client
andRdkafka::NativeKafka
.No update on this part of the code between 0.14.0 and 0.15.0.
There is a constraint in the out_rdkafka2, on the
Rdkafka::Producer#close
, to add a timeout,which is not part of the standard gem.
I try to update this behavior since following the current way to patch.
Thanks.