feat(registry): implement async RegistryClient using httpx #32
No reviewers
Labels
No labels
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Blocks
#24 feat(registry): implement async HTTP client using httpx
cleveragents/cleveractors-core
Reference
cleveragents/cleveractors-core!32
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/m1-registry-http-client"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Add async RegistryClient implementing the Package Registry Standard v1.0.0 HTTP API endpoints using httpx.AsyncClient.
Closes #24
Add async RegistryClient implementing the Package Registry Standard v1.0.0 HTTP API endpoints (§8) using httpx.AsyncClient. - GET /packages/{id} — retrieve package content by PackageId - GET /{type}/{ns}/{name}?version= — resolve references with version alias resolution (latest, vx, vX.Y.x, vX.x per §4.2) - GET /browse — discover published packages with type/namespace filters - GET /.well-known/cleverthis-packages — registry metadata discovery - Maps all 8 standard error types (§13.2) to typed exceptions - Supports anonymous reads (§9.1) and optional API key auth (§9.2) - Async context manager support (__aenter__/__aexit__) - 32 Behave BDD scenarios, 7 Robot Framework integration tests, ASV benchmarks, 100% registry module coverage ISSUES CLOSED: #243f8a2b1df21ae754cc01A few things I found.
@ -0,0 +139,4 @@Scenario: Unknown error status falls back to RegistryErrorWhen I create a RegistryClient pointed at "https://registry.example.com"When the mock server returns status 418 with non-JSON body "plain text error"The 418 return code means: "I'm a teapot". Source: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/418
Are you sure that's the return code that you want to use?
@ -0,0 +131,4 @@future = asyncio.run_coroutine_threadsafe(coro_fn(), loop)future.result(timeout=10)else:loop.run_until_complete(coro_fn())Why does the execution allow 10 seconds if the event loop is already running, but as much time as desired if the event loop needs to be started?
Fixed. Both code paths now use asyncio.wait_for() with a consistent 30s timeout (future.result(timeout=30) for running loops, loop.run_until_complete(asyncio.wait_for(coro_fn(), timeout=30)) for non-running loops, and asyncio.run(asyncio.wait_for(coro_fn(), timeout=30)) for the fallback).
@ -0,0 +63,4 @@"packages": [],"count": 0,},)This is probably not a good response. There are packages returned in the earlier parts of this
ifstatement. But nothing is listed here.A generic testing framework would want examples of packages here. It would read the example packages, then try to read them.
Fixed. fake_registry_server.py now defines EXAMPLE_PACKAGES with 2 packages (an actor and a graph) that are returned by the /browse endpoint. The browse response also supports optional ?type= filtering.
@ -0,0 +33,4 @@"skill": "pkg_skl_","mcp": "pkg_mcp_","lsp": "pkg_lsp_",}When I look at #31/files , file src/cleveractors/registry/types.py , lines 14-21 and I see this file, I get nervous. This is close to the same information in two different files.
What happens when a new package type appears in one but not the other?
I strongly recommend that the prefixes be in the same file, next to each other, so that it will be harder for them to get out of sync.
That will be consolidated better after merging this PR and with other PR merged too. For now these are independent works.
How do you know this isn't a
ValidationError?(Replying to your inline comment on
src/cleveractors/registry/exceptions.pyin theexception_for_statusfunction)The exception_for_status() function is the fallback — it only activates when the server does not include an explicit error type in the response body.
Per Package Registry Standard §13.1, compliant servers return errors as:
Extract
body["error"]["type"]from the JSON responseLook it up in
_ERROR_TYPE_MAPto get the correct exception classRaise that specific exception (e.g.
ValidationError, VersionNotFoundError, ...)Only when the body lacks a recognizable error.type field does
exception_for_status()act as the status-code-only fallbackThis means a standards-compliant server can signal either ValidationError or InvalidPackageIdError at 400 by including the appropriate type value, and the client will raise the correct exception.
Test added:
@ -0,0 +68,4 @@if status_code == 403:return AccessDeniedError(message)if status_code == 404:return PackageNotFoundError(message)The above list of errors maps from
PackageNotFoundErrorandVersionNotFoundErrorto 404. How do you know that this is aPackageNotFoundErrorinstead of aVersionNotFoundError?Fixed. The _request() method now parses the standard error format {"error": {"type": "...", "message": "..."}} from the JSON response body per Package Registry Standard §13.1. When the error type is present, it is matched against the defined types in _ERROR_TYPE_MAP to raise the correct specific exception. The status-code fallback via exception_for_status() is preserved for servers that do not include the type field or return non-standard error formats.
FOR ANYONE ELSE READING THIS -- We need to work quickly.
If, in your judgement, all of the problems have been either solved (i.e. new code) or there is an explanation, then feel free to accept this code and merge it with
master.1ae754cc0192d5305a20Thanks for the thorough review, Brent. Here is what was addressed and the rationale for each point:
Applied
#2 — Timeout asymmetry in
RegistryClientLib._run()Fixed. Both code paths now use
asyncio.wait_for()with a consistent 30s timeout (future.result(timeout=30)for running loops,loop.run_until_complete(asyncio.wait_for(coro_fn(), timeout=30))for non-running loops, andasyncio.run(asyncio.wait_for(coro_fn(), timeout=30))for the fallback).#3 — Fake server browse returns empty list
Fixed.
fake_registry_server.pynow definesEXAMPLE_PACKAGESwith 2 packages (an actor and a graph) that are returned by the/browseendpoint. The browse response also supports optional?type=filtering.#6 — 404 maps to
PackageNotFoundErrorbut could beVersionNotFoundErrorFixed. The
_request()method now parses the standard error format{"error": {"type": "...", "message": "..."}}from the JSON response body per Package Registry Standard §13.1. When the error type is present, it is matched against the defined types in_ERROR_TYPE_MAPto raise the correct specific exception. The status-code fallback viaexception_for_status()is preserved for servers that do not include the type field or return non-standard error formats.#7 — 400 maps to
InvalidPackageIdErrorbut could beValidationErrorSame fix as #6. Added Behave scenarios verifying both
VersionNotFoundError(404 + type in body) andValidationError(400 + type in body) are correctly raised when the server includes the explicit error type.Not Applied
#1 — HTTP 418 status code in test
Not changed. The 418 response status is used in a single test scenario whose purpose is to verify that unmapped HTTP status codes fall through to the generic
RegistryError. Any unregistered 4xx code serves this purpose equally well; 418 is as arbitrary as 417 or 422. The scenario description explicitly states "Unknown error status falls back to RegistryError", making the intent clear.#4/#5 — Duplicate
_PACKAGE_TYPE_PREFIXESdict vs PR #31types.pyNot changed. As noted in your follow-up comment #5, these are independent works developed on separate branches. Consolidation will happen after both PRs are merged. Duplicating a small mapping on separate branches avoids a merge-order dependency between the two PRs.
Re: How do you know this isn't a ValidationError?
(Replying to your inline comment on
src/cleveractors/registry/exceptions.pyin theexception_for_statusfunction)The
exception_for_status()function is the fallback — it only activates when the server does not include an explicit error type in the response body.Per Package Registry Standard §13.1, compliant servers return errors as:
The
_request()method now parses this structure first:body["error"]["type"]from the JSON response_ERROR_TYPE_MAPto get the correct exception classValidationError,VersionNotFoundError, ...)error.typefield doesexception_for_status()act as the status-code-only fallbackThis means a standards-compliant server can signal either
ValidationErrororInvalidPackageIdErrorat 400 by including the appropriate type value, and the client will raise the correct exception.Test added:
Looks good to me!