Skip to content

Commit d4f0962

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. 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. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CnhmFYFRBrK376tZyDtwUM
1 parent 67e0197 commit d4f0962

5 files changed

Lines changed: 705 additions & 344 deletions

File tree

.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: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,30 @@ client = APIDeploymentsClient(
113113
The retry logic uses exponential backoff with full jitter and respects the `Retry-After` header on 429 responses.
114114

115115

116+
## Errors
117+
118+
Every error either client raises derives from `UnstractError`:
119+
120+
| Exception | Raised by |
121+
|-----------|-----------|
122+
| `UnstractError` | base of both — catch this to catch everything |
123+
| `APIDeploymentError` | `APIDeploymentsClient` |
124+
| `PlatformClientError` | `PlatformKeyClient` |
125+
126+
`APIDeploymentsClientException` is an alias of `UnstractError`, so existing
127+
`except` clauses keep working.
128+
129+
Transport failures are raised as the `requests` exception types
130+
(`ConnectionError`, `Timeout`, and friends) rather than the httpx ones.
131+
116132
## Internals
117133

118134
`unstract.api_deployments._sdk_docstudio` is generated from the deployment API's
119135
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.
136+
transport. `APIDeploymentsClient` and `PlatformKeyClient` are the supported
137+
surface — import from those, or from the response models re-exported alongside
138+
them, not from the generated tree, which is regenerated wholesale whenever the
139+
spec moves.
122140

123141
## Cloning an organization
124142

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)