Serve fully cached repos without a Hub round-trip - #1650
Open
Vlor999 wants to merge 1 commit into
Open
Conversation
_download() called snapshot_download() without local_files_only, so even a fully cached repo paid an API round-trip to check for updates. load() calls _download() twice (weights and tokenizer files), so every invocation paid it twice. Try local_files_only=True first and fall back to the online path on LocalEntryNotFoundError. ModelScope has no equivalent, so the fast path is gated on the Hugging Face backend. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Proposed changes
Fixes #1649.
_download()calledsnapshot_download()withoutlocal_files_only, so a repothat is already fully cached still paid a Hub API round-trip to check for
updates before serving from the cache.
load()calls_download()twice — oncefor weights, once for tokenizer files — so every invocation paid it twice, for
the CLI and for library users alike, not only on first download.
This tries
local_files_only=Truefirst and falls back to the existing onlinecall on
LocalEntryNotFoundError, so behaviour is unchanged whenever the cachecannot satisfy the request.
Why
huggingface_hub1.26.0, warm cache, median of 5 calls on an already-cached repo:load()(2 calls)Reproducer is in #1649. The absolute number is latency to the Hub, so it grows
on a slow link — and on a flaky one the current code blocks until the check
times out, where this returns immediately.
How
LocalEntryNotFoundErroronly._supports_local_files_only, because ModelScope'ssnapshot_downloadtakes no such argument. On ModelScope the behaviour is byte-for-byte unchanged.
hf_repo_to_path()in the same file already useslocal_files_only=True, sothis follows an established pattern rather than introducing one.
Alternative rejected: catching a broader exception.
LocalEntryNotFoundErroris what a cache miss raises (verified against 1.26.0); catching more would
silently swallow real Hub errors and turn them into a redundant second attempt.
Validation
Two tests added:
test_cached_repo_skips_network— a cached repo results in exactly onesnapshot_downloadcall, made withlocal_files_only=True.test_falls_back_to_network_when_not_cached— aLocalEntryNotFoundErrorproduces a second call without
local_files_only.I checked the tests are not vacuous by flipping the new argument to
False:both fail. With the fix in place, all 11 pass.
Supersedes #1571, which was opened against an older
main. This is the same fixrebuilt on current
main, with the measurement and tests added.