Add sp sample download

AGENTS.md

@@ -137,7 +137,8 @@ sp run ls --limit 5   | jq -r '.data[]     | "\(.run_id) \(.platform) \(.status)
 
 Exit codes to branch on: `0` ok, `3` unreachable, `4` not found, `5` validation,
 `6` auth, `7` rate limited, `8` conflict, `9` `run wait` timed out, `10` `run
-wait` broke before it could finish. Under `run wait`, `1` means a run failed or
+wait` broke before it could finish, `11` `sample download` received bytes that
+do not match the sample's SHA-256 (nothing is written; retry once). Under `run wait`, `1` means a run failed or
 was canceled and nothing else does, so `1` is a verdict about the code while
 `9`/`10` say only that the platform misbehaved — block on the first, retry the
 others. **`2` means you got the command wrong** — an unknown flag, a missing
@@ -172,6 +173,11 @@ when stdout is not a terminal, so JSON on stdout is always parseable.
   fields. An unmatched line reports `level=info`, `source=web`. Do not treat
   `[ERROR]` in a build log as authoritative — ffmpeg banner output shows up
   there too.
+- **`sample download` fetches from a signed storage URL**, not the API, so the
+  token is never sent there and the URL expires after a while: a
+  `download_failed` with `http_status` 400/403 means run it again. It refuses to
+  overwrite an existing file without `--force` (exit 8). Its `-d/--dest` is not
+  the global `-o`, which stays the output format.
 - **`include_stack=true` on infra errors needs admin or contributor role** and
   403s otherwise.
 

README.md

@@ -138,6 +138,7 @@ sp run ls --pr 2309                  # ... just one pull request's runs
 sp run wait 9412 9413                # block until they finish; exit code is the verdict
 sp run create --commit <sha> --platform linux --repository owner/repo
 sp sample ls / show / details <id>   # media samples
+sp sample download 188 -d ./media    # fetch the file, verified against its SHA-256
 sp regression ls / show <id>         # regression-test definitions
 sp category ls                       # categories, with test counts
 ```

sp_cli/commands/sample.py

@@ -1,17 +1,22 @@
-"""``sp sample`` — list and inspect media samples."""
+"""``sp sample`` — list, inspect and download media samples."""
 
-from typing import Optional
+import os
+from typing import Any, Dict, Optional, Tuple
 
 import click
 
-from sp_cli.constants import (MAX_OFFSET, MAX_PAGE_LIMIT, PLATFORMS,
+from sp_cli.client import ApiError
+from sp_cli.constants import (EXIT_CHECKSUM_MISMATCH, MAX_OFFSET,
+                              MAX_PAGE_LIMIT, PLATFORMS,
                               SAMPLE_CATALOG_STATUSES, SAMPLE_STATUSES)
+from sp_cli.download import ChecksumMismatch, download_to
+from sp_cli.output import render, render_error
 from sp_cli.runner import clean_params, fetch_and_render
 
 
 @click.group()
 def sample() -> None:
-    """List and inspect media samples."""
+    """List, inspect and download media samples."""
 
 
 @sample.command('ls')
@@ -84,3 +89,127 @@ def sample_history(ctx: click.Context, sample_id: int, platform: Optional[str],
                            'created_after': created_after, 'created_before': created_before,
                            'limit': limit, 'offset': offset})
     fetch_and_render(ctx, f'/samples/{sample_id}/history', params)
