Skip to content

Commit c98792d

Browse files
praveen-formidoclaudechandrasekharan-zipstack
authored
feat: pick up the whoami and deployment-listing operations (#29)
* feat: pick up the whoami and deployment-listing operations Runs the `spec-upgrade` pipeline against Zipstack/unstract main at 520b98d7a, which merged the two operations the CLI needs (unstract#2269 and unstract#2278). The spec is copied byte-for-byte and `SPEC_SOURCE` moves with it in this same commit — revision and sha256 both — so a current copy stays distinguishable from one the backend has moved past. I verified the *previous* record before moving it: the vendored file matched its recorded sha256 and was byte-identical to the backend at `eddd4b746`, so this upgrade starts from an honest baseline. Regeneration is purely additive: new `api/identity/whoami.py` and `api/deployment/list_deployments.py`, six new models, and prose-only changes to `execute`/`status` where #2278 reworded the descriptions. No operation, field or model was removed, so this is a **minor** bump rather than a major one. The generator exited clean, and regenerating a second time produces byte-identical output, so `sdk-drift` will pass. **The new facade class.** `whoami` and `list_deployments` both authenticate with a platform key, and neither fits `APIDeploymentsClient`: that class takes a *deployment* URL and derives an organisation and API name from its last two segments, which `whoami` has neither of. So `PlatformAPIClient` sits alongside it, sharing the generated transport — both schemes are HTTP bearer, only the token differs — and raising the same exception type. Folding them together would have meant a class whose required `api_url` is meaningless for half its methods. Without this the operations are generated but unreachable, and `unstract-cli` keeps reaching into `unstract.clone.PlatformClient` — the org-cloning tool's hand-written admin client — which is how the CLI drifted off the generated surface to begin with. **A defect the new tests caught.** `_error_text` fell back to `response.text`, but the generated `Response` is an attrs wrapper carrying `.content`, and `parsed` is a model instance rather than a mapping. A 401 through the new client would have raised `AttributeError` while trying to report the refusal. Both halves are fixed; the `_error_text` change is additive, so httpx callers are unaffected. **Test coverage.** `test_every_declared_operation_is_wrapped` fired exactly as designed when the spec grew. Extending `WRAPPED_OPERATIONS` was not the right answer, though: `whoami` declares no `ErrorResponse`, so that suite's "both families are in play" assertion is false for it by construction, and `_declared_responses` indexes `content` unconditionally, which its bodyless 500 would `KeyError` on. A parallel `PLATFORM_OPERATIONS` manifest carries its own status pins and error-reporting coverage, and the whole-set comparison now unions the two — so an operation belonging to neither still fails there, which is the property that test exists for. `__version__` and the compat baseline are deliberately untouched: the release workflow reads the former as the last released version and applies the bump at dispatch, and a spec upgrade is not a reason to move the parity reference point. 430 tests pass, up from 419. `ruff check` and `format --check` clean on everything this touches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ * fix: give PlatformAPIClient the contract APIDeploymentsClient already has Six findings from the review on this PR. Five were real contract gaps and one was a missed export; all are pinned by tests that fail when the fix is reverted. **The body is read as JSON, not through the generated model.** This is the one that mattered. `sync_detailed` reaches `_parse_response`, which does `PlatformKeyError.from_dict(response.json())` on a 401 with no guard: a gateway answering 401 with HTML raises `JSONDecodeError`, and a DRF-shaped `{"detail": ...}` raises `KeyError: 'message'` -- both out of the generated parser, before this facade sees the response. So a rejected key crashed instead of being reported. The request is now issued from the generated `_get_kwargs` and the body read through `APIDeploymentsClient._read_body`, which is exactly why that helper exists. This is the same defect class as the `_error_text` fix in the previous commit. I fixed the half where reporting a refusal crashed and missed the half where building the model crashed first. **Transport failures are translated.** The class called `sync_detailed` directly, so an unreachable host raised raw `httpx.ConnectError` -- contradicting the module docstring this PR added, which promises the `requests` exception types callers catch. It now goes through `_send`, like every deployment-key request. **The credential is read per request.** `AuthenticatedClient` bakes its auth header on first use, so a key assigned after the transport was built kept sending the old one. `_send` sets the header per call. **`close`, `__enter__` and `__exit__`** -- pooled connections had nothing to release them, and the CLI builds one client per job. **Re-exported from the package root**, so it is reachable as `unstract.api_deployments.PlatformAPIClient` rather than only from the private module. Also corrected the `_error_text` comment from the previous commit: it described the platform facade raising AttributeError, which is no longer a path that exists now that both facades hand it an httpx response. 438 tests pass, up from 430. Five mutations killed: unguarded `response.json()`, dropping the transport translation, capturing the key with the transport, neutering `close`, and removing the re-export. The generated tree is untouched, so `sdk-drift` is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ * test: cover the generated parsers the facade deliberately bypasses Greptile, on PR #29. Two real problems, and the first is mine twice over. **A stale docstring.** `_platform_reply` claimed that patching at `get_httpx_client` meant "the generated parsing and model construction still run". That was true of the first commit, which called `sync_detailed`. The review fixes moved the facade to `_get_kwargs()` plus its own body read, and the sentence survived the change it described -- the same failure mode as the `openapi_schema` docstring this PR's backend counterpart had to fix. **And the coverage the sentence was standing in for did not exist.** With the facade reading bodies itself, nothing exercised `whoami._parse_response`, `list_deployments._parse_response`, or any of the six new models. A regeneration that broke them would have passed this suite. Added, exercised directly rather than through the facade: - every field of `WhoAmIResponse`, `PlatformKeyError` and the paginated listing, including the nested `APIDeploymentSummary` row; - both new `_parse_response` functions, on a declared 200 and a declared 401; - and the reason the facade does not use them -- a gateway's HTML 401 raises `ValueError` and a DRF-shaped body raises `KeyError` out of the parser. Pinning that keeps the facade's decision justified instead of looking arbitrary. The tier field is a correction too: the spec declares a ChoiceField, and I had described that as giving the client "a real enum". This generator renders it as a `Literal` alias plus a `check_api_key_permission` validator, so the value stays a plain string. The test now asserts what is actually emitted, and exercises the validator in both directions. `_deployment_page()` is shared between the facade test and the model tests: a row that satisfied one and not the other would prove nothing about either. 441 tests pass, up from 438. Two mutations killed to confirm the new coverage is real -- a model reading the wrong key, and a parser dropping its 401 branch. An earlier mutation of mine (adding a default to a required `d.pop`) survived because the test supplies the field, so it was equivalent rather than a miss. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ * refactor: fold the platform client into a shared facade base (#30) PR #29 gave the platform client its own copy of the transport: the pool, the close/reopen handling and the httpx-to-requests exception translation were duplicated from APIDeploymentsClient, and the copy had no retry policy at all. Two copies of that code drift; only one of them was getting fixes. Both clients now inherit `_HttpxFacade`, which owns the lazily built pooled transport, `close`/context-manager support, the exception translation and the retry-with-Retry-After policy. A client subclass supplies its error class and its methods, nothing else. That drops ~184 duplicated lines and gives the platform operations the retry behaviour the README already promised. Three defects fall out of sharing the code: - The transport pool is now built inside the lock. `AuthenticatedClient .get_httpx_client()` builds lazily and unsynchronised, so publishing the client before warming it let two threads build two pools. - A close during flight no longer escapes untranslated. httpx answers a send on a closed client with a bare `RuntimeError`, which is not in the subtree `_translate_transport_errors` covers, so it reached callers catching the documented `requests` types. It is translated at the send. - `list_deployments` no longer sends `workflow=None`. The generated builder renders that parameter with `str()` before it filters `None` out, so the literal string "None" went on the wire as a filter matching no workflow on every otherwise unfiltered call. Unset filters are omitted instead, which also holds if the generator special-cases another parameter later. Exceptions get a hierarchy. `APIDeploymentsClientException` never worked -- its `__init__` nested three more `def`s that were never bound to the class, so `message` was dropped and `Exception.__init__` was never called, leaving `str(e)` empty and the documented `error_message()` non-existent. It is now an alias of a new `UnstractError` base, with `APIDeploymentError` and `PlatformClientError` beneath it. Catching the old name still catches both clients, including anything added later. Also here: - The generated models are re-exported, so callers who want typing can `WhoAmIResponse.from_dict(...)` instead of us hand-writing a mirror of the spec that regeneration would not update. Facade methods keep returning `dict[str, Any]`. - The platform client gets its own logger. Both clients shared the module logger, so levelling one re-levelled the other, switching a live sibling's debug output -- which includes response bodies -- on or off as a side effect. - A 2xx body that is unreadable, or JSON that is not an object, is now an error naming what arrived rather than an `AttributeError` downstream. The ERROR log for it is bounded to the same excerpt the exception carries. - An `org_id` that is empty or blank is refused before the request, and a path on `base_url` is warned about rather than silently discarded by `urljoin`. - `.claude/skills/spec-upgrade/SKILL.md` step 5 gains the recipe for adding an operation: which class it belongs to, the method shape, and why it builds from `_get_kwargs` rather than `sync_detailed`. No runtime breaking change. The one visible shift is `type(e).__name__`, which becomes `APIDeploymentError` where it was `APIDeploymentsClientException`; `except APIDeploymentsClientException` is unaffected. Claude-Session: https://claude.ai/code/session_01CnhmFYFRBrK376tZyDtwUM Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
1 parent e7c0184 commit c98792d

19 files changed

Lines changed: 2683 additions & 212 deletions

‎.claude/skills/spec-upgrade/SKILL.md‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,31 @@ Upstream, the spec is produced by the backend that serves these endpoints
7171
the facade is where it becomes public API. Fixes belong here or upstream in
7272
the spec, never in the generated tree — regeneration overwrites that wholesale.
7373

74+
A new operation belongs to `APIDeploymentsClient` if it takes a deployment
75+
key, `PlatformKeyClient` if it takes a platform key, otherwise a new subclass
76+
of `_HttpxFacade` — never a free-standing class, or the transport, retry and
77+
exception translation get reimplemented and drift. The method is two lines:
78+
79+
```python
80+
def list_widgets(self, org_id: str, *, page: int | None = None) -> dict[str, Any]:
81+
kwargs = list_widgets._get_kwargs(org_id, **{"page": page} if page else {})
82+
return self._read_or_raise(self._request_with_retry(**kwargs), "list_widgets")
83+
```
84+
85+
- Build from `_get_kwargs`, not `sync_detailed`: the generated
86+
`_parse_response` calls `from_dict` on an error body unguarded, so an
87+
undeclared one raises before the facade sees the status. Omit unset
88+
parameters rather than passing `None` — the builder renders some before it
89+
filters `None` out. Both are private to the generator, so pin them in
90+
`tests/test_compat.py`.
91+
- Send through `_request_with_retry`, and read through `_read_or_raise`,
92+
which checks the status first.
93+
- Return `dict[str, Any]`. The generated models are exported for callers who
94+
want typing; a hand-written `TypedDict` would not survive regeneration.
95+
- A new error type subclasses `UnstractError`.
96+
97+
`PlatformKeyClient.whoami` is the smallest example in the tree.
98+
7499
6. **Run the tests:** `uv run pytest tests/`. `tests/test_compat.py` compares
75100
this client against the last released one, vendored under `tests/baseline/`.
76101
Refresh that baseline only when you mean to move the parity reference point,
@@ -87,8 +112,10 @@ fail the same way. If it is red, run step 3 and commit the result.
87112

88113
Choose the bump by what changed for callers: **major** when the spec removed or
89114
renamed something callers depend on, **minor** for new endpoints or new
90-
behaviour, **patch** for fixes that keep the surface identical. A generated diff
91-
with removals in it is the signal for major — spec upgrades produce those.
115+
behaviour — a new facade method included — **patch** for fixes that keep the
116+
surface identical. A generated diff with removals in it is the signal for major
117+
— spec upgrades produce those. Behaviour the baseline pinned that has moved goes
118+
in `ACCEPTED_DIVERGENCES` in the same commit.
92119

93120
Do not touch `__version__` in `src/unstract/api_deployments/__init__.py` in your
94121
PR. The in-repo value is the *last released* version; `main.yml` reads it,

‎README.md‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,47 @@ client = APIDeploymentsClient(
113113
The retry logic uses exponential backoff with full jitter and respects the `Retry-After` header on 429 responses.
114114

115115

116+
## Listing deployments with a platform key
117+
118+
`PlatformKeyClient` takes a **platform** API key, not a deployment key, and reads
119+
the account that key belongs to. It cannot run a deployment.
120+
121+
```python
122+
from unstract.api_deployments import PlatformKeyClient
123+
124+
with PlatformKeyClient("https://us-central.unstract.com", "your_platform_key") as client:
125+
org_id = client.whoami()["organization_id"]
126+
page = client.list_deployments(org_id, page_size=50)
127+
for deployment in page["results"]:
128+
print(deployment["api_name"], deployment["api_endpoint"])
129+
```
130+
131+
Follow `next` for further pages. `api_key` falls back to `$UNSTRACT_PLATFORM_KEY`.
132+
133+
## Errors
134+
135+
Every error either client raises derives from `UnstractError`:
136+
137+
| Exception | Raised by |
138+
|-----------|-----------|
139+
| `UnstractError` | base of both — catch this to catch everything |
140+
| `APIDeploymentError` | `APIDeploymentsClient` |
141+
| `PlatformClientError` | `PlatformKeyClient` |
142+
143+
`APIDeploymentsClientException` is an alias of `UnstractError`, so existing
144+
`except` clauses keep working.
145+
146+
Transport failures are raised as the `requests` exception types
147+
(`ConnectionError`, `Timeout`, and friends) rather than the httpx ones.
148+
116149
## Internals
117150

118151
`unstract.api_deployments._sdk_docstudio` is generated from the deployment API's
119152
OpenAPI spec by `tools/gen_sdk.sh` and is an implementation detail of the
120-
transport. `APIDeploymentsClient` is the supported surface — import from it, not
121-
from the generated tree, which is regenerated wholesale whenever the spec moves.
153+
transport. `APIDeploymentsClient` and `PlatformKeyClient` are the supported
154+
surface — import from those, or from the response models re-exported alongside
155+
them, not from the generated tree, which is regenerated wholesale whenever the
156+
spec moves.
122157

123158
## Cloning an organization
124159

0 commit comments

Comments
 (0)