Add Registry Ping Command#45
Merged
Merged
Conversation
Add 'regshape ping --registry <host>' command that issues GET /v2/ to verify registry connectivity and OCI Distribution API support. Library layer (libs/ping/): - ping() function issues GET /v2/ via RegistryClient, measures latency, and returns a PingResult dataclass with reachable, status_code, api_version (Docker-Distribution-API-Version header), and latency_ms - Raises PingError on connection/DNS/timeout failures - Raises AuthError on HTTP 401 responses CLI layer (cli/ping.py): - Top-level 'ping' command (not a subcommand group) - --registry/-r (required): registry hostname - --json: JSON output format - Auth errors treated as reachable (registry responded with 401 challenge) with a hint to run 'regshape auth login' - Exit code 0 on success/auth-required, 1 on unreachable Other changes: - Add PingError to libs/errors.py - Register ping command in cli/main.py - 23 unit tests (11 operations + 12 CLI)
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new top-level regshape ping command to validate OCI registry connectivity via GET /v2/, implemented with a small library operation layer and covered by new unit tests/spec documentation.
Changes:
- Introduces
regshape.libs.pingwithping(client)and aPingResultmodel (latency + API version header support). - Adds
regshape ping --registry ... [--json]CLI command and registers it in the main CLI. - Adds
PingErrorand new unit tests + a design/spec document for the command.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/regshape/libs/ping/operations.py | Implements library-level ping and PingResult serialization. |
| src/regshape/libs/ping/init.py | Re-exports ping API surface. |
| src/regshape/libs/errors.py | Adds PingError to shared error types. |
| src/regshape/cli/ping.py | Implements the Click command, JSON/plain output, and exit codes. |
| src/regshape/cli/main.py | Registers the new top-level ping command. |
| src/regshape/tests/test_ping_operations.py | Unit tests for ping operation behavior and serialization. |
| src/regshape/tests/test_ping_cli.py | CLI tests for output modes, exit codes, and flag propagation. |
| specs/cli/ping.md | Command design/spec and examples/test plan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The Exit Codes section says code 0 means only HTTP 200, but the CLI behavior described in this PR (and implemented in src/regshape/cli/ping.py) treats auth-required as reachable and exits 0 as well. Please update this table/description to include the auth-required reachable case so the spec matches the implementation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Toddy Mladenov <me@toddysm.com>
The CLI test plan here says Auth failure → exit code 1, but the implemented CLI/tests treat AuthError as reachable and exit 0 (see TestPingCommand.test_auth_error_exits_0_reachable). Please reconcile the spec with the intended behavior (either update the plan or adjust the CLI/tests). Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Toddy Mladenov <me@toddysm.com>
When handling PingError, passing str(exc) into _error() results in duplicated messaging because PingError messages already include Registry <host> is not reachable. This currently produces output like Error: Registry <host> is not reachable: Registry <host> is not reachable : <reason>. Consider using the underlying exception (exc.__cause__) or changing PingError to carry/format only the low-level reason so the CLI can add the prefix once. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Toddy Mladenov <me@toddysm.com>
For --json, the unreachable (non-200) path calls _error() which emits only {registry, reachable, error} and discards the structured fields you already have in PingResult (status_code, api_version, latency_ms). This diverges from the PR description/spec examples and makes the JSON output harder to consume programmatically. Consider returning a JSON object that includes at least status_code and latency_ms (and api_version if present) for non-200 responses, rather than collapsing everything into an error string.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Toddy Mladenov <me@toddysm.com>
In the AuthError JSON output you hardcode "status_code": 401, but AuthError can be raised for cases that aren't strictly an HTTP 401 (e.g. Basic auth required but no credentials, token negotiation failures that surface as 403, etc.). If you can't reliably surface a status code here, it would be more accurate to omit it / set it to null and include error/note text from the exception, or extend AuthError to optionally carry an HTTP status code similar to BlobError. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Toddy Mladenov <me@toddysm.com>
Unused import: patch is imported from unittest.mock but never used in this test module. This can trigger lint failures and adds noise—please remove it. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Toddy Mladenov <me@toddysm.com>
The documented JSON failure format shows status_code/api_version/latency_ms fields, but the current CLI implementation (_error() in src/regshape/cli/ping.py) emits {registry, reachable, error} for failures. Either update this spec example to match the actual output, or update the CLI to include those structured fields on non-200 responses.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Toddy Mladenov <me@toddysm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements
regshape ping --registry <host>command that checks OCI registry connectivity by issuingGET /v2/per the OCI Distribution Spec. Closes #32.Changes
Library Layer (
src/regshape/libs/ping/)operations.py:ping(client) -> PingResultfunction that:GET /v2/viaRegistryClientand measures round-trip latencyPingResultdataclass withreachable,status_code,api_version(fromDocker-Distribution-API-Versionheader), andlatency_msPingErroron connection, DNS, or timeout failuresAuthErroron HTTP 401 responses__init__.py: Re-exportspingandPingResultCLI Layer (
src/regshape/cli/ping.py)pingcommand registered directly on theregshapegroup--registry/-r(required),--json(flag)regshape auth login.Other Changes
PingErroradded tosrc/regshape/libs/errors.pypingcommand registered insrc/regshape/cli/main.pyspecs/cli/ping.mdUsage
Example Output
{ "reachable": true, "status_code": 200, "api_version": "registry/2.0", "latency_ms": 42.3, "registry": "ghcr.io" }Tests
23 unit tests added:
test_ping_operations.py(11 tests): PingResult serialization, success with/without API version header, non-200 responses, 401 auth error, connection/timeout/generic errors, latency measurementtest_ping_cli.py(12 tests): plain text and JSON output, exit codes, auth error handling, insecure flag propagation, required option validation