fix(http): stop replaying POST on a gateway 5xx

[anoop-narang]

The bug

HotdataClient passed its own retries= into Configuration:

Retry(total=3, connect=3, read=3, backoff_factor=0.2,
      status_forcelist=(502, 503, 504),
      allowed_methods=frozenset(["GET","HEAD","POST","PUT","DELETE","OPTIONS","PATCH"]))

Configuration(retries=...) replaces the SDK's policy rather than adding to it, and this one puts POST in allowed_methods next to a 5xx forcelist. So any intermediary with a request timeout — a gateway, a load balancer — answering 502/503/504 on a slow request causes urllib3 to silently re-send that request, up to three times.

That is fine for a GET. It is not fine for a load: the call is not idempotent, and the server is still working on the first one. The duplicate arrives while the original holds the table's write lock, is refused as a conflict within milliseconds, and the caller sees contention instead of the timeout that actually happened. Retry logic layered above then re-uploads and re-sends against a lock that is still held, so the whole load fails without the original ever having failed.

The function's own docstring says it is for "transient connection failures (e.g. stale pooled sockets)" — a transport-layer concern. Catching response status codes was not the intent.

The fix

Drop the override so the generated SDK's default applies. That default is the policy this wrapper was reaching for, and hotdata._retry states the reasoning directly:

Don't retry on response status codes by default: a status code means the request reached the server... Connection-reset retry is purely a transport-layer concern.

and

Read timeouts and status retries are left untouched and stay idempotent-only, so a POST that may have reached the server is never blindly replayed.

ConnectionResetRetry still retries a pre-response connection reset on any method including POST — the stale-pooled-socket case this wrapper was written for, where the connection was already gone when urllib3 tried to send and the server did no work. That behaviour is preserved; only the unsafe status/read retry on POST goes away.

Confirmed the floor already carries it — hotdata 0.8.0 (our pinned minimum) resolves default_retry() to status=0, empty forcelist, allowed_methods without POST.

Behaviour change to be aware of

A request that previously got three silent retries on a gateway 5xx now surfaces the 5xx to the caller on the first one. For a slow non-idempotent call that is the point — it fails cleanly instead of duplicating itself. For genuinely transient gateway blips on reads, there is currently no way to opt back in through this wrapper — HotdataClient.__init__ takes no retries= and builds Configuration itself. Adding a passthrough is a small, separate change; deliberately not bundled here, since the default this PR restores is the safe one.

Removed

hotdata_framework/http.py and default_http_retries(). The module existed only to build this policy, has no other callers in the repo, and is not in hotdata_framework.__all__. It predates hotdata._retry, which supersedes it. Called out under Removed in the changelog since this is a published package.

Tests

tests/test_retry_policy.py pins the resulting policy rather than the absence of an argument, so reintroducing an override that is unsafe for POST fails here:

• no status forcelist, and status == 0
• POST not in allowed_methods
• a pre-response connection reset is still retried on any method, while a non-reset ProtocolError is not reclassified

Verified they have teeth by running the previous policy against the same assertions — the first two fail. Full suite: 133 passed.

Lint is clean on the changed files; client.py's import-ordering warning and test_request_timeout.py's long line both already fail on main and are left alone.

[claude]

The reasoning holds up: Configuration(retries=) replaces rather than merges, and the old policy paired a 5xx forcelist with POST in allowed_methods, which is a silent replay of a non-idempotent load. Dropping the override is the right fix, and nothing else in the repo imported hotdata_framework.http (it wasn't in __all__, isn't referenced in CONTRACT.md, and the removal is called out in the changelog). hotdata is locked at 0.8.0 in uv.lock, matching the floor the description reasons about, and the release-metadata check will pass since the version is unchanged.

Two things I could not verify and am not claiming: the three assertions in tests/test_retry_policy.py all describe hotdata 0.8.0's default_retry(), and that package isn't installed in the review checkout — so whether retries is a ConnectionResetRetry with status == 0 and no POST rests on the CI run, which had not reported when this review started. If Configuration's default were None instead, test_no_status_forcelist would fail on None.status_forcelist rather than pass vacuously, so the test does have a floor under it either way.

Non-blocking notes inline on the test comment wording and on the now-stale README bullet.

[claude]

Prior nits addressed: the test comment now matches the value constructed and notes the urllib3-private dependency, and the README bullet is in sync with the change. No new findings.