Warn when a restricted Hugging Face download resolves no HUGGING_FACE_TOKEN - #540
Conversation
0572d7a to
f5992fd
Compare
…vely Fold in the non-blocking findings from the independent review of #540: - Say a 401 means no token was sent and a 403 means a gated repo has not approved the token; huggingface_hub raises GatedRepoError for both and documents the unapproved case as 403. - Say the fallback to huggingface_hub's cached token is what normally happens: HF_HUB_DISABLE_IMPLICIT_TOKEN turns it off. - Name `huggingface-cli login` as well as `hf auth login`: the `hf` command first ships in huggingface_hub 0.34, and pyproject still allows 0.25.1. - Assert the warning in the two older no-token tests that now emit it, and give the ungated never-prompts test's getpass mock a string return value so a widened predicate fails on the assertion, not on os.environ. - Reword the changelog fragment: after #538 the repo may be private or gated, and a cached token that is not approved gives a 403. Add TestTokenRoutingInvariants, which runs every combination of repo state (ungated, gated None, fields missing, gated auto, gated manual, private, not found), environment (token unset, empty, only HF_TOKEN, set), TTY and prompt entry (112 cases) through the real function and checks it against a spec written out in the test: the token passed on, whether getpass is called, that the token is never "", and that exactly one warning fires when a repo requiring authentication ends up with no token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f5992fd to
ded7983
Compare
download_huggingface_dataset keeps passing token=None through to hf_hub_download when a repo that requires authentication (private, or gated since #538) yields no HUGGING_FACE_TOKEN (per #422: huggingface_hub then applies its own cached token, so `hf auth login` users keep working), but now emits a UserWarning first so that the bare 401 huggingface_hub raises when that fallback is empty too can be traced to the missing or unapproved token (#529). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
With #538, a public but gated repo (private=False, gated="manual", as policyengine/policyengine-uk-data-private is) also requires authentication, so it takes the same path that now warns when no HUGGING_FACE_TOKEN resolves. Add a "gated" lookup to TestNoTokenWarning: the three non-interactive no-token environments must warn, and an environment token must reach hf_hub_download without a warning. With the private-only predicate restored, the four new gated cases fail; with the warning removed, all ten warning-asserting cases fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…vely Fold in the non-blocking findings from the independent review of #540: - Say a 401 means no token was sent and a 403 means a gated repo has not approved the token; huggingface_hub raises GatedRepoError for both and documents the unapproved case as 403. - Say the fallback to huggingface_hub's cached token is what normally happens: HF_HUB_DISABLE_IMPLICIT_TOKEN turns it off. - Name `huggingface-cli login` as well as `hf auth login`: the `hf` command first ships in huggingface_hub 0.34, and pyproject still allows 0.25.1. - Assert the warning in the two older no-token tests that now emit it, and give the ungated never-prompts test's getpass mock a string return value so a widened predicate fails on the assertion, not on os.environ. - Reword the changelog fragment: after #538 the repo may be private or gated, and a cached token that is not approved gives a 403. Add TestTokenRoutingInvariants, which runs every combination of repo state (ungated, gated None, fields missing, gated auto, gated manual, private, not found), environment (token unset, empty, only HF_TOKEN, set), TTY and prompt entry (112 cases) through the real function and checks it against a spec written out in the test: the token passed on, whether getpass is called, that the token is never "", and that exactly one warning fires when a repo requiring authentication ends up with no token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a "private-gated" repo state (private=True, gated="manual") to TestTokenRoutingInvariants, so a predicate that is true for private or gated but false for both (for example an xor) now fails 16 cases instead of passing all of them. Assert getpass's call_count rather than whether it was called, so a double prompt fails 30 grid cases, not one side test. The grid grows from 112 to 128 cases. Also give the Warns: docstring the same `huggingface-cli login` caveat the runtime warning already carries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ded7983 to
7b8e76c
Compare
…text The non-interactive warning test now also asserts that the message names RepositoryNotFoundError, GatedRepoError, the 403 case and the pre-0.34 `huggingface-cli login` command. Each of those clauses could previously be deleted without failing a test; each deletion now fails 9 of 167. The TestNoTokenWarning docstring no longer calls the 401 "missing or unapproved": with an empty fallback there is no token to be unapproved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merge audit for head 1de9bc6. This is the round-4 review, after the rebase onto 4fb64c9 (3.32.10). Independent review. An Opus 5.5 peer (subfleet job
Main-session checks on this head:
Gates:
Squash-merged with |
Follow-up to #529 (fixed by #538). Related: #553.
Summary
download_huggingface_datasetnow emits aUserWarningwhen the repo requires authentication andget_or_prompt_hf_token()returnedNone. Since #538, a repo requires authentication whenmodel_inforeports it private or gated, or raisesRepositoryNotFoundError. The warning names the repo, says that noHUGGING_FACE_TOKENwas available, and says that huggingface_hub normally falls back to its own cached token (HF_TOKEN, or the file written byhf auth login, which ishuggingface-cli loginbefore huggingface_hub 0.34). It then explains the failure that can follow:RepositoryNotFoundErrororGatedRepoError, as a 401 when no token was sent or a 403 when a gated repo has not approved the token.Behaviour is otherwise unchanged, deliberately (per #422).
token=Noneis still passed tohf_hub_download, there is no raise and no extra prompt, and core does not readHF_TOKEN. The fallback belongs to huggingface_hub: in 1.4.1,utils/_headers.py::get_token_to_sendcallsget_token()whentoken is None(unlessHF_HUB_DISABLE_IMPLICIT_TOKENis set), andutils/_http.py::hf_raise_for_statusmapsX-Error-Code: GatedRepotoGatedRepoErrorand a 401 on a resolve URL toRepositoryNotFoundError.The warning uses
stacklevel=2, so it points at the caller (for exampleDataset.download_from_huggingface). It is not emitted for public, ungated repos, which also gettoken=None. It still fires for users whose only token isHF_TOKENor the login file, even though their download may succeed; #553 tracks skipping it (and the prompt) in that case.This branch is rebased onto current
master(4fb64c9, 3.32.10), so it sits on top of #538 (released in 3.32.9) and #539 (3.32.7). The rebase was clean, andgit range-diffshows the three original commits unchanged.Invariants
TestTokenRoutingInvariantsruns every combination of 8 repo states × 4 environments × TTY or not × empty or non-empty prompt entry (128 cases) through the real function, withmodel_info,hf_hub_downloadandgetpassmocked. It compares each result with a spec written out by hand in_expected(), which lists the states that need a token rather than recomputing the predicate:autoormanual), private, private-and-gated and not-found repos require authentication.HUGGING_FACE_TOKENif it is non-empty; otherwise, on a TTY, whatever is entered at a single prompt; otherwiseNone.Noneand is never prompted for.Noneor a non-empty string, never"".None, and none fires otherwise.Tests
TestNoTokenWarning:HF_TOKENis set, non-interactively, and when an interactive prompt is left empty.token=Noneis passed on and there is no extra prompt.stacklevelare asserted. The message must name the repo, both fallbacks (HF_TOKEN,hf auth loginand the pre-0.34huggingface-cli login),RepositoryNotFoundError,GatedRepoError, the 401 and the 403.test_download_private_repo_no_token(from Fix vacuous test for private Hugging Face repo download without a token #539) andtest_download_gated_repo_non_interactive_without_token(from Send HUGGING_FACE_TOKEN for public but gated Hugging Face repos #538) now wrap the call inpytest.warns(UserWarning, match="no HUGGING_FACE_TOKEN"). Their existing assertions are unchanged.TestTokenRoutingInvariants, described above.pytest tests/core/tools/test_hugging_face.pyon 1de9bc6 gives 167 passed, in the project.venv(CPython 3.14; pytest 9.1.1 and huggingface_hub 1.4.1, the versions inuv.lock): 39 other tests plus the 128-case grid.Mutation checks on 1de9bc6, where each mutant is applied to
hugging_face.pyalone and the file is then restored:bool(private)aloneget_or_prompt_hf_token() or ""RepositoryNotFoundErrortreated as publicbool(private) != bool(gated)(xor)getpasscalled twicestacklevel=1GatedRepoError, or thehuggingface-cli logincaveat deleted from the message (each alone)HUGGING_FACE_TOKENif not authentication_token:instead ofis Noneget_or_prompt_hf_token()returnsNoneor a non-empty stringThe PR's earlier independent review also checked mutants for "always requires authentication", "warn even with a token", "TTY ignored", "environment token ignored" and several more; each failed in the grid.
uvx ruff format --check .anduvx ruff checkon the changed files are clean.Not run:
make test(the full suite with coverage and reruns) andmake documentation. CI covers the tests on Ubuntu and Windows for Python 3.11–3.14, and no docs pages change.Documentation review
Warns:section in thedownload_huggingface_datasetdocstring, and the towncrier fragmentchangelog.d/warn-hf-no-token.changed.md(changed, since this adds a diagnostic and does not change control flow). Nodocs/page or README describes this function's token handling, so nothing else needed updating.token=Noneandgetpassassertions and the grid pin down.GatedRepoErrordocstring and was not reproduced with a real unapproved token.HF_TOKENor login-file token will work).axiom: n/a: core infrastructure (Hugging Face download diagnostics), no policy change.
🤖 Generated with Claude Code