Skip to content

Commit e876dd8

Browse files
refactor: fold the platform client into a shared facade base
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. Two real 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. - `_send` resolves the transport inside the translated region. A client closed by another thread mid-flight raised httpx's own `RuntimeError`, which is outside the translated subtree and reached the caller untranslated. 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. - An empty `org_id` 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 becomes a recipe: which class a new operation belongs to, the method shape, why it builds from `_get_kwargs` rather than `sync_detailed`, and the rule that a new error type subclasses `UnstractError`. No runtime breaking change. The one visible shift is `type(e).__name__`, which becomes `APIDeploymentError` where it was `APIDeploymentsClientException`; `except APIDeploymentsClientException` is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CnhmFYFRBrK376tZyDtwUM
1 parent 3e9717e commit e876dd8

5 files changed

Lines changed: 690 additions & 351 deletions

File tree

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

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,45 @@ 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+
**Which class does it belong to?** A deployment key and a deployment URL mean
75+
`APIDeploymentsClient`; a platform key means `PlatformKeyClient`. Anything
76+
else is a new subclass of `_HttpxFacade`, never a free-standing class — the
77+
base owns the pooled transport, the retry policy and the translation of httpx
78+
failures into the `requests` types callers catch, and a class that reimplements
79+
those will drift from the one that does not.
80+
81+
**The shape of a facade method**, which is all a new operation should need:
82+
83+
```python
84+
def list_widgets(self, org_id: str, *, page: int | None = None) -> dict[str, Any]:
85+
"""One sentence, then Args and Returns."""
86+
kwargs = list_widgets._get_kwargs(org_id, page=page)
87+
return self._read_or_raise(self._request_with_retry(**kwargs), "list_widgets")
88+
```
89+
90+
Four rules behind it:
91+
92+
- **Build from `_get_kwargs`, not from `sync_detailed`.** The generated
93+
`_parse_response` calls `from_dict` on a declared error status with no
94+
guard, so a gateway answering 401 with HTML, or DRF answering
95+
`{"detail": ...}`, raises `JSONDecodeError` or `KeyError` before the facade
96+
sees the status. `_get_kwargs` is private to the generator, so pin its shape
97+
in `tests/test_compat.py` — a generator bump then fails there rather than at
98+
a customer's call.
99+
- **Send through `_request_with_retry`**, so the operation gets the retry
100+
policy and `Retry-After` handling the README already promises.
101+
- **Check the status before reading the body**, via `_read_or_raise`. Never
102+
`from_dict` a body the server was not obliged to send.
103+
- **Return `dict[str, Any]`.** The generated models are exported for callers
104+
who want typing; do not hand-write a `TypedDict` mirroring the spec, because
105+
regeneration will not update it.
106+
107+
**A new error type** subclasses `UnstractError`, never `Exception` directly —
108+
`APIDeploymentsClientException` is an alias of that base, and callers catching
109+
it must keep catching everything.
110+
111+
`PlatformKeyClient.whoami` is the smallest worked example in the tree.
112+
74113
6. **Run the tests:** `uv run pytest tests/`. `tests/test_compat.py` compares
75114
this client against the last released one, vendored under `tests/baseline/`.
76115
Refresh that baseline only when you mean to move the parity reference point,
@@ -87,7 +126,11 @@ fail the same way. If it is red, run step 3 and commit the result.
87126

88127
Choose the bump by what changed for callers: **major** when the spec removed or
89128
renamed something callers depend on, **minor** for new endpoints or new
90-
behaviour, **patch** for fixes that keep the surface identical. A generated diff
129+
behaviour — including an operation reached only through a new facade method —
130+
**patch** for fixes that keep the surface identical. If behaviour the baseline
131+
pinned has moved, name the divergence in `ACCEPTED_DIVERGENCES` and in the
132+
`tests/test_compat.py` module docstring in the same commit; a divergence pinned
133+
by a test but missing from that list is only findable by reading all of them. A generated diff
91134
with removals in it is the signal for major — spec upgrades produce those.
92135

