Add infrahubctl marketplace CLI for fetching schemas and collections#952
Conversation
- Probe schema and collection endpoints in parallel so `infrahubctl marketplace download <ns>/<name>` no longer requires the user to pass `--collection` up front. Collision between the two resolves to schema with a warning (pass `--collection` to force the other path). `_detect_item_type` returns the winning 200 response so the download helper reuses it instead of re-fetching. - Remove the `--load` convenience flag; `download` is write-only. Users who want to push into Infrahub chain with `infrahubctl schema load`. - Introduce a four-class error taxonomy (invalid-input, not-found, network) with exit codes 1 vs 2, distinguishing "version not found" from "schema not found" when `--version` is passed. - Surface filesystem failures (unwritable `--output-dir`) cleanly rather than as a traceback. - Regenerate `infrahubctl-marketplace.mdx`; no longer advertises `--load`. - Add spec, plan, research, contract, quickstart, tasks, and checklist artifacts under `specs/001-marketplace-api-update/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deploying infrahub-sdk-python with
|
| Latest commit: |
63f1162
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a209d48c.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://knotty-dibble.infrahub-sdk-python.pages.dev |
Spell out 'semver' and replace 'config/env' with the full words so vale's spelling and word-swap rules pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## stable #952 +/- ##
==========================================
+ Coverage 81.41% 81.67% +0.26%
==========================================
Files 134 135 +1
Lines 11347 11626 +279
Branches 1703 1759 +56
==========================================
+ Hits 9238 9496 +258
- Misses 1566 1577 +11
- Partials 543 553 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 54 files with indirect coverage changes 🚀 New features to boost your workflow:
|
infrahubctl marketplace CLI for downloading schemas and collections
There was a problem hiding this comment.
3 issues found across 12 files
You’re at about 95% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="infrahub_sdk/ctl/marketplace.py">
<violation number="1" location="infrahub_sdk/ctl/marketplace.py:99">
P1: Partial probe failures are misclassified as not-found. A transport failure on either endpoint (when no 200 winner exists) should return a network error, not deterministic not-found.</violation>
<violation number="2" location="infrahub_sdk/ctl/marketplace.py:243">
P1: Network/HTTP errors during the actual download path can still exit with code 1 because uncaught `httpx` exceptions are handled by the default decorator exit code.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
CI runs markdownlint v0.40.0 which flags MD060 (table-column-style) when the separator row uses compact pipes while the data rows are padded. Match the repo's existing style (docs/python-sdk/introduction.mdx) by padding the separator rows too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
infrahubctl marketplace CLI for downloading schemas and collectionsinfrahubctl marketplace CLI for fetching schemas and collections
- Rename `infrahubctl marketplace download` to `infrahubctl marketplace get` to match the convention used by similar tools (kubectl, gh, etc.) and stay consistent with future `marketplace list` / `marketplace search` commands. - Add `--stdout/-s` flag that streams content to stdout (status, warnings, and errors routed to stderr) so output can be piped into commands like `infrahubctl schema check` once stdin support lands. Skips disk writes entirely. For collections, schemas are concatenated as multi-doc YAML, only injecting `---` separators when missing. - Route `_fail` errors through stderr unconditionally so failures don't pollute stdout when piping. - Regenerate `infrahubctl-marketplace.mdx` and add `stdout`/`stderr` to the vale spelling exceptions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # infrahub_sdk/ctl/cli_commands.py # specs~origin_stable
…rors 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>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…config, marketplace_url - Replace ErrorClass Literal + _ERROR_EXIT_CODES dict with _ErrorClass(Enum) so the exit code mapping is co-located with the error definition - Remove type: ignore[return-value] by using inline isinstance guards so the type checker narrows httpx.Response naturally in each branch - Build httpx.AsyncClient via _make_http_client() using the SDK's proxy and TLS config (ConfigBase.proxy, proxy_mounts, tls_context) and follow_redirects=True - Move marketplace_url from infrahubctl Settings to SDK ConfigBase (INFRAHUB_MARKETPLACE_URL); regenerate config.mdx Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… config, helper extraction Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ssages, collection=False test, spec renames
- Move is_transport_failure to module level as _is_transport_failure
- Replace f"Error: {detail}" 404 messages with consistent "No schema/collection named..." format
- Remove now-unused contextlib.suppress import
- Add test_collection_false_downloads_schema covering the defensive bool|None path
- Update spec.md, plan.md, tasks.md: rename command references from download to get
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
| ) | ||
| proxy: str | None = Field(default=None, description="Proxy address") | ||
| proxy_mounts: ProxyMountsConfig = Field(default=ProxyMountsConfig(), description="Proxy mounts configuration") | ||
| marketplace_url: str = Field( |
There was a problem hiding this comment.
Is this configuration actually used anywhere? It seems like the app is using either the parameter defined when running the command line or the entry in the infrahubctl config which shadows this one.
It looks like this is just dead code now, but could be fine if we later refactor infrahubctl and remove that config section alltogether.
There was a problem hiding this comment.
The marketplace_url field in ConfigBase isn't wired into the marketplace command today (which reads from SETTINGS.active.marketplace_url). It's a forward-looking field intended to survive once we retire the infrahubctl.toml / Settings layer — happy to drop it now if you'd prefer to keep things minimal.
| None, "--version", "-v", help="Specific schema version, for example 1.2.0. Default: latest published." | ||
| ), | ||
| collection: bool | None = typer.Option( | ||
| None, |
There was a problem hiding this comment.
By default we'll have a falsy statement here, i.e. None the function signature makes us believe that collection can have three states True, False, None which is not true, the only possible options are True or None as there doesn't seem to be a way to set it to False. As a result we have some dead code below that also acknowledges this. It seems like we compensate for the error here with code that we know will be unreachable below. This should just be a bool that defaults to False.
There was a problem hiding this comment.
Fixed in 69fc5ba — changed to collection: bool = False with is_flag=True. The elif collection: / else: branches are now just if collection: ... else: auto-detect, removing the unreachable force-schema path entirely.
| # Typer exposes only --collection, not --no-collection, so collection=False | ||
| # is unreachable from the CLI. Kept for defensive completeness against the | ||
| # bool | None type. | ||
| item_type = "schema" |
There was a problem hiding this comment.
Refactor once the param above has been changed to a pure bool and not a bool | None, then reverse the condition so the first option is the True one.
There was a problem hiding this comment.
I'd also suggest that after the first check for collections we add the type as:
if collection:
item_type: MarketplaceItemType = "collection"There was a problem hiding this comment.
Fixed in 69fc5ba — condition is now if collection: item_type: MarketplaceItemType = "collection" / else: auto-detect.
There was a problem hiding this comment.
Fixed in 69fc5ba — added item_type: MarketplaceItemType = "collection" on the collection branch.
| if prefetched is not None: | ||
| resp = prefetched | ||
| else: | ||
| resp = await client.get(_collection_url(base_url, namespace, name)) |
There was a problem hiding this comment.
In general code is easier to read if we lead with a truthy statement, so:
if prefetched:
resp = prefetched
else:
resp = await client.get(_collection_url(base_url, namespace, name))There was a problem hiding this comment.
Fixed in 69fc5ba — if prefetched: now leads.
| # Typer exposes only --collection, not --no-collection, so collection=False | ||
| # is unreachable from the CLI. Kept for defensive completeness against the | ||
| # bool | None type. | ||
| item_type = "schema" |
There was a problem hiding this comment.
I'd also suggest that after the first check for collections we add the type as:
if collection:
item_type: MarketplaceItemType = "collection"| namespace, | ||
| name, | ||
| version, | ||
| output_dir, |
There was a problem hiding this comment.
When we have this many parameters to a function I'd suggest to use keyword arguments for everything so that it's crystal clear regarding what parameter each one is instead on relying on positional parameters.
There was a problem hiding this comment.
Fixed in 69fc5ba — both _download_schema and _download_collection calls now use keyword arguments throughout.
| if version: | ||
| _status_console(stdout).print( | ||
| "[yellow]Warning: --version is ignored when downloading a collection." | ||
| ) |
There was a problem hiding this comment.
I don't really have a strong opinion here, but if a user specifically requests a specific version but ends up on a collection instead I wonder if a better approach would be to fail with and error so that the user could choose if they want to remove the --version param instead or if this is fine. Only raising a thought here.
There was a problem hiding this comment.
Kept the warn-and-proceed behaviour for now — feels less surprising for a user who runs get acme/starter-pack --version 1.0.0 and just forgot collections don't have versions. Happy to flip it to a hard error if you'd prefer.
| By default, auto-detects whether `namespace/name` is a schema or a collection. | ||
| Pass --collection to force the collection path when an identifier exists as both. | ||
| """ | ||
| namespace, name = _parse_identifier(identifier) |
There was a problem hiding this comment.
Here we assume that we have the correct order of the variables returned from _parse_identifier(identifier).
I think it could be a bit cleaner to have _parse_identifier return an object like this:
@dataclass(frozen=True, slots=True)
class MarketplaceIdentifier:
namespace: str
name: strThat way there's no confusion and there are several other functions where we pass in both name and namespace which could be replaced by one variable.
There was a problem hiding this comment.
Fixed in 69fc5ba — _parse_identifier now returns a MarketplaceIdentifier(namespace, name) frozen dataclass, eliminating positional ordering assumptions.
| resp = await client.get(_schema_url(base_url, namespace, name, version=version)) | ||
|
|
||
| if resp.status_code == 404: | ||
| if version is not None and schema_confirmed_exists: |
There was a problem hiding this comment.
Assuming we don't want to support an empty string as a version I'd change this to:
if version and schema_confirmed_exists:There was a problem hiding this comment.
Fixed in 69fc5ba — changed to if version and schema_confirmed_exists:.
- Add MarketplaceIdentifier dataclass; _parse_identifier now returns it instead of a bare tuple, removing positional ordering assumptions - collection: bool | None = None → bool = False with is_flag=True; the if/elif/else block becomes if collection: ... else: auto-detect - Reverse prefetched condition to lead with truthy (if prefetched:) - if version is not None → if version - Add MarketplaceItemType annotation on the collection branch - Use keyword arguments throughout _download_schema / _download_collection calls Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="infrahub_sdk/ctl/marketplace.py">
<violation number="1" location="infrahub_sdk/ctl/marketplace.py:255">
P1: ZIP entry path not validated before write. This can write files outside --output-dir. Validate and block absolute/parent-traversal paths before writing.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| _mkdir_or_fail(output_dir) | ||
| for schema_name in schema_names: | ||
| content = archive.read(schema_name).decode("utf-8") | ||
| file_path = output_dir / schema_name |
There was a problem hiding this comment.
P1: ZIP entry path not validated before write. This can write files outside --output-dir. Validate and block absolute/parent-traversal paths before writing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/ctl/marketplace.py, line 255:
<comment>ZIP entry path not validated before write. This can write files outside --output-dir. Validate and block absolute/parent-traversal paths before writing.</comment>
<file context>
@@ -182,55 +214,49 @@ async def _download_collection(
+ _mkdir_or_fail(output_dir)
+ for schema_name in schema_names:
+ content = archive.read(schema_name).decode("utf-8")
+ file_path = output_dir / schema_name
+ file_path.write_text(content, encoding="utf-8")
+ console.print(f"[green]Downloaded {schema_name} -> {file_path}")
</file context>
| file_path = output_dir / schema_name | |
| candidate_path = Path(schema_name) | |
| if candidate_path.is_absolute() or ".." in candidate_path.parts: | |
| _fail(_ErrorClass.INVALID_INPUT, f"Invalid schema path in archive: {schema_name}") | |
| file_path = (output_dir / candidate_path).resolve() | |
| if output_dir.resolve() not in file_path.parents and file_path != output_dir.resolve(): | |
| _fail(_ErrorClass.INVALID_INPUT, f"Invalid schema path in archive: {schema_name}") |
@DataClass + from __future__ import annotations triggers a Python 3.12 bug in dataclasses._is_type during docs generation. NamedTuple avoids this entirely while providing the same immutable named-field semantics. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
typer 0.25.1 (used in CI via the stable lockfile) removed explicit mix_stderr from CliRunner.__init__, causing ty to flag the call as unknown-argument even though it still works at runtime via **kwargs. Silence with type: ignore[call-arg]. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
ReadTimeout and similar httpx exceptions can have an empty str() representation, producing "Marketplace request failed: " with nothing after the colon. Fall back to the exception class name so the output is always informative (e.g. "Marketplace request failed: ReadTimeout"). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
In typer 0.25.1 result.stdout only captures stdout; error messages written to err_console (stderr) only appear in result.output. Switching all assertions to result.output works across both typer versions. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Summary
Adds a new
infrahubctl marketplacesub-app with agetcommand that pulls schemas and collections from the public Infrahub Marketplace (marketplace.infrahub.app) into a local directory or to stdout.What the command does
/api/v1/schemas/...,/api/v1/collections/...); no GraphQL involved.namespace/namecollision, schema wins with a warning;--collection/-cforces the other path.--version <semver>pins the download to a specific published schema version; default is whatever the marketplace resolves as latest (echoed back on success).--stdout/-swrites content to real stdout and routes status, warnings, and errors to stderr, so the output can be piped into other tools (e.g.infrahubctl schema checkonce it grows stdin support). Collections are emitted as multi-doc YAML, with---separators only inserted when a schema doesn't already start with one. No files are written when--stdoutis set.--output-dir/-o(default./schemas) controls where files land; missing parent directories are created.--marketplace-url,infrahubctl.tomlmarketplace_url, orINFRAHUB_MARKETPLACE_URLenv var let you point at staging or a local instance.1for deterministic failures and2for transient/network so CI can branch on it. No tracebacks on an unwritable output directory, a missing schema, or a connection refused.Options
IDENTIFIER(positional)namespace/name— a schema or collection-v, --version TEXT-c, --collection-s, --stdout-o, --output-dir PATH./schemas--marketplace-url TEXThttps://marketplace.infrahub.appBehaviour notes
Downloaded schema …/Collection …) so the user can detect an unintended match in a collision case.--versionalongside a collection identifier prints a warning and proceeds with the collection download, rather than failing.--version, distinct from "schema not found".Out of scope
No
--loadconvenience flag.getis fetch-only — chain withinfrahubctl schema load <dir>(or pipe via--stdoutonce stdin support lands) to push the result into a running Infrahub.Test plan
uv run pytest tests/unit/ctl/test_marketplace_app.py— 20 tests covering auto-detect, collision, error classes,--version,--output-dir,--marketplace-url,--collectionoverride, and--stdoutmode (single schema, collection, separator injection).uv run invoke lint— ruff, ty, mypy, vale, markdownlint all clean.uv run invoke generate-infrahubctl—docs/docs/infrahubctl/infrahubctl-marketplace.mdxregenerated.infrahubctl marketplace get nonexistent/nonexistentreturns the expected not-found message with exit 1.Artefacts
Full spec, plan, research, contracts, quickstart, and task breakdown under
specs/001-marketplace-api-update/for reviewers who want the design rationale (auto-detect strategy evaluation, error taxonomy decisions, collision precedence).🤖 Generated with Claude Code