-
Notifications
You must be signed in to change notification settings - Fork 690
feat: support HF_HOME/_ENDPOINT env for Hugging Face models #2642
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
Conversation
|
👋 Hi hhk7734! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
WalkthroughUpdated Hugging Face client initialization in lib/llm/src/hub.rs::from_hf by replacing ApiBuilder::new() with ApiBuilder::from_env(), keeping subsequent builder methods unchanged. No public API signatures were altered. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant from_hf as from_hf()
participant Builder as ApiBuilder
participant HF as HuggingFace API
Caller->>from_hf: request client
from_hf->>Builder: from_env()
Note right of Builder: Preload env defaults<br/>(e.g., base URL, proxies)
from_hf->>Builder: with_progress(true)<br/>with_token(token)<br/>high()
from_hf->>Builder: build()
Builder-->>from_hf: Api client
from_hf-->>Caller: client
Caller->>HF: use client
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/llm/src/hub.rs (1)
44-49: Guard against clearing tokens discovered byfrom_env().
Calling.with_token(None)will wipe out any token already loaded byApiBuilder::from_env()(e.g. from the HuggingFace CLI’s token file). Only override whenHF_TOKENis explicitly set.Please update
lib/llm/src/hub.rs(around lines 44–49) as follows:@@ -42,11 +42,16 @@ pub async fn from_hf(name: impl AsRef<Path>, ignore_weights: bool) -> anyhow::Result<PathBuf> { - let token = env::var(HF_TOKEN_ENV_VAR).ok(); - let api = ApiBuilder::from_env() - .with_progress(true) - .with_token(token) - .high() - .build()?; + // Let from_env() load any token from the HF_HOME cache; only override when HF_TOKEN is provided. + let mut builder = ApiBuilder::from_env() + .with_progress(true) + .high(); + if let Ok(t) = env::var(HF_TOKEN_ENV_VAR) { + builder = builder.with_token(Some(t)); + } + let api = builder.build()?;Optional: you can drop explicit token handling entirely if you prefer to rely solely on
from_env().
🧹 Nitpick comments (2)
lib/llm/src/hub.rs (2)
57-61: Clarify error message for offline/endpoint configurations.Small UX improvement: mention HF_HUB_OFFLINE and HF_ENDPOINT hints to aid troubleshooting.
- return Err(anyhow::anyhow!( - "Failed to fetch model '{}' from HuggingFace: {}. Is this a valid HuggingFace ID?", - model_name, - e - )); + return Err(anyhow::anyhow!( + "Failed to fetch model '{}' from Hugging Face: {}. \ +If running with HF_HUB_OFFLINE=1, ensure the repo is cached under HF_HOME; \ +otherwise verify HF_ENDPOINT/HF_TOKEN and that the ID is valid.", + model_name, + e + ));
120-127: Simplify and make image check case-insensitive.Current logic duplicates extensions to handle case. Normalize once and compare.
-fn is_image(s: &str) -> bool { - s.ends_with(".png") - || s.ends_with("PNG") - || s.ends_with(".jpg") - || s.ends_with("JPG") - || s.ends_with(".jpeg") - || s.ends_with("JPEG") -} +fn is_image(s: &str) -> bool { + let s = s.to_ascii_lowercase(); + s.ends_with(".png") || s.ends_with(".jpg") || s.ends_with(".jpeg") +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/llm/src/hub.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build and Test - dynamo
- GitHub Check: pre-merge-rust (lib/runtime/examples)
- GitHub Check: pre-merge-rust (lib/bindings/python)
- GitHub Check: pre-merge-rust (.)
🔇 Additional comments (1)
lib/llm/src/hub.rs (1)
45-49: Switch to ApiBuilder::from_env() achieves the PR goal.Using
from_env()is the right move to pick up HF_HOME/HF_ENDPOINT/HF_HUB_OFFLINE automatically. The rest of the builder chain remains intact. Looks good.
47f1ea7 to
ea09af2
Compare
Signed-off-by: Hyeonki Hong <hhk7734@gmail.com>
|
Thank you @hhk7734 ! |
Signed-off-by: Hyeonki Hong <hhk7734@gmail.com> Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Signed-off-by: Hyeonki Hong <hhk7734@gmail.com>
Signed-off-by: Hyeonki Hong <hhk7734@gmail.com> Signed-off-by: Jason Zhou <jasonzho@jasonzho-mlt.client.nvidia.com>
Signed-off-by: Hyeonki Hong <hhk7734@gmail.com> Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Hyeonki Hong <hhk7734@gmail.com> Signed-off-by: nnshah1 <neelays@nvidia.com>
Overview:
This PR adds support for the
HF_HOMEandHF_ENDPOINTenvironment variables, allowing the use of already downloaded models or a Hugging Face cache server.Details:
Replaced
hf_hub::api::tokio::ApiBuilder::new()withhf_hub::api::tokio::ApiBuilder::from_env().Where should the reviewer start?
lib/llm/src/hub.rsRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
HF_ENDPOINTwhile connects to huggingface. #2631Test
Before this PR, DecodeWorkers did not work. After this change, the following tests pass.
hf_home_test.yaml
hf_endpoint_test.yaml
Summary by CodeRabbit