Skip to content

Commit 526ecc7

Browse files
authored
fix: correctness, resource-handling, and documentation fixes across the SDK (#5)
PR: #5
1 parent 03a5f44 commit 526ecc7

132 files changed

Lines changed: 3929 additions & 637 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
A round of platform improvements to `dexpace-sdk-core`: new optional building
1111
blocks (typed serialization, webhook verification, pagination, two pipeline
1212
policies), tightened retry and tracing behaviour, and a batch of correctness
13-
fixes across bodies, SSE parsing, Digest auth, and error reporting. Everything
14-
lands in `core`; the transport packages are unchanged. No public symbol was
15-
removed, so existing code continues to work without modification.
13+
fixes across bodies, SSE parsing, Digest auth, and error reporting. Most of this
14+
lands in `core`; the transport adapters additionally get consistent connect- vs
15+
read-phase timeout classification and tighter resource release. The only removed
16+
public symbol is the unused `RetryConfig` (see Removed); existing code otherwise
17+
continues to work without modification.
1618

1719
### Added
1820

@@ -41,6 +43,12 @@ removed, so existing code continues to work without modification.
4143
- **Log correlation** (`instrumentation.correlation`). A `contextvar`-backed
4244
correlation id that flows through the pipeline and is attached to log records,
4345
so logs from a single logical request can be tied together.
46+
- **Reconnecting SSE client** (`http.sse.connection`). `SseConnection` and
47+
`AsyncSseConnection` resume an interrupted event stream by replaying the
48+
`Last-Event-ID` header and reconnecting with jittered backoff that honours the
49+
server's `retry:` hint. Built on the shared dispatch seam
50+
(`pipeline.dispatch`), which lets both the SSE client and the paginator accept
51+
either a pipeline or a bare send-callable.
4452

4553
### Changed
4654

@@ -60,6 +68,20 @@ removed, so existing code continues to work without modification.
6068
- **Error reporting** (`errors.http`). HTTP errors now expose whether they are
6169
`retryable` and carry a bounded body snapshot for diagnostics, with the
6270
snapshot capped so an error never holds an unbounded payload.
71+
- **`HttpRange.suffix`** (`http.common.http_range`) now returns a public
72+
`HttpRange` (carrying an `is_suffix` flag) instead of a private helper type,
73+
so a `bytes=-N` suffix range composes with `HttpRange.format_many` alongside
74+
ordinary ranges.
75+
- **`CallContext`** (`http.context`) is now an `abc.ABC`. It declares no
76+
abstract methods, so existing subclasses are unaffected; the change only
77+
prevents the base from being instantiated directly.
78+
79+
### Removed
80+
81+
- **`RetryConfig`** (`pipeline` / `pipeline.step.config`). It was exported but
82+
never wired into the retry policy, so it configured nothing; `RetryPolicy`'s
83+
constructor is the real configuration surface. Code that imported
84+
`RetryConfig` should configure `RetryPolicy` directly.
6385

6486
### Fixed
6587

CLAUDE.md

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,18 @@ bodies are modelled as typed Pythonic abstractions instead.
4343
`from_form` / `from_stream` / `from_iter` / `from_file`. `ResponseBody`
4444
exposes `iter_bytes` / `bytes` / `string`. Single-use bodies (stream /
4545
iter) raise `RuntimeError` on second consumption — call `to_replayable()`
46-
before the first send if retries are needed.
46+
before the first send if retries are needed. `AsyncRequestBody` /
47+
`AsyncResponseBody` are the async twins (`aiter_bytes`), and
48+
`MultipartField` / `MultipartRequestBody` build `multipart/form-data`
49+
payloads.
4750
- **Body capture for logging uses `BytesIO`.** `LoggableRequestBody` mirrors
4851
writes into a `BytesIO` tap; `LoggableResponseBody` caches drained bytes
4952
for repeatable reads. Both honour a configurable byte cap.
5053
- **Thread-safety where stated.** `ContextStore` is safe under concurrent
51-
use; individual bodies and streams are not. Per-context lookups rely on
52-
CPython's GIL for atomic dict ops and use a lock only for check-and-set.
54+
use; individual bodies and streams are not. Every store operation
55+
(`get` / `put` / `set` / `remove`) acquires a `threading.Lock`, so the
56+
guarantee survives free-threaded CPython (PEP 703) and runtimes without
57+
atomic dict ops rather than relying on the GIL.
5358
- **Public API is narrow.** Helpers and concrete adapter classes are
5459
module-private (leading underscore). The public surface for each subpackage
5560
is what its `__init__.py` re-exports.
@@ -103,14 +108,20 @@ python-sdk/
103108
│ │ │ ├── common/ # Headers, HttpHeaderName, MediaType,
104109
│ │ │ │ # Protocol, Url, QueryParams, ETag,
105110
│ │ │ │ # HttpRange, RequestConditions,
106-
│ │ │ │ # common_media_types
107-
│ │ │ ├── request/ # Request, RequestBody, FileRequestBody,
108-
│ │ │ │ # LoggableRequestBody, Method
109-
│ │ │ ├── response/ # Response, ResponseBody,
110-
│ │ │ │ # LoggableResponseBody, Status
111+
│ │ │ │ # common_media_types; pagination.py
112+
│ │ │ │ # (ItemPaged/Pager + async twins),
113+
│ │ │ │ # streaming.py (jsonl/chunked-frame iters)
114+
│ │ │ ├── request/ # Request, RequestBody, AsyncRequestBody,
115+
│ │ │ │ # FileRequestBody, LoggableRequestBody,
116+
│ │ │ │ # MultipartField/MultipartRequestBody, Method
117+
│ │ │ ├── response/ # Response, AsyncResponse, ResponseBody,
118+
│ │ │ │ # AsyncResponseBody, LoggableResponseBody,
119+
│ │ │ │ # Status
111120
│ │ │ ├── context/ # CallContext, DispatchContext,
112121
│ │ │ │ # RequestContext, ExchangeContext,
113122
│ │ │ │ # ContextStore
123+
│ │ │ ├── sse/ # Server-Sent Events parser + connection
124+
│ │ │ ├── webhooks/ # webhook signature verification
114125
│ │ │ └── auth/ # TokenCredential, BearerTokenPolicy,
115126
│ │ │ # BasicAuthPolicy, KeyCredentialPolicy,
116127
│ │ │ # ChallengeHandler (Basic/Digest/Composite),
@@ -119,19 +130,24 @@ python-sdk/
119130
│ │ │ │ # Stage, StagedPipelineBuilder, defaults,
120131
│ │ │ │ # sans-io + transport runners under the hood
121132
│ │ │ │
122-
│ │ │ ├── policies/ # retry, redirect, logging, tracing,
123-
│ │ │ │ # set_date (+ async twins)
124-
│ │ │ └── step/ # PipelineStep, StepMetadata, RetryConfig
133+
│ │ │ ├── policies/ # redirect, idempotency, retry, set_date,
134+
│ │ │ │ # client_identity, logging, tracing
135+
│ │ │ │ # (async twins only for the first five)
136+
│ │ │ └── step/ # PipelineStep, StepMetadata
125137
│ │ ├── client/ # HttpClient + AsyncHttpClient Protocols
126138
│ │ ├── config/ # Configuration
127139
│ │ ├── serde/ # Serde, Serializer, Deserializer Protocols
128140
│ │ ├── errors/ # SDK-level exception hierarchy
129141
│ │ ├── instrumentation/ # InstrumentationContext, Span, Tracer,
130-
│ │ │ # TracingScope, noops
142+
│ │ │ # TracingScope, noops, metrics,
143+
│ │ │ # correlation, client_logger, http_tracer,
144+
│ │ │ # identifiers, log_level, url_redactor
145+
│ │ ├── pagination/ # Page, Paginator, link-header + strategy
131146
│ │ └── util/ # clock, proxy helpers
132147
│ └── tests/ # pytest suite — auth/, config/, context/,
133148
│ # errors/, http/, instrumentation/,
134-
│ # pipeline/, serde/, sse/, util/
149+
│ # pagination/, pipeline/, serde/, sse/,
150+
│ # util/, webhooks/
135151
├── dexpace-sdk-http-stdlib/ # reference stdlib transports:
136152
│ │ # UrllibHttpClient, AsyncioHttpClient
137153
│ └── src/dexpace/sdk/http/stdlib/
@@ -143,6 +159,10 @@ python-sdk/
143159
└── src/dexpace/sdk/http/requests/
144160
```
145161

162+
Community-health and tooling files (`CHANGELOG.md`, `CONTRIBUTING.md`,
163+
`SECURITY.md`, `CODE_OF_CONDUCT.md`, `conftest.py`, `tools/`) are elided from
164+
the tree above.
165+
146166
Every transport package depends on `dexpace-sdk-core` and adapts its HTTP
147167
library to the `HttpClient` / `AsyncHttpClient` Protocols. Namespace
148168
packaging (no `__init__.py` at `src/dexpace/`, `src/dexpace/sdk/`, or
@@ -185,12 +205,16 @@ Layered, bottom-up:
185205
evict on `CallContext.close()`.
186206
4. **`pipeline`**`Policy` (and `AsyncPolicy`) wrap the downstream chain;
187207
`Pipeline` / `AsyncPipeline` run an ordered set of policies grouped into
188-
`Stage`s via `StagedPipelineBuilder`. Shipped policies: retry, redirect,
189-
logging, tracing, set-date (each with an async twin under
190-
`pipeline/policies/`). `default_pipeline()` / `default_async_pipeline()`
191-
assemble the standard stack. The lower-level `pipeline/step/PipelineStep`
192-
Protocol (`(input, context) -> output`) plus `StepMetadata` / `RetryConfig`
193-
remain for custom composition.
208+
`Stage`s via `StagedPipelineBuilder`. Shipped policies: redirect,
209+
idempotency, retry, set-date, client-identity, logging, tracing. Async
210+
twins under `pipeline/policies/` exist only for redirect, idempotency,
211+
retry, set-date, and client-identity; logging and tracing are sync-only.
212+
`default_pipeline()` / `default_async_pipeline()` assemble the standard
213+
stack in the order redirect → idempotency → retry → set-date →
214+
client-identity → [auth] → logging → tracing (the async pipeline omits
215+
logging and tracing). The lower-level `pipeline/step/PipelineStep` Protocol
216+
(`(input, context) -> output`) plus `StepMetadata` remain for custom
217+
composition.
194218
5. **`client/HttpClient`** — single-method Protocol
195219
(`execute(request) -> Response`). Transport is **not** provided by `core`;
196220
the `dexpace-sdk-http-*` packages (stdlib, httpx, aiohttp, requests) each

README.md

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,9 @@ with UrllibHttpClient() as client, client.execute(request) as response:
7979
### A configured pipeline
8080

8181
`default_pipeline()` returns a `StagedPipelineBuilder` pre-wired with the
82-
canonical policy stack (redirect, retry, set-date, logging, tracing). Add
83-
authentication and adjust whatever the defaults get wrong for you:
82+
canonical policy stack (redirect, idempotency, retry, set-date,
83+
client-identity, logging, tracing). Add authentication and adjust whatever
84+
the defaults get wrong for you:
8485

8586
```python
8687
from dexpace.sdk.core.http.auth import BearerTokenPolicy
@@ -112,7 +113,8 @@ from dexpace.sdk.core.http.request import RequestBody
112113
RequestBody.from_stream(open("payload.bin", "rb"))
113114
RequestBody.from_iter([b"chunk-1", b"chunk-2"])
114115

115-
# Replayable (transports may use zero-copy sendfile)
116+
# Replayable; a transport could special-case file bodies (e.g. zero-copy
117+
# sendfile), though none of the shipped transports do so today
116118
RequestBody.from_file("upload.bin")
117119

118120
# Convert any single-use body into a replayable one before retrying
@@ -130,8 +132,9 @@ the way back up. The terminal policy hands the request to an `HttpClient`
130132
transport.
131133

132134
```
133-
caller → Pipeline → REDIRECT → RETRY → SET_DATE → AUTH → LOGGING → POST_LOGGING → HttpClient → wire
134-
(pillar) (pillar) (pillar) (pillar)
135+
caller → Pipeline → REDIRECT → POST_REDIRECT → RETRY → POST_RETRY → [AUTH] → LOGGING → POST_LOGGING → HttpClient → wire
136+
(pillar) idempotency (pillar) set-date (pillar) (pillar) tracing
137+
client-identity
135138
```
136139

137140
Ordering is governed by `Stage`, an `IntEnum` whose values sit 100 apart so
@@ -169,12 +172,14 @@ Bottom-up, the layers are:
169172
| `http.common` | `Headers`, `HttpHeaderName`, `MediaType`, `Protocol`, `Url`, `QueryParams`, `ETag`, `HttpRange`, `RequestConditions`, paging primitives |
170173
| `http.context` | `CallContext``DispatchContext``RequestContext``ExchangeContext` chain, `ContextStore` |
171174
| `http.auth` | `BearerTokenPolicy`, `BasicAuthPolicy`, `KeyCredentialPolicy`, `DigestChallengeHandler`, RFC 7235 challenge parser, `TokenCache` |
172-
| `http.sse` | `SseParser` for Server-Sent Events streams |
175+
| `http.sse` | `SseParser`, plus reconnecting `SseConnection` / `AsyncSseConnection` (Last-Event-ID replay + backoff) |
176+
| `http.webhooks` | `WebhookVerifier`, `InvalidWebhookSignatureError` — HMAC signature verification with timestamp tolerance |
177+
| `pagination` | `Page`, `Paginator` / `AsyncPaginator`, `PaginationStrategy` (`CursorStrategy`, `PageNumberStrategy`, `LinkHeaderStrategy`) |
173178
| `pipeline` | `Pipeline`, `AsyncPipeline`, `Policy` ABC, `Stage` enum, `StagedPipelineBuilder`, `default_pipeline()` |
174-
| `pipeline.policies` | `RetryPolicy`, `RedirectPolicy`, `SetDatePolicy`, `LoggingPolicy`, `TracingPolicy` (+ async twins) |
179+
| `pipeline.policies` | `RedirectPolicy`, `IdempotencyPolicy`, `RetryPolicy`, `SetDatePolicy`, `ClientIdentityPolicy`, `LoggingPolicy`, `TracingPolicy` (async twins for all but logging/tracing) |
175180
| `client` | `HttpClient` and `AsyncHttpClient` Protocols |
176181
| `serde` | `Serde`, `Serializer`, `Deserializer` Protocols + `JsonSerde` reference impl |
177-
| `instrumentation` | `ClientLogger`, `UrlRedactor`, `Tracer`, `Span`, `InstrumentationContext`, noop singletons |
182+
| `instrumentation` | `ClientLogger`, `UrlRedactor`, `Tracer`, `Span`, `InstrumentationContext`, `contextvars` correlation helpers, noop singletons |
178183
| `errors` | `SdkError` hierarchy: `ServiceRequestError`, `ServiceResponseError`, `HttpResponseError[ModelT]`, … |
179184
| `util` | `Clock`, `AsyncClock`, `ProxyOptions` |
180185
| `config` | `Configuration` (layered env-var + override lookup) + `ConfigurationBuilder` |
@@ -203,7 +208,16 @@ Bottom-up, the layers are:
203208
OpenTelemetry-compatible spans via `TracingPolicy`, URL redaction with
204209
allowlisted query parameters, and capped body capture for diagnostics.
205210
- **Server-Sent Events.** A WHATWG-compliant `SseParser` with a bounded
206-
line buffer.
211+
line buffer, plus reconnecting `SseConnection` / `AsyncSseConnection`
212+
that resume with `Last-Event-ID` and honour server `retry:` backoff.
213+
- **Pagination.** A top-level `pagination` package: `Page`, sync and async
214+
`Paginator`s that iterate item-by-item or page-by-page, and pluggable
215+
`PaginationStrategy` (cursor, page-number, and `Link`-header).
216+
- **Webhooks.** `WebhookVerifier` checks HMAC signatures with a timestamp
217+
tolerance and constant-time comparison, raising
218+
`InvalidWebhookSignatureError` on mismatch.
219+
- **Correlation.** `contextvars`-based trace/span propagation so the
220+
idempotency and client-identity policies and logging share one id.
207221
- **A lean core.** `dexpace-sdk-core` carries a single runtime dependency
208222
(`furl`, which backs `Url` parsing); each transport adapter adds exactly
209223
one HTTP library.
@@ -220,8 +234,8 @@ uv sync
220234
```
221235

222236
```bash
223-
uv run pytest -q # 646 tests across 5 packages
224-
uv run mypy --strict # type-check (171 source files)
237+
uv run pytest -q # run the full test suite across 5 packages
238+
uv run mypy --strict # type-check every package under strict mode
225239
uv run ruff check # lint
226240
uv run ruff format --check # formatting gate
227241
```

docs/architecture.md

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@ plug in a concrete transport via the `HttpClient` Protocol.
1717
│ - Pipeline │ │ - BearerToken │
1818
│ - Policy │ │ - KeyCredential │
1919
│ - PipelineStep │ │ - BasicAuth │
20-
│ - retry, log, │ │ - TokenCache │
20+
│ - redirect, │ │ - TokenCache │
21+
│ idempotency, │ │ │
22+
│ retry, │ │ │
23+
│ set-date, │ │ │
24+
│ client- │ │ │
25+
│ identity, │ │ │
26+
│ logging, │ │ │
2127
│ tracing │ │ │
2228
└──────────┬────────┘ └─────────────────────┘
2329
@@ -32,9 +38,13 @@ plug in a concrete transport via the `HttpClient` Protocol.
3238
3339
┌──────────▼─────────────────────────────────────────┐
3440
│ client/ HttpClient + AsyncHttpClient │
41+
│ - Protocols only; transports plug in here │
42+
└──────────┬─────────────────────────────────────────┘
43+
44+
┌──────────▼─────────────────────────────────────────┐
45+
│ dexpace-sdk-http-stdlib (separate distribution) │
3546
│ - UrllibHttpClient (sync reference) │
3647
│ - AsyncioHttpClient (async reference) │
37-
│ - real transports plug in here │
3848
└────────────────────────────────────────────────────┘
3949
```
4050

@@ -67,5 +77,5 @@ The Java port has an `IoProvider` / `Buffer` / `Source` / `Sink` layer
6777
(a port of Okio). In Python, `bytes` / `bytearray` / `memoryview` /
6878
`BytesIO` / `BinaryIO` already cover the same surface idiomatically.
6979
Bodies use `iter_bytes(chunk_size)` for streaming and ordinary stdlib
70-
primitives for everything else. See `to-implement.md` for the design
71-
rationale.
80+
primitives for everything else. See the "Things That Will Bite You"
81+
section in `CLAUDE.md` for the design rationale.

docs/auth.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,10 @@ All concrete credentials redact secrets in their `__repr__`.
3333
- Calls `AccessTokenInfo.needs_refresh()` before each request; refreshes
3434
proactively when `refresh_on` has passed or `expires_on - leeway`
3535
(default 300 s) has been reached.
36-
- On a 401 response with a `WWW-Authenticate` header, invalidates the
37-
cached token and calls `on_challenge(request, response)`. Override
38-
`on_challenge` in a subclass to handle CAE / claims-challenge flows.
36+
- On any 401 response, invalidates the cached token. If the response
37+
also carries a `WWW-Authenticate` header, then calls
38+
`on_challenge(request, response)`. Override `on_challenge` in a
39+
subclass to handle CAE / claims-challenge flows.
3940

4041
## Token cache
4142

0 commit comments

Comments
 (0)