-
Notifications
You must be signed in to change notification settings - Fork 68
feat: add HF_TOKEN bearer token authentication for model pulls #447
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
The CLI now detects when pulling from hf.co/* repositories and automatically reads the HF_TOKEN environment variable. The token is passed through the ModelCreateRequest to the backend, which uses authn.Bearer for authentication with the registry. Co-authored-by: Ignasi <[email protected]> Signed-off-by: Dorin Geman <[email protected]>
Summary of ChangesHello @doringeman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new feature that enables seamless bearer token authentication for pulling models, specifically from Hugging Face repositories. By automatically detecting Hugging Face model references and utilizing the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
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.
Code Review
This pull request adds support for bearer token authentication when pulling models from Hugging Face, using the HF_TOKEN environment variable. The changes are well-structured, introducing a new BearerToken field in the ModelCreateRequest and propagating it through the layers to the distribution client. The use of functional options and a temporary client for authentication is a clean approach. I've made one suggestion to simplify the code in pkg/inference/models/manager.go for better maintainability. Overall, the implementation is solid and effectively addresses the feature requirement.
| var err error | ||
| if bearerToken != "" { | ||
| m.log.Infoln("Using provided bearer token for authentication") | ||
| err = m.distributionClient.PullModel(r.Context(), model, progressWriter, bearerToken) | ||
| } else { | ||
| err = m.distributionClient.PullModel(r.Context(), model, progressWriter) | ||
| } |
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.
This if/else block can be simplified. The distribution.Client.PullModel function takes a variadic bearerToken ...string argument. Calling it with an empty string for bearerToken is handled correctly by the receiver and has the same effect as calling it without the variadic argument. You can remove the else block and always pass the bearerToken.
if bearerToken != "" {
m.log.Infoln("Using provided bearer token for authentication")
}
err = m.distributionClient.PullModel(r.Context(), model, progressWriter, bearerToken)…tion header on redirect Co-authored-by: Ignasi <[email protected]> Signed-off-by: Dorin Geman <[email protected]>
Fixes #441.
The CLI now detects when pulling from hf.co/* repositories and automatically reads the HF_TOKEN environment variable. The token is passed through the ModelCreateRequest to the backend, which uses authn.Bearer for authentication with the registry.