rust-rdkafka
rust-rdkafka copied to clipboard
Upgrade librdkafka to v1.9.0.
Resolves #485
Thanks! Did you run update-bindings.sh?
@benesch Apologies, somehow completely missed the Contributing doc. I updated the bindings now, and also checked that no new errors were added in this version (by comparing pub enum rd_kafka_resp_err_t
to pub fn rd_kafka_resp_err_t_to_rdkafka_error
).
No problem! Thanks for the PR. This looks good to me. I’m liable to forget to check back after CI completes, so feel free to ping me if/when this goes green.
The ci / check
is failing on librdkafka
adding a HTTP client support via curl, which I didn't update here yet. I'll update the PR when I have the time - I'll try to add it under a feature flag, statically linked by default, with the option to dynamically link against system curl
similar to how zstd
is handled.
I checked the existing features and decided to go with the libz
path for the curl
feature - optional, enabled by default, links dynamic library by default, and can be toggled to include curl that is statically linked.
Hi @duarten, thanks for triggering the CI job! I missed one more place to put the curl
default feature - now the check phase is passing (at least locally on my macOS).
I also added a missing build steps that correctly enable / disable CURL. I tested this locally by adding and removing the curl
and curl-static
from features when running cargo build
, and all variants (no curl, curl
, curl-static
) pass for me as well.
One thing I'm wondering - with the changes I made, libcurl
is a new dependency that is required by default (since the curl
feature is turned on by default). @benesch do you think that's okay, or should I rewrite to disable curl
by default, similar to zstd
and everything else? Thanks!
Hey all, I'm still struggling a little bit with making the Windows tests pass - I was able to add some missing library / include links for the compiler (so the top-level cargo build --all-targets --verbose --features "cmake-build,libz-static,curl-static"
step is now passing), but it's still failing on the cargo-sys cargo test --features "cmake-build,libz-static,curl-static"
test run. Below is the error message, and here's a link to the failed run.
Any help appreciated!
rdkafka.lib(rdhttp.obj) : error LNK2019: unresolved external symbol __imp_curl_global_init referenced in function rd_http_global_init
rdkafka.lib(rdhttp.obj) : error LNK2019: unresolved external symbol __imp_curl_easy_init referenced in function rd_http_req_init
rdkafka.lib(rdhttp.obj) : error LNK2019: unresolved external symbol __imp_curl_easy_setopt referenced in function rd_http_post_expect_json
rdkafka.lib(rdhttp.obj) : error LNK2019: unresolved external symbol __imp_curl_easy_perform referenced in function rd_http_req_perform_sync
rdkafka.lib(rdhttp.obj) : error LNK2019: unresolved external symbol __imp_curl_easy_cleanup referenced in function rd_http_get
rdkafka.lib(rdhttp.obj) : error LNK2019: unresolved external symbol __imp_curl_easy_getinfo referenced in function rd_http_get_json
D:\a\rust-rdkafka\rust-rdkafka\target\debug\deps\rdkafka_sys-6a9be4af9b107aaf.exe : fatal error LNK1120: 6 unresolved externals
I think I got it; I needed to include the -DCURL_STATICLIB
in the CFLAGS, as described in the curl docs. Here's a link to the passing windows check run: https://github.com/gyfis/rust-rdkafka/runs/7191217214?check_suite_focus=true
Hi @duarten, @thijsc ! Thanks for the previous actions trigger. I’m done with the PR, ready for review - care to take a look? (cc @benesch) 🙌
:+1: would be useful to get the updated librdkafka. I've been seeing some issues with mixed librdkafka version consumers in the same consumer group
Hi again @duarten, @thijsc ! Sorry for the repeated notifications, and thanks for the previous actions trigger. I’m done with the PR, ready for review - care to take a look? (cc @benesch) 🙌
Hi @benesch ! Apologies for pinging so much, just wondering what you think about the changes and if we can get the tests running to progress with the PR :) It's ready to be reviewed/merged. Let me know if there's anything you'd like to change here!
@gyfis maybe is worth updating to latest patch version 1.9.2 ?
Has anyone been testing this successfully? We've been hit with this bug from librdkafka https://github.com/edenhill/librdkafka/pull/3774/commits/7435bf4b9388f2d50fd3ac00636be71b13ea6a98 in production a few times, but can't really update.
Sorry for the delay here! Bumped the version to v1.9.2 and merged. I also disabled the curl
feature by default, since we keep all the other optionals disabled by default.
Thanks very much for all the work here, @gyfis! This was an awesome PR.