diff --git a/.github/workflows/citest.yaml b/.github/workflows/citest.yaml index a023025..9dfb4c6 100644 --- a/.github/workflows/citest.yaml +++ b/.github/workflows/citest.yaml @@ -20,6 +20,11 @@ on: - "LICENSE" - ".github/workflows/publish.yaml" + # The cross-repo job below is opt-in: it installs the umbrella SDK from its + # own default branch, so gating every push on that would make this pipeline + # fail whenever an unrelated change lands there. + workflow_dispatch: + concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true @@ -77,3 +82,122 @@ jobs: - name: Mypy run: mypy src/modelscope_hub/ + + cli-entrypoints: + # This package installs every ModelScope console script, and the umbrella + # `modelscope` SDK deliberately declares none of its own. That split is the + # contract between the two: if these four stop being installed here, the + # brand CLI disappears for everyone downstream. Installed non-editable, + # because that is how users actually receive the scripts. + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.13" + + - name: Install package + run: pip install . + + - name: Every alias is installed and runnable + run: | + for cmd in modelscope ms modelscope-hub ms-hub; do + command -v "$cmd" >/dev/null || { echo "::error::missing console script: $cmd"; exit 1; } + "$cmd" --version >/dev/null || { echo "::error::$cmd --version failed"; exit 1; } + done + + - name: Hub aliases speak only for this package + run: | + for cmd in modelscope-hub ms-hub; do + "$cmd" --version | grep -q '^modelscope-hub ' \ + || { echo "::error::$cmd --version should report only modelscope-hub"; exit 1; } + if "$cmd" --help | grep -qF 'pip install modelscope'; then + echo "::error::$cmd must not advertise the umbrella SDK"; exit 1 + fi + done + + - name: Brand aliases flag the absent SDK + run: | + for cmd in modelscope ms; do + "$cmd" --version | grep -qF 'not installed' \ + || { echo "::error::$cmd --version should flag the missing SDK"; exit 1; } + "$cmd" --help | grep -qF 'pip install modelscope' \ + || { echo "::error::$cmd --help should explain how to get SDK commands"; exit 1; } + done + + cli-with-umbrella-sdk: + # Manual only (see `workflow_dispatch` above). Verifies the half of the + # contract that lives in the other repository: with both packages present + # the brand aliases must report the SDK, the SDK's commands must appear as + # plugins, and removing the SDK must not take the CLI down with it. + if: github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.13" + + - name: Install this package, then the umbrella SDK + run: | + pip install . + pip install "modelscope @ git+https://github.com/modelscope/modelscope" + # Reinstate the checkout: resolving the SDK may have pulled a + # released modelscope-hub over the build under test. + pip install --force-reinstall --no-deps . + + - name: The umbrella SDK must not ship competing scripts + run: | + python - <<'PY' + import importlib.metadata as md + import sys + scripts = [ + ep.name + for ep in md.distribution("modelscope").entry_points + if ep.group == "console_scripts" + ] + if scripts: + print(f"::error::modelscope declares console scripts {scripts}; " + "they belong to modelscope-hub") + sys.exit(1) + PY + + - name: Brand aliases report the SDK, hub aliases do not + run: | + for cmd in modelscope ms; do + "$cmd" --version | grep -q '^modelscope ' \ + || { echo "::error::$cmd --version should lead with the SDK version"; exit 1; } + "$cmd" --version | grep -qF 'modelscope-hub' \ + || { echo "::error::$cmd --version should still name the hub"; exit 1; } + done + ms-hub --version | grep -q '^modelscope-hub ' \ + || { echo "::error::ms-hub --version must stay hub-only"; exit 1; } + + - name: SDK commands are discovered as plugins, without collisions + run: | + help_text="$(modelscope --help 2>&1)" + for cmd in pipeline studio modelcard download upload; do + printf '%s' "$help_text" | grep -qF "$cmd" \ + || { echo "::error::command '$cmd' missing from help"; exit 1; } + done + if printf '%s' "$help_text" | grep -qF 'already registered'; then + echo "::error::a plugin collided with a built-in command"; exit 1 + fi + python -m modelscope.cli.cli --version >/dev/null \ + || { echo "::error::python -m modelscope.cli.cli regressed"; exit 1; } + + - name: Removing the SDK leaves the CLI intact + run: | + pip uninstall -y modelscope + for cmd in modelscope ms modelscope-hub ms-hub; do + "$cmd" --version >/dev/null \ + || { echo "::error::$cmd died with the SDK uninstall"; exit 1; } + done diff --git a/README.md b/README.md index f348a62..3455325 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,36 @@ pip install modelscope-hub Requires Python 3.10+. Lightweight — only `requests`, `tqdm`, `filelock`, `urllib3`. +### Command names + +This package installs every ModelScope console script, and all four are the same +program — pick whichever reads best in your shell: + +| Command | Use it for | +| --- | --- | +| `modelscope` | the primary, brand-level command | +| `ms` | short form of `modelscope` | +| `modelscope-hub` | explicit hub-only invocation | +| `ms-hub` | short form of `modelscope-hub` | + +Owning all four in one distribution is deliberate: when the umbrella +[`modelscope`](https://github.com/modelscope/modelscope) SDK also declared +`modelscope` / `ms`, installing or upgrading either package could overwrite or +delete the other's scripts and leave you with no CLI at all. OS packagers (such +as FreeBSD pkg) likewise refuse to let two ports own the same path. The SDK now +contributes its extra commands as plugins instead. + +So `modelscope` and `ms` work on a hub-only install too. Commands that belong to +the full SDK (`pipeline`, `server`, `studio`, `modelcard`, …) appear once you +also `pip install modelscope`; until then `--help` says so. The two `*-hub` +aliases always report just this package, which is handy for scripts that must +pin the lightweight client: + +```bash +ms-hub --version # modelscope-hub X.Y.Z +modelscope --version # modelscope A.B.C (modelscope-hub X.Y.Z) +``` + --- ## Quick Start diff --git a/pyproject.toml b/pyproject.toml index eda3b10..4ebd36d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,6 +35,14 @@ dev = [ ] [project.scripts] +# This package owns every ModelScope console script. Keeping all four in one +# distribution means only one installer ever writes these files: when the +# umbrella `modelscope` package also declared `modelscope`/`ms`, upgrading +# either package could overwrite or delete the other's scripts and leave the +# user with no CLI at all. It also keeps OS packagers (e.g. FreeBSD pkg, which +# refuses two ports owning one path) free of file conflicts. +modelscope = "modelscope_hub.cli.main:run_cmd" +ms = "modelscope_hub.cli.main:run_cmd" modelscope-hub = "modelscope_hub.cli.main:run_cmd" ms-hub = "modelscope_hub.cli.main:run_cmd" diff --git a/src/modelscope_hub/_legacy_api.py b/src/modelscope_hub/_legacy_api.py index b460e33..2fc7547 100644 --- a/src/modelscope_hub/_legacy_api.py +++ b/src/modelscope_hub/_legacy_api.py @@ -15,6 +15,7 @@ from __future__ import annotations import uuid +from concurrent.futures import ThreadPoolExecutor, as_completed from typing import IO, Any, BinaryIO from urllib.parse import quote_plus, urlparse @@ -26,6 +27,9 @@ API_MAX_RETRIES, API_TIMEOUT, LEGACY_API_PREFIX, + REPO_FILES_TRUNCATION_LIMIT, + REPO_TREE_MAX_REQUESTS, + REPO_TREE_WALK_WORKERS, UPLOAD_BLOB_CONNECT_TIMEOUT, UPLOAD_BLOB_READ_TIMEOUT, UPLOAD_RETRY_ALLOWED_METHODS, @@ -62,6 +66,21 @@ def _resolve_segment(repo_type: str) -> str: return _REPO_TYPE_SEGMENT.get(repo_type, f"{repo_type}s") +def _is_dataset(repo_type: str) -> bool: + """Whether the repo_type addresses the dataset endpoints.""" + return repo_type in (RepoType.DATASET, "dataset", "datasets") + + +def _entry_path(entry: dict) -> str: + """Return the repo-relative path of a file-tree entry.""" + return entry.get("Path") or entry.get("path") or entry.get("Name") or "" + + +def _is_dir_entry(entry: dict) -> bool: + """Whether a file-tree entry is a directory (git tree object).""" + return (entry.get("Type") or entry.get("type") or "blob") == "tree" + + class LegacyClient: """Internal client for /api/v1/ endpoints not covered by OpenAPI. @@ -316,7 +335,46 @@ def list_repo_files( Models/studios/etc: GET /api/v1/{type}s/{repo_id}/repo/files Datasets: GET /api/v1/datasets/{repo_id}/repo/tree + + ``repo/files`` supports no pagination and silently truncates at + :data:`REPO_FILES_TRUNCATION_LIMIT` entries, so a recursive listing that + comes back exactly at the limit is re-enumerated directory by directory + (see :meth:`_walk_repo_files`). Datasets take the ``repo/tree`` + endpoint, which does paginate, so they are paged through instead. """ + if _is_dataset(repo_type): + if recursive: + return self.list_dataset_files_paginated( + repo_id=repo_id, + revision=revision, + root_path=root or "/", + ) + return self._list_files_page(repo_id, repo_type, revision, recursive=False, root=root) + + entries = self._list_files_page(repo_id, repo_type, revision, recursive=recursive, root=root) + if len(entries) < REPO_FILES_TRUNCATION_LIMIT: + return entries + if not recursive: + logger.warning( + "Directory %r of %s holds at least %d entries and the server returns no more " + "than that for a single listing; the result is incomplete.", + root or "/", + repo_id, + REPO_FILES_TRUNCATION_LIMIT, + ) + return entries + return self._walk_repo_files(repo_id, repo_type, revision, root=root, prefetched=entries) + + def _list_files_page( + self, + repo_id: str, + repo_type: str, + revision: str, + *, + recursive: bool, + root: str | None = None, + ) -> list[dict]: + """Perform a single file-tree request and unwrap the entry list.""" segment = _resolve_segment(repo_type) params: dict[str, Any] = { "Revision": revision, @@ -325,7 +383,7 @@ def list_repo_files( if root: params["Root"] = root - suffix = "repo/tree" if repo_type in (RepoType.DATASET, "dataset", "datasets") else "repo/files" + suffix = "repo/tree" if _is_dataset(repo_type) else "repo/files" resp = self._request("GET", f"{segment}/{repo_id}/{suffix}", params=params) data = self._json_data(resp) if isinstance(data, list): @@ -335,6 +393,158 @@ def list_repo_files( return data.get("Files") or data.get("files") or [] return [] + def _walk_repo_files( + self, + repo_id: str, + repo_type: str, + revision: str, + *, + root: str | None = None, + prefetched: list[dict] | None = None, + ) -> list[dict]: + """Enumerate a file tree the server truncated, one directory at a time. + + Since ``repo/files`` caps every response, the only way to see past the + cap is to scope requests with ``Root``. Each subtree is first requested + whole; only the subtrees that come back at the cap are listed shallowly + and walked per child directory. The request count therefore scales with + the number of oversized directories, not with the directory total. + + The walk proceeds one tree level at a time and issues the listings of a + level concurrently, since a deep repo needs hundreds of round trips. + + ``prefetched`` is the already-truncated listing of ``root``, reused so + the caller's request is not repeated. Entries are de-duplicated by path. + Directories with more direct children than the cap cannot be enumerated + at all — those are reported through a warning and the returned list is + then knowingly incomplete. + """ + limit = REPO_FILES_TRUNCATION_LIMIT + collected: dict[str, dict] = {} + oversized: list[str] = [] + requests_made = 0 + progress_mark = 200 + budget_spent = False + + logger.info( + "Repo %s: file listing came back at the server cap of %d entries; walking the " + "tree per directory to recover the full list ...", + repo_id, + limit, + ) + + def absorb(entries: list[dict]) -> None: + for entry in entries: + path = _entry_path(entry) + if path: + collected.setdefault(path, entry) + + def fetch_level(roots: list[str | None], *, recursive: bool) -> dict[str | None, list[dict]]: + """List several directories at once, clamped to the request budget.""" + nonlocal requests_made, budget_spent, progress_mark + if not roots: + return {} + remaining = REPO_TREE_MAX_REQUESTS - requests_made + if remaining <= 0: + budget_spent = True + return {} + if len(roots) > remaining: + roots = roots[:remaining] + budget_spent = True + requests_made += len(roots) + + def one(subroot: str | None) -> list[dict]: + return self._list_files_page( + repo_id, + repo_type, + revision, + recursive=recursive, + root=subroot, + ) + + if len(roots) == 1: + listings = {roots[0]: one(roots[0])} + else: + workers = min(REPO_TREE_WALK_WORKERS, len(roots)) + with ThreadPoolExecutor(max_workers=workers) as pool: + futures = {pool.submit(one, subroot): subroot for subroot in roots} + listings = {futures[future]: future.result() for future in as_completed(futures)} + + if requests_made >= progress_mark: + logger.info( + "Repo %s: %d listings done, %d entries collected so far ...", + repo_id, + requests_made, + len(collected), + ) + progress_mark = requests_made + 200 + return listings + + frontier: list[str | None] = [root] + known: dict[str | None, list[dict]] = {} if prefetched is None else {root: prefetched} + + while frontier: + known.update(fetch_level([r for r in frontier if r not in known], recursive=True)) + + truncated: list[str | None] = [] + for subroot in frontier: + subtree = known.pop(subroot, None) + if subtree is None: + continue # budget ran out before this directory was reached + # A truncated subtree is still a valid prefix, so keep its entries + # and split the directory up to reach the rest. + absorb(subtree) + if len(subtree) >= limit: + truncated.append(subroot) + if not truncated or budget_spent: + break + + shallow_listings = fetch_level(truncated, recursive=False) + next_frontier: list[str | None] = [] + for subroot in truncated: + shallow = shallow_listings.get(subroot) + if shallow is None: + continue + absorb(shallow) + child_dirs = [path for path in map(_entry_path, filter(_is_dir_entry, shallow)) if path] + if len(shallow) >= limit or not child_dirs: + # This level alone exceeds the cap, or it has no sub-directories + # to split by: part of it stays invisible whatever we do. Still + # descend into the children we did see — that recovers strictly + # more than giving up here. + oversized.append(subroot or "/") + next_frontier.extend(child_dirs) + if budget_spent: + break + frontier = list(dict.fromkeys(next_frontier)) + + if budget_spent: + logger.warning( + "Repo %s: stopped walking the file tree after %d listings " + "(MODELSCOPE_REPO_TREE_MAX_REQUESTS=%d); the file list is incomplete.", + repo_id, + requests_made, + REPO_TREE_MAX_REQUESTS, + ) + if oversized: + shown = ", ".join(oversized[:5]) + (" ..." if len(oversized) > 5 else "") + logger.warning( + "Repo %s: %d director%s hold %d or more direct entries, which the server " + "cannot enumerate in full; the file list is incomplete: %s", + repo_id, + len(oversized), + "y" if len(oversized) == 1 else "ies", + limit, + shown, + ) + logger.info( + "Repo %s: collected %d entries from %d directory listings.", + repo_id, + len(collected), + requests_made, + ) + return list(collected.values()) + def list_dataset_files_paginated( self, repo_id: str, diff --git a/src/modelscope_hub/cli/main.py b/src/modelscope_hub/cli/main.py index 1bd198e..28cefcf 100644 --- a/src/modelscope_hub/cli/main.py +++ b/src/modelscope_hub/cli/main.py @@ -1,10 +1,17 @@ -"""Entry point for the ``modelscope-hub`` / ``ms-hub`` console scripts. +"""Entry point for every ModelScope console script. -This module is also reused by the umbrella ``modelscope`` package for its -``modelscope`` / ``ms`` commands. Because a single parser serves all four -aliases, the program name is derived from ``sys.argv[0]`` (rather than -hard-coded) so help/usage output shows whichever command was actually -invoked. +This package owns all four aliases -- ``modelscope``, ``ms``, +``modelscope-hub`` and ``ms-hub`` -- so exactly one distribution writes those +files and no cross-package overwrite can leave a user without a working CLI. +The umbrella ``modelscope`` SDK contributes its commands as plugins instead of +shipping a competing script. + +Because a single parser serves all four aliases, the program name, +``--version`` output and help epilog are derived from ``sys.argv[0]`` rather +than hard-coded, so each alias reports itself accurately. Only the two +``*-hub`` aliases are treated as hub-only; anything else (including +``python -m``) gets the brand view, which always names the hub version too and +so cannot misreport this package. Subcommands live in dedicated modules and are wired in via their :meth:`CLICommand.register` static method. :func:`run_cmd` is intentionally @@ -18,8 +25,10 @@ import importlib.metadata import logging import sys -from argparse import SUPPRESS +from argparse import SUPPRESS, Action from collections.abc import Sequence +from functools import cache +from pathlib import Path from .. import __version__ from ..constants import MODELSCOPE_ASCII @@ -60,20 +69,112 @@ _PLUGIN_GROUP = "modelscope_hub.cli_plugins" +# --------------------------------------------------------------------------- +# Invocation identity +# --------------------------------------------------------------------------- +# Aliases that address this package alone. The other two (``modelscope`` / +# ``ms``) front the whole brand, so they lead with the umbrella SDK version +# whenever it is installed alongside. +_HUB_ONLY_PROGS = frozenset({"modelscope-hub", "ms-hub"}) + +# Distribution name of the umbrella SDK that contributes the plugin commands. +_SDK_DIST = "modelscope" + + +def _invoked_as() -> str: + """Basename of the console script that started this process.""" + return Path(sys.argv[0]).name + + +@cache +def _sdk_version() -> str | None: + """Version of the umbrella ``modelscope`` SDK, or ``None`` when absent. + + Cached because resolving it walks ``sys.path`` looking for installed + distribution metadata, and both the version line and the help epilog ask + the same question. + """ + try: + return importlib.metadata.version(_SDK_DIST) + except importlib.metadata.PackageNotFoundError: + return None + + +def _version_text() -> str: + """Build the ``--version`` line for the alias actually invoked.""" + hub = f"modelscope-hub {__version__}" + if _invoked_as() in _HUB_ONLY_PROGS: + return hub + sdk = _sdk_version() + if sdk is None: + return f"{hub} (full ModelScope SDK not installed)" + return f"modelscope {sdk} ({hub})" + + +def _brand_epilog() -> str | None: + """Tell brand-alias users why SDK subcommands are missing, if they are. + + ``modelscope`` / ``ms`` ship with this package, so they exist even on a + lightweight hub-only install -- in which case the SDK subcommands are + genuinely absent and the user deserves to know how to get them. + """ + if _invoked_as() in _HUB_ONLY_PROGS or _sdk_version() is not None: + return None + return ( + "Commands from the full ModelScope SDK (pipeline, server, studio,\n" + "modelcard, ...) are not installed. Add them with:\n" + " pip install modelscope" + ) + + +class _VersionAction(argparse.Action): + """``--version`` resolved on use rather than at parser-build time. + + The stock ``version`` action needs its string up front, which would make + every invocation pay for a distribution-metadata lookup just to run an + unrelated subcommand. + """ + + def __init__( + self, + option_strings: Sequence[str], + dest: str = SUPPRESS, + default: str = SUPPRESS, + help: str | None = None, + ) -> None: + super().__init__( + option_strings=list(option_strings), + dest=dest, + default=default, + nargs=0, + help=help, + ) + + def __call__( + self, + parser: argparse.ArgumentParser, + namespace: argparse.Namespace, + values: object, + option_string: str | None = None, + ) -> None: + print(_version_text()) + parser.exit() + + def _build_parser() -> argparse.ArgumentParser: # ``prog`` is intentionally left unset so argparse derives it from - # ``sys.argv[0]``. The same parser backs the standalone - # ``modelscope-hub`` / ``ms-hub`` scripts and the umbrella - # ``modelscope`` / ``ms`` scripts, so help output reflects whichever - # command the user actually ran. + # ``sys.argv[0]``: one parser backs all four console scripts, so usage and + # help output must reflect whichever command the user actually ran. parser = argparse.ArgumentParser( description="ModelScope Hub command-line interface.", + epilog=_brand_epilog(), + formatter_class=argparse.RawDescriptionHelpFormatter, ) parser.add_argument( "-V", "--version", - action="version", - version=f"modelscope-hub {__version__}", + action=_VersionAction, + help="Show the version of the invoked command and exit.", ) parser.add_argument( "--token", @@ -175,20 +276,40 @@ def execute(self) -> None: # --------------------------------------------------------------------------- # Plugin discovery # --------------------------------------------------------------------------- -def _discover_plugins(subparsers) -> None: - """Discover CLI plugins registered via entry_points.""" +def _discover_plugins(subparsers: Action) -> None: + """Discover CLI plugins registered via entry_points. + + A plugin must never be able to break the whole CLI, so every failure is + contained. Failure severity differs though: a plugin that cannot even be + imported is usually an optional extra the user simply did not install + (``server`` needs the HTTP stack, for instance), which would make a warning + on every invocation pure noise -- whereas a name collision is always a + packaging mistake and stays silent forever unless we say so. + """ # ``entry_points(group=...)`` is available on all supported Pythons (3.10+). eps = importlib.metadata.entry_points(group=_PLUGIN_GROUP) + log = logging.getLogger(__name__) + # Live reference: it also grows as plugins register, so plugin-vs-plugin + # collisions are caught too, not just plugin-vs-built-in. + registered = getattr(subparsers, "choices", None) for ep in eps: try: cmd_cls = ep.load() + name = getattr(cmd_cls, "name", ep.name) + if registered is not None and name in registered: + log.warning( + "Skipping CLI plugin %r: command %r is already registered.", + ep.name, + name, + ) + continue if hasattr(cmd_cls, "register"): cmd_cls.register(subparsers) elif hasattr(cmd_cls, "define_args"): cmd_cls.define_args(subparsers) except Exception as exc: - logging.getLogger(__name__).debug("Failed to load CLI plugin %r: %s", ep.name, exc) + log.debug("Failed to load CLI plugin %r: %s", ep.name, exc) # --------------------------------------------------------------------------- diff --git a/src/modelscope_hub/constants.py b/src/modelscope_hub/constants.py index aec9061..203d98b 100644 --- a/src/modelscope_hub/constants.py +++ b/src/modelscope_hub/constants.py @@ -295,6 +295,30 @@ def _env_register( "API_MAX_RETRIES", ) +REPO_FILES_TRUNCATION_LIMIT: int = 3000 +"""Server-side hard cap on a single ``repo/files`` listing. + +``GET /api/v1/{type}s/{repo_id}/repo/files`` silently truncates the file tree at +this many entries: the response is ``HTTP 200`` with ``Success: true``, carries +neither ``TotalCount`` nor a truncation flag, and ignores every pagination +parameter. A listing whose length equals this limit therefore means "there may +be more", and the tree has to be re-enumerated with ``Root``-scoped requests. +""" + +REPO_TREE_MAX_REQUESTS: int = _env_int( + "MODELSCOPE_REPO_TREE_MAX_REQUESTS", + 5000, + "Request budget for walking a truncated repo file tree", + "Network", +) + +REPO_TREE_WALK_WORKERS: int = _env_int( + "MODELSCOPE_REPO_TREE_WALK_WORKERS", + 8, + "Concurrent listings when walking a truncated repo file tree", + "Network", +) + # --------------------------------------------------------------------------- # Endpoint switching # --------------------------------------------------------------------------- diff --git a/src/modelscope_hub/version.py b/src/modelscope_hub/version.py index a1bc874..42ac468 100644 --- a/src/modelscope_hub/version.py +++ b/src/modelscope_hub/version.py @@ -1,3 +1,3 @@ """Version information for modelscope_hub.""" -__version__ = "0.2.0+main" +__version__ = "0.3.0+main" diff --git a/tests/cli/test_entry_points.py b/tests/cli/test_entry_points.py new file mode 100644 index 0000000..d2f500b --- /dev/null +++ b/tests/cli/test_entry_points.py @@ -0,0 +1,326 @@ +"""Tests for console-script ownership and per-alias invocation identity. + +This package installs all four ModelScope console scripts, so one parser has to +answer "which command am I?" correctly: the program name in usage, the +``--version`` line, and whether to advertise the umbrella SDK. Split ownership +is what previously stranded users without a CLI at all -- two distributions +writing the same file meant upgrading either one could delete it -- so the +packaging declaration is pinned here alongside the runtime behaviour that +depends on it. +""" + +from __future__ import annotations + +import importlib.metadata +import logging +import sys +from pathlib import Path + +import pytest + +from modelscope_hub import __version__ +from modelscope_hub.cli import main as cli_main +from modelscope_hub.cli.main import _brand_epilog, _build_parser, _version_text, run_cmd + +# Every alias this distribution owns, and the single callable they all reach. +_EXPECTED_SCRIPTS = frozenset({"modelscope", "ms", "modelscope-hub", "ms-hub"}) +_ENTRY_TARGET = "modelscope_hub.cli.main:run_cmd" + +# Aliases that must never mention the umbrella SDK: they promise the hub only. +_HUB_ALIASES = ("ms-hub", "modelscope-hub") +# Aliases that front the whole brand and therefore report the SDK when present. +_BRAND_ALIASES = ("ms", "modelscope") + +_FAKE_SDK_VERSION = "9.9.9" + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- +@pytest.fixture(autouse=True) +def _clear_sdk_cache(): + """``_sdk_version`` caches for the process; each test needs a fresh answer.""" + cli_main._sdk_version.cache_clear() + yield + cli_main._sdk_version.cache_clear() + + +@pytest.fixture +def invoked_as(monkeypatch): + """Pretend the process was started by a particular console script.""" + + def _set(prog: str) -> None: + # Full path on purpose: the real argv[0] is absolute, and only the + # basename may drive the decision. + monkeypatch.setattr(sys, "argv", [f"/usr/local/bin/{prog}"]) + cli_main._sdk_version.cache_clear() + + return _set + + +@pytest.fixture +def sdk_absent(monkeypatch): + """Simulate a lightweight hub-only install (no ``modelscope`` distribution).""" + real = importlib.metadata.version + + def _fake(dist: str) -> str: + if dist == cli_main._SDK_DIST: + raise importlib.metadata.PackageNotFoundError(dist) + return real(dist) + + monkeypatch.setattr(importlib.metadata, "version", _fake) + + +@pytest.fixture +def sdk_present(monkeypatch): + """Simulate the umbrella SDK installed at a known version.""" + real = importlib.metadata.version + + def _fake(dist: str) -> str: + return _FAKE_SDK_VERSION if dist == cli_main._SDK_DIST else real(dist) + + monkeypatch.setattr(importlib.metadata, "version", _fake) + + +class _FakeEntryPoint: + """Minimal stand-in for :class:`importlib.metadata.EntryPoint`.""" + + def __init__(self, name: str, target: object, *, boom: bool = False) -> None: + self.name = name + self._target = target + self._boom = boom + + def load(self) -> object: + if self._boom: + raise ImportError("optional dependency missing") + return self._target + + +@pytest.fixture +def only_plugins(monkeypatch): + """Replace plugin discovery with a fixed set, so the env cannot leak in.""" + + def _install(*eps: _FakeEntryPoint) -> None: + monkeypatch.setattr( + importlib.metadata, + "entry_points", + lambda group=None: list(eps), + ) + + return _install + + +@pytest.fixture +def plugin_warnings(): + """Collect warnings emitted by the CLI module. + + The SDK keeps ``propagate = False`` on its own logger tree so it never + hijacks application logging -- which also means ``caplog``, wired to the + root logger, cannot see these records. Attach a handler to the emitting + logger instead, and pin its level so an ambient ``MODELSCOPE_LOG_LEVEL`` + cannot filter the record away. + """ + logger = logging.getLogger(cli_main.__name__) + records: list[logging.LogRecord] = [] + + class _Collector(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + records.append(record) + + handler = _Collector(level=logging.WARNING) + previous_level = logger.level + logger.addHandler(handler) + logger.setLevel(logging.WARNING) + try: + yield records + finally: + logger.removeHandler(handler) + logger.setLevel(previous_level) + + +# --------------------------------------------------------------------------- +# Packaging declaration +# --------------------------------------------------------------------------- +class TestPackagingDeclaration: + """The four aliases must be declared here, by exactly one distribution.""" + + @staticmethod + def _scripts() -> dict[str, str]: + # tomllib is stdlib from 3.11; the declaration it reads is not + # version-specific, so checking it on newer interpreters is enough. + tomllib = pytest.importorskip("tomllib", reason="stdlib tomllib requires Python 3.11+") + pyproject = Path(__file__).resolve().parents[2] / "pyproject.toml" + return tomllib.loads(pyproject.read_text(encoding="utf-8"))["project"]["scripts"] + + def test_declares_exactly_the_four_aliases(self): + assert set(self._scripts()) == set(_EXPECTED_SCRIPTS) + + def test_every_alias_shares_one_entry_point(self): + """Divergent targets would let the aliases drift apart in behaviour.""" + assert set(self._scripts().values()) == {_ENTRY_TARGET} + + +# --------------------------------------------------------------------------- +# Program name +# --------------------------------------------------------------------------- +class TestProgramName: + """Usage text must name the alias that ran, not a hard-coded default.""" + + @pytest.mark.parametrize("prog", sorted(_EXPECTED_SCRIPTS)) + def test_prog_follows_argv0(self, prog, invoked_as): + invoked_as(prog) + assert _build_parser().prog == prog + + +# --------------------------------------------------------------------------- +# Version line +# --------------------------------------------------------------------------- +class TestVersionLine: + """``--version`` must name the layer the user actually asked about.""" + + @pytest.mark.parametrize("prog", _HUB_ALIASES) + def test_hub_alias_reports_only_this_package(self, prog, invoked_as, sdk_present): + """Even with the SDK installed, a ``*-hub`` alias speaks for the hub.""" + invoked_as(prog) + assert _version_text() == f"modelscope-hub {__version__}" + + @pytest.mark.parametrize("prog", _BRAND_ALIASES) + def test_brand_alias_leads_with_sdk_version(self, prog, invoked_as, sdk_present): + invoked_as(prog) + text = _version_text() + assert text.startswith(f"modelscope {_FAKE_SDK_VERSION}") + assert f"modelscope-hub {__version__}" in text + + @pytest.mark.parametrize("prog", _BRAND_ALIASES) + def test_brand_alias_flags_missing_sdk(self, prog, invoked_as, sdk_absent): + """A brand alias on a hub-only install must not imply the SDK is there.""" + invoked_as(prog) + text = _version_text() + assert text.startswith(f"modelscope-hub {__version__}") + assert "not installed" in text + + @pytest.mark.parametrize("prog", _HUB_ALIASES) + def test_hub_alias_stays_quiet_about_missing_sdk(self, prog, invoked_as, sdk_absent): + """Nothing is missing from a hub-only alias, so there is nothing to say.""" + invoked_as(prog) + assert _version_text() == f"modelscope-hub {__version__}" + + def test_version_flag_prints_to_stdout_and_exits_zero(self, invoked_as, capsys): + invoked_as("ms-hub") + with pytest.raises(SystemExit) as exc_info: + run_cmd(["--version"]) + assert exc_info.value.code == 0 + assert f"modelscope-hub {__version__}" in capsys.readouterr().out + + def test_version_is_not_resolved_while_building_the_parser(self, invoked_as, monkeypatch): + """Metadata lookups must not be charged to unrelated subcommands. + + ``--version`` is the only consumer of the SDK version, so building the + parser for e.g. ``download`` should never pay for it. + """ + invoked_as("ms-hub") + + def _explode(dist: str) -> str: + raise AssertionError(f"unexpected metadata lookup for {dist!r}") + + monkeypatch.setattr(importlib.metadata, "version", _explode) + assert _build_parser() is not None + + +# --------------------------------------------------------------------------- +# Help epilog +# --------------------------------------------------------------------------- +class TestBrandEpilog: + """Brand aliases exist on a hub-only install; help must explain the gap.""" + + @pytest.mark.parametrize("prog", _BRAND_ALIASES) + def test_epilog_offers_install_hint_when_sdk_absent(self, prog, invoked_as, sdk_absent): + invoked_as(prog) + epilog = _brand_epilog() + assert epilog is not None + assert "pip install modelscope" in epilog + + @pytest.mark.parametrize("prog", _BRAND_ALIASES) + def test_no_epilog_when_sdk_present(self, prog, invoked_as, sdk_present): + """Nothing is missing, so the hint would be noise.""" + invoked_as(prog) + assert _brand_epilog() is None + + @pytest.mark.parametrize("prog", _HUB_ALIASES) + def test_no_epilog_for_hub_alias(self, prog, invoked_as, sdk_absent): + """``ms-hub`` does exactly what it advertises; no SDK is implied.""" + invoked_as(prog) + assert _brand_epilog() is None + + def test_epilog_reaches_help_output(self, invoked_as, sdk_absent, capsys): + invoked_as("modelscope") + with pytest.raises(SystemExit): + run_cmd(["--help"]) + assert "pip install modelscope" in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# Plugin discovery +# --------------------------------------------------------------------------- +class TestPluginDiscovery: + """A contributed plugin must never shadow or break the built-in commands.""" + + def test_duplicate_name_is_skipped_with_a_warning(self, only_plugins, plugin_warnings): + """Silently dropping a collision leaves a packaging bug undiagnosable.""" + + class _Shadow: + name = "download" # collides with a built-in command + + @staticmethod + def register(subparsers): + raise AssertionError("collision must be caught before register()") + + only_plugins(_FakeEntryPoint("shadow", _Shadow)) + + parser = _build_parser() + + assert any("already registered" in record.getMessage() for record in plugin_warnings) + # The built-in survives intact. + args = parser.parse_args(["download", "owner/name"]) + assert args.repo_id == "owner/name" + + def test_unimportable_plugin_does_not_break_the_cli(self, only_plugins, plugin_warnings): + """Optional extras are legitimately absent, so this must stay quiet.""" + only_plugins(_FakeEntryPoint("broken", None, boom=True)) + + parser = _build_parser() + + assert parser is not None + assert plugin_warnings == [] + + def test_plugin_registers_its_command(self, only_plugins): + recorded: list[str] = [] + + class _Extra: + name = "brand-new" + + @staticmethod + def register(subparsers): + recorded.append("registered") + subparsers.add_parser(_Extra.name) + + only_plugins(_FakeEntryPoint("extra", _Extra)) + + parser = _build_parser() + + assert recorded == ["registered"] + assert "brand-new" in parser.parse_args(["brand-new"]).command + + def test_plugin_using_define_args_is_supported(self, only_plugins): + """Legacy plugins expose ``define_args`` instead of ``register``.""" + + class _Legacy: + name = "legacy-cmd" + + @staticmethod + def define_args(subparsers): + subparsers.add_parser(_Legacy.name) + + only_plugins(_FakeEntryPoint("legacy", _Legacy)) + + assert _build_parser().parse_args(["legacy-cmd"]).command == "legacy-cmd" diff --git a/tests/test_repo_files_truncation.py b/tests/test_repo_files_truncation.py new file mode 100644 index 0000000..088b1e5 --- /dev/null +++ b/tests/test_repo_files_truncation.py @@ -0,0 +1,243 @@ +"""Unit tests for the client-side mitigation of the ``repo/files`` entry cap. + +The server truncates every ``repo/files`` listing at +``REPO_FILES_TRUNCATION_LIMIT`` entries, returns ``HTTP 200`` with no truncation +marker, and ignores all pagination parameters. ``LegacyClient.list_repo_files`` +therefore treats a listing that lands exactly on the cap as "there may be more" +and re-enumerates the tree with ``Root``-scoped requests. + +Network-free: ``_list_files_page`` is replaced by an in-memory repo that mimics +the endpoint, cap included. The cap is patched down to a small number so the +fixtures stay readable. +""" + +from __future__ import annotations + +from unittest import mock + +import pytest + +from modelscope_hub import _legacy_api +from modelscope_hub._legacy_api import LegacyClient + +CAP = 5 + + +def _blob(path: str) -> dict: + return {"Path": path, "Name": path.rsplit("/", 1)[-1], "Type": "blob", "Size": 1} + + +def _tree(path: str) -> dict: + return {"Path": path, "Name": path.rsplit("/", 1)[-1], "Type": "tree", "Size": 0} + + +class _FakeRepo: + """Stand-in for ``repo/files``, including its silent truncation.""" + + def __init__(self, blobs: list[str], cap: int = CAP) -> None: + self.blobs = sorted(blobs) + self.cap = cap + self.calls: list[tuple[str | None, bool]] = [] + + def _entries(self, root: str | None, recursive: bool) -> list[dict]: + prefix = f"{root}/" if root else "" + found: dict[str, dict] = {} + for blob in self.blobs: + if not blob.startswith(prefix): + continue + rest = blob[len(prefix) :] + if "/" not in rest: + found[blob] = _blob(blob) + elif recursive: + parts = rest.split("/") + for depth in range(1, len(parts)): + nested = prefix + "/".join(parts[:depth]) + found[nested] = _tree(nested) + found[blob] = _blob(blob) + else: + child = prefix + rest.split("/")[0] + found[child] = _tree(child) + return list(found.values()) + + def listing(self, repo_id, repo_type, revision, *, recursive, root=None) -> list[dict]: + """Signature-compatible replacement for ``_list_files_page``.""" + self.calls.append((root, recursive)) + return self._entries(root, recursive)[: self.cap] + + def full_paths(self, recursive: bool = True) -> set[str]: + """Every path the endpoint would expose if it did not truncate.""" + return {_legacy_api._entry_path(e) for e in self._entries(None, recursive)} + + +@pytest.fixture +def client() -> LegacyClient: + return LegacyClient(token=None, endpoint="https://example.com") + + +def _install(repo: _FakeRepo): + """Patch the transport and shrink the cap to the fake repo's cap.""" + return ( + mock.patch.object(LegacyClient, "_list_files_page", side_effect=repo.listing), + mock.patch.object(_legacy_api, "REPO_FILES_TRUNCATION_LIMIT", repo.cap), + ) + + +class TestFastPath: + def test_small_repo_takes_exactly_one_request(self, client): + repo = _FakeRepo(["config.json", "model.safetensors", "sub/extra.bin"]) + transport, cap = _install(repo) + with transport, cap: + out = client.list_repo_files("owner/name", "model") + + assert len(repo.calls) == 1 + assert repo.calls[0] == (None, True) + assert {_legacy_api._entry_path(e) for e in out} == { + "config.json", + "model.safetensors", + "sub", + "sub/extra.bin", + } + + def test_listing_just_below_cap_is_not_rewalked(self, client): + # cap - 1 entries: a complete tree that must not trigger the fallback. + repo = _FakeRepo([f"f{i}.bin" for i in range(CAP - 1)]) + transport, cap = _install(repo) + with transport, cap: + out = client.list_repo_files("owner/name", "model") + + assert len(repo.calls) == 1 + assert len(out) == CAP - 1 + + +class TestTruncatedTreeIsRewalked: + def _repo(self) -> _FakeRepo: + # Root recursive listing hits the cap, so the tree must be walked: + # 3 top-level blobs + two directories holding 4 blobs each. + return _FakeRepo( + [ + "README.md", + "config.json", + "model.safetensors", + *[f"weights/part{i}.bin" for i in range(4)], + *[f"tokenizer/vocab{i}.txt" for i in range(4)], + ] + ) + + def test_full_tree_is_recovered(self, client): + repo = self._repo() + transport, cap = _install(repo) + with transport, cap: + out = client.list_repo_files("owner/name", "model") + + got = {_legacy_api._entry_path(e) for e in out} + assert got == repo.full_paths() + # The truncated single call would have returned only `cap` entries. + assert len(got) > repo.cap + + def test_walk_scopes_requests_with_root(self, client): + repo = self._repo() + transport, cap = _install(repo) + with transport, cap: + client.list_repo_files("owner/name", "model") + + roots_visited = {root for root, _ in repo.calls} + assert {"weights", "tokenizer"} <= roots_visited + # The caller's initial listing is reused, never repeated. + assert [call for call in repo.calls if call == (None, True)] == [(None, True)] + # The root level is listed shallowly to discover the child directories. + assert (None, False) in repo.calls + + def test_entries_are_deduplicated(self, client): + repo = self._repo() + transport, cap = _install(repo) + with transport, cap: + out = client.list_repo_files("owner/name", "model") + + paths = [_legacy_api._entry_path(e) for e in out] + assert len(paths) == len(set(paths)) + + def test_nested_oversized_subtree_is_split_further(self, client): + # `deep` alone exceeds the cap and only resolves via its children. + repo = _FakeRepo( + [ + "README.md", + *[f"deep/a/f{i}.bin" for i in range(4)], + *[f"deep/b/f{i}.bin" for i in range(4)], + ] + ) + transport, cap = _install(repo) + with transport, cap: + out = client.list_repo_files("owner/name", "model") + + assert {_legacy_api._entry_path(e) for e in out} == repo.full_paths() + assert {"deep", "deep/a", "deep/b"} <= {root for root, _ in repo.calls} + + +class TestIncompleteResultsAreReported: + @staticmethod + def _warnings(warn_mock) -> list[str]: + return [call.args[0] for call in warn_mock.call_args_list] + + def test_flat_oversized_directory_warns(self, client): + # A single directory with more direct blob children than the cap cannot + # be enumerated: no sub-directories exist to scope requests by. + repo = _FakeRepo([f"flat/f{i}.bin" for i in range(CAP + 3)]) + transport, cap = _install(repo) + with transport, cap, mock.patch.object(_legacy_api.logger, "warning") as warn: + out = client.list_repo_files("owner/name", "model") + + assert len(out) <= repo.cap + 1 # partial, best effort + assert any("incomplete" in message for message in self._warnings(warn)) + + def test_non_recursive_listing_at_cap_warns_without_walking(self, client): + repo = _FakeRepo([f"f{i}.bin" for i in range(CAP + 3)]) + transport, cap = _install(repo) + with transport, cap, mock.patch.object(_legacy_api.logger, "warning") as warn: + out = client.list_repo_files("owner/name", "model", recursive=False) + + assert len(repo.calls) == 1 # no fallback walk for a shallow listing + assert len(out) == repo.cap + assert any("incomplete" in message for message in self._warnings(warn)) + + def test_request_budget_stops_the_walk(self, client): + # Root holds 3 directories (a shallow listing fits under the cap), but the + # recursive listing does not — so the walk starts and then runs out of budget. + repo = _FakeRepo([f"d{d}/f{i}.bin" for d in range(3) for i in range(4)]) + transport, cap = _install(repo) + with ( + transport, + cap, + mock.patch.object(_legacy_api, "REPO_TREE_MAX_REQUESTS", 3), + mock.patch.object(_legacy_api.logger, "warning") as warn, + ): + out = client.list_repo_files("owner/name", "model") + + assert len(repo.calls) <= 1 + 3 # initial detection listing + the walk's budget + assert len(out) > 0 # whatever was collected is still returned + assert any("MODELSCOPE_REPO_TREE_MAX_REQUESTS" in message for message in self._warnings(warn)) + + +class TestDatasetRouting: + def test_recursive_dataset_listing_uses_the_paginated_endpoint(self, client): + pages = [{"Path": "data/train.csv", "Type": "blob", "Size": 1}] + with ( + mock.patch.object(LegacyClient, "list_dataset_files_paginated", return_value=pages) as paged, + mock.patch.object(LegacyClient, "_list_files_page") as single, + ): + out = client.list_repo_files("owner/ds", "dataset", revision="v1", root="data") + + assert out == pages + single.assert_not_called() + _, kwargs = paged.call_args + assert kwargs["revision"] == "v1" + assert kwargs["root_path"] == "data" + + def test_non_recursive_dataset_listing_stays_single_page(self, client): + with ( + mock.patch.object(LegacyClient, "list_dataset_files_paginated") as paged, + mock.patch.object(LegacyClient, "_list_files_page", return_value=[]) as single, + ): + client.list_repo_files("owner/ds", "dataset", recursive=False) + + paged.assert_not_called() + single.assert_called_once()