93136
Do not touch `__version__` in `src/unstract/api_deployments/__init__.py` in your

‎README.md‎

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

115115

116+
## Reading an account with a platform key
117+
118+
`PlatformKeyClient` takes a **platform** API key rather than a deployment key,
119+
and answers what that key can see. A platform key cannot run a deployment, and a
120+
deployment key cannot describe the account, so the two clients are not
121+
interchangeable.
122+
123+
```python
124+
from unstract.api_deployments import PlatformKeyClient
125+
126+
with PlatformKeyClient("https://us-central.unstract.com", "your_platform_key") as pkc:
127+
identity = pkc.whoami()
128+
page = pkc.list_deployments(identity["organization_id"], page_size=50)
129+
for deployment in page["results"]:
130+
print(deployment["api_name"], deployment["api_endpoint"])
131+
```
132+
133+
`api_key` falls back to `$UNSTRACT_PLATFORM_KEY`. `whoami()` resolves the
134+
`organization_id` every other operation needs, so it takes no organisation
135+
argument. `list_deployments()` is paginated — follow `next` rather than assuming
136+
`results` is the whole set. Retries, TLS and timeout settings are the same
137+
parameters, with the same defaults, as `APIDeploymentsClient`.
138+
139+
Both methods return plain dicts. To have them typed, hand the result to the
140+
generated model for that shape:
141+
142+
```python
143+
from unstract.api_deployments import WhoAmIResponse
144+
145+
identity = WhoAmIResponse.from_dict(pkc.whoami())
146+
```
147+
148+
## Errors
149+
150+
Every error either client raises derives from `UnstractError`:
151+
152+
| Exception | Raised by |
153+
|-----------|-----------|
154+
| `UnstractError` | base of both — catch this to catch everything |
155+
| `APIDeploymentError` | `APIDeploymentsClient` |
156+
| `PlatformClientError` | `PlatformKeyClient` |
157+
158+
`APIDeploymentsClientException` is the name this exception shipped under and
159+
remains an alias of `UnstractError`, so existing `except` clauses keep working
160+
unchanged.
161+
162+
Transport failures are raised as the `requests` exception types
163+
(`ConnectionError`, `Timeout`, and friends) rather than the underlying httpx
164+
ones, so callers written against earlier releases catch them by the same names.
165+
116166
## Internals
117167

118168
`unstract.api_deployments._sdk_docstudio` is generated from the deployment API's
119169
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.
170+
transport. `APIDeploymentsClient` and `PlatformKeyClient` are the supported
171+
surface — import from those, not from the generated tree, which is regenerated
172+
wholesale whenever the spec moves. The response models re-exported from
173+
`unstract.api_deployments` are generated from the same spec and are safe to
174+
import.
122175

123176
## Cloning an organization
124177

‎src/unstract/api_deployments/__init__.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,22 @@
11
__version__ = "1.6.0"
22

3+
from ._sdk_docstudio.models import (
4+
APIDeploymentSummary as APIDeploymentSummary,
5+
)
6+
from ._sdk_docstudio.models import (
7+
PaginatedAPIDeploymentSummaryList as PaginatedAPIDeploymentSummaryList,
8+
)
9+
from ._sdk_docstudio.models import (
10+
WhoAmIResponse as WhoAmIResponse,
11+
)
12+
from .client import APIDeploymentError as APIDeploymentError
313
from .client import APIDeploymentsClient as APIDeploymentsClient
4-
from .client import PlatformAPIClient as PlatformAPIClient
14+
from .client import (
15+
APIDeploymentsClientException as APIDeploymentsClientException,
16+
)
17+
from .client import PlatformClientError as PlatformClientError
18+
from .client import PlatformKeyClient as PlatformKeyClient
19+
from .client import UnstractError as UnstractError
520

621

722
def get_sdk_version():

0 commit comments

Comments
 (0)