fix(Huggingface): support modern hf CLI and new token location#482
Open
ilopezluna wants to merge 3 commits intomodelpack:mainfrom
Open
fix(Huggingface): support modern hf CLI and new token location#482ilopezluna wants to merge 3 commits intomodelpack:mainfrom
hf CLI and new token location#482ilopezluna wants to merge 3 commits intomodelpack:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request enhances HuggingFace integration by supporting multiple token storage locations (including HF_HOME, modern cache, and legacy paths) and providing compatibility with both the modern 'hf' and legacy 'huggingface-cli' tools. It also introduces comprehensive unit tests for authentication and token retrieval. The review feedback focuses on improving the robustness of CLI detection by using a boolean flag to identify legacy versions instead of relying on platform-dependent file path parsing, and recommends using absolute paths for command execution to avoid redundant lookups.
Signed-off-by: Ignacio López Luna <ignasi.lopez.luna@gmail.com>
…methods Signed-off-by: Ignacio López Luna <ignasi.lopez.luna@gmail.com>
- Return isLegacy boolean from findHFCLI instead of relying on filepath.Base - Use absolute path from exec.LookPath in checkHuggingFaceAuth Signed-off-by: Ignacio López Luna <ignasi.lopez.luna@gmail.com>
63c9d9a to
937c965
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem:
The HuggingFace provider fails authentication and download for users with the modern HuggingFace tooling. The code only checks the legacy token path (
~/.huggingface/token) and the legacy CLI (huggingface-cli), but the currenthuggingface_hub(≥ 0.14) stores tokens at~/.cache/huggingface/tokenand ships thehfCLI instead. Additionally, the--local-dir-use-symlinksflag used during download doesn't exist in thehfCLI.Solution:
$HF_HOME/token→~/.cache/huggingface/token→~/.huggingface/tokenhfCLI first, fall back tohuggingface-clifor auth checks and model downloads--local-dir-use-symlinkswhen using the legacyhuggingface-cli