From 0572d7a1f62fbf5cfac242e57afc4504cfe468c1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 3 Sep 2026 15:53:02 -0400 Subject: [PATCH] Warn when a restricted Hugging Face download resolves no token download_huggingface_dataset keeps passing token=None through to hf_hub_download when a private repo 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 --- changelog.d/warn-hf-no-token.changed.md | 1 + policyengine_core/tools/hugging_face.py | 25 ++++ tests/core/tools/test_hugging_face.py | 187 ++++++++++++++++++++++++ 3 files changed, 213 insertions(+) create mode 100644 changelog.d/warn-hf-no-token.changed.md diff --git a/changelog.d/warn-hf-no-token.changed.md b/changelog.d/warn-hf-no-token.changed.md new file mode 100644 index 00000000..9373a1f5 --- /dev/null +++ b/changelog.d/warn-hf-no-token.changed.md @@ -0,0 +1 @@ +Warn when a Hugging Face repo that requires authentication is downloaded with no HUGGING_FACE_TOKEN available, so that a 401 raised after huggingface_hub's own cached-token fallback (HF_TOKEN or the `hf auth login` file) comes up empty is easy to trace back to the missing or unapproved token. diff --git a/policyengine_core/tools/hugging_face.py b/policyengine_core/tools/hugging_face.py index 7964658f..f56745df 100644 --- a/policyengine_core/tools/hugging_face.py +++ b/policyengine_core/tools/hugging_face.py @@ -57,6 +57,13 @@ def download_huggingface_dataset( repo_filename (str): The filename of the dataset. version (str, optional): The version of the dataset. Defaults to None. local_dir (str, optional): The local directory to save the dataset to. Defaults to None. + + Warns: + UserWarning: If the repo requires authentication but no + HUGGING_FACE_TOKEN was available. The download still runs with + token=None, so huggingface_hub applies its own cached token + (HF_TOKEN or the `hf auth login` file) if it has one; the + warning explains a 401 that follows when it does not. """ # Attempt connection to Hugging Face model_info endpoint # (https://huggingface.co/docs/huggingface_hub/v0.26.5/en/package_reference/hf_api#huggingface_hub.HfApi.model_info) @@ -80,6 +87,24 @@ def download_huggingface_dataset( authentication_token: str = None if is_repo_private: authentication_token: str = get_or_prompt_hf_token() + if authentication_token is None: + # Deliberately not an error: huggingface_hub resolves its own + # cached token when token=None, so `hf auth login` users still + # work. Warn so that the bare 401 huggingface_hub raises when + # that fallback is empty too can be traced back here (#529). + warnings.warn( + f"Hugging Face repo '{repo}' requires authentication, but no " + "HUGGING_FACE_TOKEN was available (the environment variable " + "is unset or empty, and no token was entered at a prompt). " + "huggingface_hub will use its own cached token instead if one " + "exists (the HF_TOKEN environment variable or the " + "`hf auth login` file). A 401 on the download that follows " + "(RepositoryNotFoundError or GatedRepoError) means neither " + "token was set, or the token in use is not approved for this " + "repo. Set HUGGING_FACE_TOKEN to a token whose account has " + "access.", + stacklevel=2, + ) return hf_hub_download( repo_id=repo, diff --git a/tests/core/tools/test_hugging_face.py b/tests/core/tools/test_hugging_face.py index 2a68682e..0436138e 100644 --- a/tests/core/tools/test_hugging_face.py +++ b/tests/core/tools/test_hugging_face.py @@ -1,4 +1,5 @@ import os +import warnings import pytest from unittest.mock import patch, MagicMock from huggingface_hub import ModelInfo @@ -225,3 +226,189 @@ def test_deep_subdirectory(self): def test_invalid_url_too_short(self): with pytest.raises(ValueError, match="Invalid hf:// URL format"): parse_hf_url("hf://owner/repo") + + +class TestNoTokenWarning: + """download_huggingface_dataset warns when it passes token=None for a + repo that needs authentication. + + Core deliberately does not raise or prompt in that case (#422): with + token=None, huggingface_hub falls back to its own cached token (HF_TOKEN + or the `hf auth login` file) and raises its own 401 if that is missing + too. The warning is what makes that 401 traceable to a missing or + unapproved HUGGING_FACE_TOKEN (#529). + """ + + repo = "test_owner/test_repo" + filename = "test_filename" + version = "test_version" + local_dir = "test_dir" + + def _download(self): + return download_huggingface_dataset( + self.repo, self.filename, self.version, self.local_dir + ) + + def _assert_downloaded_with(self, mock_download, token): + mock_download.assert_called_once_with( + repo_id=self.repo, + repo_type="model", + filename=self.filename, + revision=self.version, + token=token, + local_dir=self.local_dir, + ) + + @staticmethod + def _lookup_response(lookup): + """Configure model_info for the given repo visibility. + + "public": the repo is public, so no token is ever needed. + "private-flag": model_info answers with private=True. + "not-found": model_info raises RepositoryNotFoundError, which core + treats as "probably private". + """ + if lookup == "public": + return {"return_value": ModelInfo(id="test_repo", private=False)} + if lookup == "private-flag": + return {"return_value": ModelInfo(id="test_repo", private=True)} + assert lookup == "not-found" + mock_response = MagicMock() + mock_response.status_code = 404 + mock_response.headers = {} + return { + "side_effect": RepositoryNotFoundError("Test error", response=mock_response) + } + + @pytest.mark.parametrize("lookup", ["private-flag", "not-found"]) + @pytest.mark.parametrize( + "environ", + [{}, {"HUGGING_FACE_TOKEN": ""}, {"HF_TOKEN": "hf_cached_token"}], + ids=["token-unset", "token-empty", "hf-token-only"], + ) + def test_warns_when_no_token_resolved_non_interactively(self, lookup, environ): + """No HUGGING_FACE_TOKEN, no TTY: warn, then pass token=None through. + + The hf-token-only case pins that the warning still fires when only + huggingface_hub's own HF_TOKEN is set: core resolved nothing, and + the warning itself says the fallback will be used if present. + """ + model_info_config = self._lookup_response(lookup) + + with patch.dict(os.environ, environ, clear=True): + with patch("os.isatty", return_value=False): + with patch( + "policyengine_core.tools.hugging_face.getpass" + ) as mock_getpass: + mock_getpass.return_value = "prompted_token" + with patch( + "policyengine_core.tools.hugging_face.hf_hub_download" + ) as mock_download: + with patch( + "policyengine_core.tools.hugging_face.model_info", + **model_info_config, + ): + with pytest.warns( + UserWarning, match="no HUGGING_FACE_TOKEN" + ) as record: + result = self._download() + + # Behaviour is unchanged: no prompt, no raise, token=None passed on. + assert result is mock_download.return_value + mock_getpass.assert_not_called() + self._assert_downloaded_with(mock_download, token=None) + + # Exactly one warning, naming the repo, the fallback, and the 401. + assert len(record) == 1 + message = str(record[0].message) + assert self.repo in message + assert "HF_TOKEN" in message + assert "hf auth login" in message + assert "401" in message + # stacklevel=2: the warning points at the caller, not at core. + assert record[0].filename == __file__ + + def test_warns_when_interactive_prompt_left_empty(self): + """TTY present but the user enters nothing: same warning, token=None.""" + with patch.dict(os.environ, {}, clear=True): + with patch("os.isatty", return_value=True): + with patch( + "policyengine_core.tools.hugging_face.getpass", + return_value="", + ) as mock_getpass: + with patch( + "policyengine_core.tools.hugging_face.hf_hub_download" + ) as mock_download: + with patch( + "policyengine_core.tools.hugging_face.model_info", + **self._lookup_response("private-flag"), + ): + with pytest.warns( + UserWarning, match="no HUGGING_FACE_TOKEN" + ): + self._download() + + mock_getpass.assert_called_once() + self._assert_downloaded_with(mock_download, token=None) + + @pytest.mark.parametrize( + ("lookup", "environ", "isatty", "prompted", "expected_token"), + [ + pytest.param("public", {}, False, None, None, id="public-repo"), + pytest.param( + "public", + {"HUGGING_FACE_TOKEN": "env_token"}, + False, + None, + None, + id="public-repo-ignores-env-token", + ), + pytest.param( + "private-flag", + {"HUGGING_FACE_TOKEN": "env_token"}, + False, + None, + "env_token", + id="private-flag-env-token", + ), + pytest.param( + "not-found", + {"HUGGING_FACE_TOKEN": "env_token"}, + False, + None, + "env_token", + id="not-found-env-token", + ), + pytest.param( + "private-flag", + {}, + True, + "prompted_token", + "prompted_token", + id="private-flag-prompted-token", + ), + ], + ) + def test_no_warning_when_a_token_is_passed_or_not_needed( + self, lookup, environ, isatty, prompted, expected_token + ): + """Public repos pass token=None without warning; a resolved token + never warns. Guards against warning on every public download.""" + with patch.dict(os.environ, environ, clear=True): + with patch("os.isatty", return_value=isatty): + with patch( + "policyengine_core.tools.hugging_face.getpass", + return_value=prompted, + ): + with patch( + "policyengine_core.tools.hugging_face.hf_hub_download" + ) as mock_download: + with patch( + "policyengine_core.tools.hugging_face.model_info", + **self._lookup_response(lookup), + ): + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + self._download() + + self._assert_downloaded_with(mock_download, token=expected_token)