Skip to content

Commit 76f1948

Browse files
minitrigaclaude
andcommitted
Address cubic review: classify partial probe failures and download errors as network
- `_detect_item_type`: a transport failure on either probe (when no 200 winner exists) now raises `network`, not `not-found`. Mixing a 404 on one endpoint with a 5xx or transport failure on the other previously fell through to `not-found`, which masked real connectivity problems. Identified by cubic. - `get`: wrap the post-detect download calls so `httpx.HTTPError` (transport failures and 5xx via `raise_for_status`) raises `_fail("network", ...)` (exit 2) instead of being swallowed by the default `@catch_exception` exit 1. Identified by cubic. - Update quickstart success example to match the actual `Downloaded schema …` output. - Add three tests: partial probe failure -> network, versioned download network error, and `--collection` flag network error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 67dd3cf commit 76f1948

3 files changed

Lines changed: 69 additions & 12 deletions

File tree

dev/specs/001-marketplace-api-update/quickstart.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ This must exercise the same auto-detection behaviour against the overridden host
6666
For a schema:
6767

6868
```text
69-
Downloaded acme/network-base v1.2.0 -> schemas/network-base.yml
69+
Downloaded schema acme/network-base v1.2.0 -> schemas/network-base.yml
7070
```
7171

7272
For a collection:

infrahub_sdk/ctl/marketplace.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ def is_transport_failure(r: object) -> bool:
105105
return True
106106
return isinstance(r, httpx.Response) and r.status_code >= 500
107107

108-
if is_transport_failure(schema_resp) and is_transport_failure(collection_resp):
108+
if is_transport_failure(schema_resp) or is_transport_failure(collection_resp):
109109
raise _fail(
110110
"network",
111111
f"Could not reach marketplace at {base_url}. Check your connection or --marketplace-url.",
@@ -279,13 +279,16 @@ async def get(
279279
else:
280280
item_type = "schema"
281281

282-
if item_type == "collection":
283-
if version:
284-
_status(stdout).print("[yellow]Warning: --version is ignored when downloading a collection.")
285-
await _download_collection(
286-
client, base_url, namespace, name, output_dir, stdout=stdout, prefetched=prefetched
287-
)
288-
else:
289-
await _download_schema(
290-
client, base_url, namespace, name, version, output_dir, stdout=stdout, prefetched=prefetched
291-
)
282+
try:
283+
if item_type == "collection":
284+
if version:
285+
_status(stdout).print("[yellow]Warning: --version is ignored when downloading a collection.")
286+
await _download_collection(
287+
client, base_url, namespace, name, output_dir, stdout=stdout, prefetched=prefetched
288+
)
289+
else:
290+
await _download_schema(
291+
client, base_url, namespace, name, version, output_dir, stdout=stdout, prefetched=prefetched
292+
)
293+
except httpx.HTTPError as exc:
294+
raise _fail("network", f"Marketplace request failed: {exc}") from exc

tests/unit/ctl/test_marketplace_app.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,60 @@ def test_download_collection_with_skipped(httpx_mock: HTTPXMock, tmp_path: Path)
449449
assert "1/2 schemas downloaded" in result.stdout
450450

451451

452+
def test_autodetect_partial_probe_failure_is_network(httpx_mock: HTTPXMock, tmp_path: Path) -> None:
453+
"""Schema 404 + collection transport failure should be classified as network, not not-found."""
454+
httpx_mock.add_response(
455+
method="GET",
456+
url="https://marketplace.infrahub.app/api/v1/schemas/acme/foo/download",
457+
status_code=404,
458+
json={"detail": "Schema not found"},
459+
)
460+
httpx_mock.add_exception(
461+
httpx.ConnectError("connection refused"),
462+
url="https://marketplace.infrahub.app/api/v1/collections/acme/foo/download",
463+
)
464+
result = runner.invoke(app, ["get", "acme/foo", "-o", str(tmp_path)])
465+
466+
assert result.exit_code == 2
467+
assert "Could not reach marketplace" in result.stdout
468+
469+
470+
def test_versioned_download_network_error(httpx_mock: HTTPXMock, tmp_path: Path) -> None:
471+
"""A network failure during the post-detect versioned fetch should exit with code 2."""
472+
httpx_mock.add_response(
473+
method="GET",
474+
url="https://marketplace.infrahub.app/api/v1/schemas/acme/network-base/download",
475+
text=SCHEMA_YAML,
476+
headers={"x-schema-version": "1.2.0"},
477+
)
478+
httpx_mock.add_response(
479+
method="GET",
480+
url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download",
481+
status_code=404,
482+
json={"detail": "Collection not found"},
483+
)
484+
httpx_mock.add_exception(
485+
httpx.ConnectError("connection refused"),
486+
url="https://marketplace.infrahub.app/api/v1/schemas/acme/network-base/versions/1.0.0/download",
487+
)
488+
result = runner.invoke(app, ["get", "acme/network-base", "-v", "1.0.0", "-o", str(tmp_path)])
489+
490+
assert result.exit_code == 2
491+
assert "Marketplace request failed" in result.stdout
492+
493+
494+
def test_collection_flag_network_error(httpx_mock: HTTPXMock, tmp_path: Path) -> None:
495+
"""A network failure on the explicit --collection fetch should exit with code 2."""
496+
httpx_mock.add_exception(
497+
httpx.ConnectError("connection refused"),
498+
url="https://marketplace.infrahub.app/api/v1/collections/acme/foo/download",
499+
)
500+
result = runner.invoke(app, ["get", "acme/foo", "-c", "-o", str(tmp_path)])
501+
502+
assert result.exit_code == 2
503+
assert "Marketplace request failed" in result.stdout
504+
505+
452506
def test_get_schema_stdout(httpx_mock: HTTPXMock, tmp_path: Path) -> None:
453507
httpx_mock.add_response(
454508
method="GET",

0 commit comments

Comments
 (0)