+
+
+@sample.command('download')
+@click.argument('sample_id', type=int)
+@click.option('--dest', '-d', 'dest', type=click.Path(file_okay=True, dir_okay=True), default=None,
+              help='Directory to save into, or a file path. Default: the current directory, '
+                   'under the name the platform stores it as.')
+@click.option('--media-info', 'media_info', is_flag=True, default=False,
+              help='Download the MediaInfo XML instead of the sample itself.')
+@click.option('--extra-file', 'extra_file_id', type=click.IntRange(1), default=None,
+              help='Download this accompanying file (an id from `sp sample details`) instead.')
+@click.option('--force', is_flag=True, default=False, help='Overwrite an existing file.')
+@click.pass_context
+def sample_download(ctx: click.Context, sample_id: int, dest: Optional[str], media_info: bool,
+                    extra_file_id: Optional[int], force: bool) -> None:
+    """Download a sample's media file, verifying it against its SHA-256.
+
+    Samples are stored under their SHA-256, so the file is checked against the
+    platform's record before it is moved into place. A mismatch leaves nothing
+    behind and exits 11. The MediaInfo XML and extra files have no recorded
+    digest and are written unverified (`verified: false`).
+
+    The API hands out a short-lived signed storage URL; the bytes come straight
+    from storage and the platform token is never sent there. A byte counter is
+    drawn on stderr when it is a terminal, in either output mode.
+    """
+    if media_info and extra_file_id is not None:
+        raise click.UsageError('--media-info and --extra-file are mutually exclusive.')
+
+    client = ctx.obj['client']
+    output = ctx.obj['output']
+    try:
+        kind, located, expected_sha = _locate(client, sample_id, media_info, extra_file_id)
+        url = located.get('download_url')
+        if not url:
+            raise ApiError(
+                'not_downloadable',
+                'The platform has this file only on its own disk (storage_status='
+                f"{located.get('storage_status')}), so there is no URL to fetch it from.",
+                404, {'storage_status': located.get('storage_status')})
+        # basename: the name comes from the server and must not pick the directory.
+        name = os.path.basename(str(located.get('filename') or f'sample-{sample_id}'))
+        path = _resolve_destination(dest, name)
+        if os.path.exists(path) and not force:
+            raise ApiError('file_exists', f'{path} already exists; pass --force to overwrite it.',
+                           409, {'path': path})
+        written = download_to(url, path, expected_sha256=expected_sha, timeout=client.timeout)
+    except ChecksumMismatch as mismatch:
+        render_error(ApiError(
+            'checksum_mismatch',
+            'The downloaded file does not match the SHA-256 the platform has on record; '
+            'nothing was written. Retry once; a repeat means the stored copy is damaged.',
+            None, {'expected': mismatch.expected, 'actual': mismatch.actual}), output)
+        raise SystemExit(EXIT_CHECKSUM_MISMATCH)
+    except ApiError as error:
+        render_error(error, output)
+        raise SystemExit(error.exit_code)
+    except OSError as exc:
+        render_error(ApiError('write_failed', f'Could not write the file: {exc}'), output)
+        raise SystemExit(1)
+
+    payload: Dict[str, Any] = {
+        'sample_id': sample_id,
+        'file': kind,
+        'path': os.path.abspath(path),
+        'bytes': written['bytes'],
+        'sha256': written['sha256'],
+        'verified': expected_sha is not None,
+        'storage_status': located.get('storage_status'),
+    }
+    if extra_file_id is not None:
+        payload['extra_file_id'] = extra_file_id
+    render(payload, output, ctx.obj.get('color', False))
+
+
+def _locate(client: Any, sample_id: int, media_info: bool,
+            extra_file_id: Optional[int]) -> Tuple[str, Dict[str, Any], Optional[str]]:
+    """
+    Ask the API where the requested file is, and what it should hash to.
+
+    :param client: The configured API client.
+    :type client: Any
+    :param sample_id: Sample the file belongs to.
+    :type sample_id: int
+    :param media_info: Whether the MediaInfo XML was asked for.
+    :type media_info: bool
+    :param extra_file_id: Accompanying file asked for, if any.
+    :type extra_file_id: Optional[int]
+    :return: ``(kind, download envelope, expected sha256 or None)``.
+    :rtype: Tuple[str, Dict[str, Any], Optional[str]]
+    """
+    if media_info:
+        return 'media_info', client.get(f'/samples/{sample_id}/media-info/download'), None
+    if extra_file_id is not None:
+        located = client.get(f'/samples/{sample_id}/extra-files/{extra_file_id}/download')
+        return 'extra_file', located, None
+    # The digest comes from the sample record, not the download envelope, so a
+    # wrong URL cannot vouch for itself.
+    expected = client.get(f'/samples/{sample_id}').get('sha')
+    return 'sample', client.get(f'/samples/{sample_id}/download'), expected
+
+
+def _resolve_destination(dest: Optional[str], name: str) -> str:
+    """
+    Turn ``--dest`` into the file path to write.
+
+    :param dest: The option as given: a directory, a file path, or ``None``.
+    :type dest: Optional[str]
+    :param name: The platform's name for the file.
+    :type name: str
+    :raises click.BadParameter: when the parent directory does not exist.
+    :return: The file path to write.
+    :rtype: str
+    """
+    if dest is None:
+        return name
+    if os.path.isdir(dest):
+        return os.path.join(dest, name)
+    if dest.endswith(os.sep) or (os.altsep and dest.endswith(os.altsep)):
+        raise click.BadParameter(f'directory {dest} does not exist.', param_hint='--dest')
+    parent = os.path.dirname(os.path.abspath(dest))
+    if not os.path.isdir(parent):
+        raise click.BadParameter(f'directory {parent} does not exist.', param_hint='--dest')
+    return dest

sp_cli/constants.py

@@ -45,6 +45,12 @@
 #: do map to a code of their own (3 through 8) keep it.
 EXIT_WAIT_ABORTED = 10
 
+#: ``sp sample download`` exits with this when the received bytes do not hash to
+#: the SHA-256 the platform has on record. Its own code, because a corrupt
+#: transfer is neither "not found" nor a generic failure: retrying may fix it,
+#: and a repeat mismatch means the stored sample itself is damaged.
+EXIT_CHECKSUM_MISMATCH = 11
+
 #: How many failing tests ``sp run report`` lists per reference in table mode.
 #: The counts are the answer and the rows are evidence for it; JSON output is
 #: never truncated, so nothing is lost to a script.

sp_cli/download.py

@@ -0,0 +1,160 @@
+"""Stream a file from a signed storage URL to disk, hashing it on the way.
+
+The API never serves sample bytes itself: it hands out a short-lived signed URL
+to the storage bucket (see ``/samples/{id}/download``). This module fetches that
+URL. It deliberately does **not** go through :class:`sp_cli.client.ApiClient`:
+that client attaches the bearer token to every request, and a signed URL points
+at a third-party host that has no business seeing the platform credential.
+"""
+
+import hashlib
+import os
+import tempfile
+from typing import Any, Dict, Optional
+
+import requests  # type: ignore[import-untyped]
+
+from sp_cli.client import ApiError
+from sp_cli.progress import Spinner
+
+#: Bytes read per chunk. Large enough that hashing, not syscalls, dominates.
+_CHUNK_SIZE = 1024 * 1024
+
+
+class ChecksumMismatch(Exception):
+    """Raised when a downloaded file does not hash to the value the API reported."""
+
+    def __init__(self, expected: str, actual: str) -> None:
+        """
+        Record both digests so the caller can report them.
+
+        :param expected: The SHA-256 the platform has on record.
+        :type expected: str
+        :param actual: The SHA-256 of the bytes actually received.
+        :type actual: str
+        """
+        super().__init__(f'SHA-256 mismatch: expected {expected}, got {actual}')
+        self.expected = expected
+        self.actual = actual
+
+
+def download_to(url: str, destination: str, expected_sha256: Optional[str] = None,
+                timeout: int = 30, show_progress: bool = True) -> Dict[str, Any]:
+    """
+    Download ``url`` into ``destination``, verifying it when a digest is known.
+
+    The bytes go to a temporary file beside the destination and are moved into
+    place only once complete and verified, so an interrupted or corrupt transfer
+    never leaves a file that looks like a finished sample.
+
+    :param url: Signed storage URL, fetched without any platform credential.
+    :type url: str
+    :param destination: Final file path.
+    :type destination: str
+    :param expected_sha256: Hex digest the file must match, or ``None`` to skip.
+    :type expected_sha256: Optional[str]
+    :param timeout: Connect/read timeout in seconds (per read, not total).
+    :type timeout: int
+    :param show_progress: Whether to draw a byte counter on stderr.
+    :type show_progress: bool
+    :raises ApiError: when storage cannot be reached or refuses the request.
+    :raises ChecksumMismatch: when the received bytes do not match.
+    :return: ``{bytes, sha256}`` for the file written.
+    :rtype: Dict[str, Any]
+    """
+    directory = os.path.dirname(os.path.abspath(destination))
+    digest = hashlib.sha256()
+    received = 0
+    fd, partial = tempfile.mkstemp(prefix='.sp-download-', suffix='.part', dir=directory)
+    try:
+        with os.fdopen(fd, 'wb') as handle:
+            try:
+                response = requests.get(url, stream=True, timeout=timeout)
+            except requests.RequestException as exc:
+                raise ApiError('connection_error', f'Could not reach storage: {exc}')
+            with response:
+                if response.status_code >= 400:
+                    # Not the API's error envelope: storage answers in its own
+                    # format. An expired signature is a 400 or 403 here.
+                    raise ApiError(
+                        'download_failed',
+                        f'Storage refused the download with HTTP {response.status_code}; '
+                        'the signed URL may have expired, so run the command again.',
+                        None, {'http_status': response.status_code})
+                total = _content_length(response)
+                label = os.path.basename(destination)
+                with Spinner(_progress_label(label, 0, total), enabled=show_progress) as spinner:
+                    try:
+                        for chunk in response.iter_content(chunk_size=_CHUNK_SIZE):
+                            if not chunk:
+                                continue
+                            handle.write(chunk)
+                            digest.update(chunk)
+                            received += len(chunk)
+                            spinner.label = _progress_label(label, received, total)
+                    except requests.RequestException as exc:
+                        raise ApiError('connection_error',
+                                       f'Download interrupted after {received} bytes: {exc}')
+
+        actual = digest.hexdigest()
+        if expected_sha256 is not None and actual != expected_sha256.lower():
+            raise ChecksumMismatch(expected_sha256, actual)
+        # mkstemp creates the file 0600; give it the mode any other new file gets.
+        os.chmod(partial, 0o666 & ~_umask())
+        os.replace(partial, destination)
+    except BaseException:
+        if os.path.exists(partial):
+            os.remove(partial)
+        raise
+
+    return {'bytes': received, 'sha256': actual}
+
+
+def _umask() -> int:
+    """
+    Read the process umask, which can only be done by setting it.
+
+    :return: The current umask.
+    :rtype: int
+    """
+    current = os.umask(0)
+    os.umask(current)
+    return current
+
+
+def _content_length(response: Any) -> Optional[int]:
+    """
+    Read the advertised size, if the server sent a usable one.
+
+    :param response: The streaming response.
+    :type response: Any
+    :return: The size in bytes, or ``None`` when absent or malformed.
+    :rtype: Optional[int]
+    """
+    headers = getattr(response, 'headers', None) or {}
+    value = headers.get('Content-Length')
+    if value is None:
+        return None
+    try:
+        return int(value)
+    except (TypeError, ValueError):
+        return None
+
+
+def _progress_label(name: str, received: int, total: Optional[int]) -> str:
+    """
+    Format the progress line shown beside the spinner.
+
+    :param name: File being written.
+    :type name: str
+    :param received: Bytes written so far.
+    :type received: int
+    :param total: Expected size, when known.
+    :type total: Optional[int]
+    :return: e.g. ``abc.ts 12.0/340.5 MiB``.
+    :rtype: str
+    """
+    mib = 1024 * 1024
+    if total:
+        return f'{name} {received / mib:.1f}/{total / mib:.1f} MiB'
+    return f'{name} {received / mib:.1f} MiB'

sp_cli/progress.py

@@ -4,6 +4,10 @@
 output is not a live terminal, whenever ``NO_COLOR`` is set, and always in JSON
 mode. It draws on **stderr**, so even when it does run it cannot contaminate the
 payload a caller is parsing on stdout.
+
+The one exception to the JSON-mode rule is ``sp sample download``: a transfer
+of hundreds of megabytes with no sign of life reads as a hang, and the TTY check
+alone already keeps the counter away from any script or agent.
 """
 
 import itertools