diff --git a/src/tt_kernel/cli.py b/src/tt_kernel/cli.py index 7a320c9..6f98050 100644 --- a/src/tt_kernel/cli.py +++ b/src/tt_kernel/cli.py @@ -1675,16 +1675,15 @@ def publish( if was_private: # The catalog is public by definition; publishing implies public (same as the --publish # flag on push). Make the visibility change loud — it is never a silent side effect. - typer.secho( - f"! {repo_id} is private — making it public so it can be listed in the public catalog.", - fg=typer.colors.YELLOW, + console.note( + f"{repo_id} is private — making it public so it can be listed in the public catalog", + marker="!", style="warning", ) hub.set_visibility(repo_id, private=False) hub.set_catalog_listing(repo_id, listed=True) - typer.secho( - f"✓ Listed {repo_id} in the community catalog (public pointer only; content stays yours). " - f"Delist with `tt-model unpublish {repo_id}`.", - fg=typer.colors.GREEN, + console.milestone( + f"listed {repo_id} in the community catalog (public pointer only; content stays " + f"yours) — delist with `tt-model unpublish {repo_id}`" ) @@ -1695,10 +1694,9 @@ def unpublish( ) -> None: """Remove a bundle from the community catalog. The repo itself is untouched.""" hub.set_catalog_listing(repo_id, listed=False) - typer.secho( - f"✓ Delisted {repo_id} from the community catalog (it drops off on the next crawl). " - "The repo and its content are unchanged.", - fg=typer.colors.GREEN, + console.milestone( + f"delisted {repo_id} from the community catalog (it drops off on the next crawl) " + "— the repo and its content are unchanged" ) diff --git a/src/tt_kernel/hub.py b/src/tt_kernel/hub.py index 593dbb2..8489eca 100644 --- a/src/tt_kernel/hub.py +++ b/src/tt_kernel/hub.py @@ -409,7 +409,17 @@ def classify_hub_error(exc: BaseException, repo_id: str, *, weights: bool = Fals (``GatedRepoError``, ``EntryNotFoundError``), then status code, then message text last. A status code alone can't tell a missing repo from a missing file — both 404 — which is why ``EntryNotFoundError`` is checked before the 404 branch below rather than after it. + + Class checks use ``isinstance``, never the class NAME. huggingface_hub raises + SUBCLASSES for the cases it can be specific about — 1.x answers a missing file with + ``RemoteEntryNotFoundError`` and a cache miss under offline mode with + ``LocalEntryNotFoundError``, both deriving from ``EntryNotFoundError`` — so a name + comparison stops matching the moment the library gets more precise, with no error + anywhere: the branch simply falls through to the repo-level 404 and tells the user + their repo does not exist when only one file inside it is missing. """ + from huggingface_hub.utils import EntryNotFoundError, LocalEntryNotFoundError + subject = "weights repo" if weights else "bundle" name = type(exc).__name__ text = str(exc) @@ -418,7 +428,10 @@ def classify_hub_error(exc: BaseException, repo_id: str, *, weights: bool = Fals # Offline / DNS / TLS beats everything: an unreachable Hub also can't confirm a repo, # and "you are offline" is the more useful half of that pair. - if (name in ("LocalEntryNotFoundError", "OfflineModeIsEnabled") + # LocalEntryNotFoundError first, and it must stay first: it is also an + # EntryNotFoundError, so the missing-file branch below would otherwise claim an + # offline read is a repo with no manifest. + if (isinstance(exc, LocalEntryNotFoundError) or name == "OfflineModeIsEnabled" or any(k in low for k in ("no such host", "dial tcp", "i/o timeout", "tls handshake", "connection refused", "connection error", "max retries exceeded", @@ -444,7 +457,7 @@ def classify_hub_error(exc: BaseException, repo_id: str, *, weights: bool = Fals # render "this repo ships no manifest" as "no such repo, or no access" — the type is # the only thing that tells the two apart. Bundles only: a weights repo is never # expected to carry a manifest, so for weights this is left to the 404 branch below. - if not weights and (name == "EntryNotFoundError" or MANIFEST_NAME in text): + if not weights and (isinstance(exc, EntryNotFoundError) or MANIFEST_NAME in text): return { "cause": "not a tt-model bundle", "detail": f"The repo exists but has no {MANIFEST_NAME}, so there is nothing for tt-model " diff --git a/tests/test_catalog_publish.py b/tests/test_catalog_publish.py index f94c439..0f4b50f 100644 --- a/tests/test_catalog_publish.py +++ b/tests/test_catalog_publish.py @@ -34,6 +34,29 @@ def test_publish_makes_a_private_repo_public_then_lists(monkeypatch): assert "public" in res.output.lower() # and the flip was announced +def test_publish_reports_through_the_phase_body_not_raw_echo(monkeypatch): + """Both lines go through console.py, and this test can tell the difference. + + `typer.secho` writes flush-left, unmarked, and wraps wherever the terminal happens + to break — so a reader gets no gutter to scan and a wrapped line lands under the + phase label instead of under its own marker. The assertions below are on exactly + what console.py adds and secho cannot: a `!`/`✓` marker in a two-space gutter, and + padding that every wrapped continuation line keeps. + """ + monkeypatch.setattr(hub, "is_private", lambda repo_id: True) + monkeypatch.setattr(hub, "set_visibility", lambda repo_id, private: None) + monkeypatch.setattr(hub, "set_catalog_listing", lambda repo_id, listed: None) + + res = runner.invoke(cli.app, ["publish", "me/private-bundle"]) + assert res.exit_code == 0, res.output + lines = [ln for ln in res.output.splitlines() if ln.strip()] + assert lines, res.output + assert any(ln.startswith(" ! ") for ln in lines), res.output # the warning note + assert any(ln.startswith(" ✓ ") for ln in lines), res.output # the milestone + # Padding, not a string indent: continuation lines stay in the body column. + assert all(ln.startswith(" ") for ln in lines), res.output + + def test_publish_lists_public_repo(monkeypatch): calls = [] monkeypatch.setattr(hub, "is_private", lambda repo_id: False) diff --git a/tests/test_hub_errors.py b/tests/test_hub_errors.py index ac68a5d..2350992 100644 --- a/tests/test_hub_errors.py +++ b/tests/test_hub_errors.py @@ -10,6 +10,7 @@ """ import pytest +from huggingface_hub import utils as _hf_errors from typer.testing import CliRunner from tt_kernel import MANIFEST_NAME @@ -24,9 +25,23 @@ def __init__(self, code): def _exc(name, message, status=None): - """Build a stand-in that looks like the huggingface_hub exception of that name.""" - cls = type(name, (Exception,), {}) - e = cls(message) + """Build a stand-in for the huggingface_hub exception of that name. + + The REAL class when huggingface_hub ships one, because ``classify_hub_error`` + branches with ``isinstance``: a same-named fake would still be routed by the message + heuristics and the test would pass while the type evidence went unexercised. Falls + back to a fabricated class for names the installed version does not define, so this + file keeps covering exceptions from other hub versions. + """ + cls = getattr(_hf_errors, name, None) + if not (isinstance(cls, type) and issubclass(cls, BaseException)): + cls = type(name, (Exception,), {}) + try: + e = cls(message) + except TypeError: + # 1.x errors that demand a response kwarg; build without running __init__. + e = cls.__new__(cls) + Exception.__init__(e, message) if status is not None: e.response = _Resp(status) return e @@ -132,6 +147,46 @@ def test_a_real_missing_manifest_404_is_not_reported_as_a_missing_repo(): assert d["cause"] == "not a tt-model bundle" +def test_the_subclass_hf_actually_raises_for_a_missing_file_is_recognised(): + """huggingface_hub 1.x does not raise ``EntryNotFoundError`` for a missing file — it + raises ``RemoteEntryNotFoundError``, a subclass. Matching on the class NAME therefore + stopped matching with no error anywhere: the branch fell through to the repo-level + 404 and told the user their repo did not exist when only one file in it was missing. + + The message here deliberately carries neither the manifest filename nor a network + keyword, so the class is the only evidence available — which is the point. + """ + # Only `huggingface_hub.errors` exports the subclass; production imports the BASE + # class from `.utils` (the >=0.23 floor has no `.errors` module) and relies on + # isinstance to cover it, which is exactly what this asserts. + cls = None + try: + from huggingface_hub import errors as _hf_errors_mod + + cls = getattr(_hf_errors_mod, "RemoteEntryNotFoundError", None) + except ImportError: + pass + if cls is None: # pre-1.x: the base class is what gets raised, covered above + pytest.skip("this huggingface_hub has no RemoteEntryNotFoundError") + exc = cls.__new__(cls) + Exception.__init__(exc, "404 Client Error. (Request ID: Root=1-deadbeef;0001)") + exc.response = _Resp(404) + d = classify_hub_error(exc, "x/y") + assert d["cause"] == "not a tt-model bundle" + + +def test_an_offline_cache_miss_still_beats_the_missing_file_branch(): + """``LocalEntryNotFoundError`` is ALSO an ``EntryNotFoundError``, so the offline + branch has to be tested first or an offline read reports "this repo ships no + manifest". Message carries no network keyword, so only the class ordering can save + it.""" + exc = _exc("LocalEntryNotFoundError", "An error happened while trying to locate the " + "file on the Hub and we cannot find the " + "requested files in the local cache.") + d = classify_hub_error(exc, "x/y") + assert d["cause"] == "cannot reach the Hub" + + # ------------------------------------------------------- the same taxonomy, for weights # # `pull` classifies a failed WEIGHTS fetch through this same function. The failure modes