fix(deps): drop default features on hf-hub; opt into rustls-tls#1459
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughHidden review stack artifactWalkthroughThis PR updates the ChangesDependency Feature Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @krishung5, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
There was a problem hiding this comment.
Code Review
This pull request updates the hf-hub dependency configuration in both crates/multimodal/Cargo.toml and crates/tokenizer/Cargo.toml. The changes disable default features and explicitly enable the tokio and rustls-tls features for this dependency. I have no feedback to provide as there were no review comments.
600aedf to
cd214f5
Compare
cd214f5 to
7db3afa
Compare
Both `multimodal` and `tokenizer` only use the async (tokio) hf-hub
client (`hf_hub::api::tokio::ApiBuilder`); they don't need the
default-on `native-tls` + blocking `ureq` features. Pin
`hf-hub = { default-features = false, features = ["tokio", "rustls-tls"] }`
so downstream crates don't have to pull in the openssl/native-tls
toolchain just to use lightseek.
This unblocks dynamo (https://github.com/ai-dynamo/dynamo) and any
other consumer that bans `native-tls` / `openssl-sys` in cargo-deny.
Signed-off-by: krishung5 <krish@nvidia.com>
7db3afa to
ae28503
Compare
|
@slin1237 Thanks for the quick review and merge! Quick question on release timeline, is there a target window for the next llm-multimodal crates.io publish that would include this change? |
hopefully by Wednesday |
Thanks, Wednesday time sounds good! Yes, in dynamo's Rust frontend we'd like to utilize the crates from smg project for resolving image token id and expanding image tokens for our MM aware KV routing. |
Description
Both
multimodalandtokenizeronly use the async (tokio) hf-hub client (hf_hub::api::tokio::ApiBuilder); they don't need the default-onnative-tls+ blockingureqfeatures. Pinhf-hub = { default-features = false, features = ["tokio", "rustls-tls"] }so downstream crates don't have to pull in the openssl/native-tls toolchain just to use lightseek.Problem
crates/multimodalandcrates/tokenizerdepend onhf-hub = "0.5.0"with default features (["default-tls", "tokio", "ureq"]). That transitively pulls in:native-tls,tokio-native-tls,hyper-tlsopenssl,openssl-sys,openssl-macrosopenssl-src(vendored OpenSSL C source — requiresperlat build time)ureqclient (with native-tls)Neither crate ever calls the blocking client — the only hf-hub usage in both crates is
hf_hub::api::tokio::ApiBuilder. Consumers that depend onllm-multimodal(e.g.ai-dynamo/dynamo, which explicitly bansnative-tlsandopenssl-sysin theircargo-denyconfig) currently have to fork or[patch.crates-io]to use the crate.Solution
Pin
hf-hubwithdefault-features = falseand the minimal feature set actually used (tokio+rustls-tls). The asyncApiBuilderAPI stays available; the openssl/native-tls subtree disappears from the dep graph; the build no longer needsperlto compile vendored OpenSSL.Changes
crates/multimodal/Cargo.toml:hf-hub = "0.5.0"→hf-hub = { version = "0.5.0", default-features = false, features = ["tokio", "rustls-tls"] }crates/tokenizer/Cargo.toml:hf-hub = { version = "0.5.0", features = ["tokio"] }→hf-hub = { version = "0.5.0", default-features = false, features = ["tokio", "rustls-tls"] }No code changes — both crates already only call
hf_hub::api::tokio::ApiBuilder, which is in thetokiofeature.Test Plan
Before:
After:
cargo tree -i native-tlsreturns "did not match any packages".cargo check -p llm-multimodal -p llm-tokenizerpasses:Existing tests in both crates pass; no behavior change in the async
ApiBuilderpath that the crates actually use.Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit