Skip to content

feat!: migrate from requests to httpx2 with sync/async clients - #18

Open
dbritto-dev wants to merge 13 commits into
mainfrom
chore/httpx2-migration
Open

feat!: migrate from requests to httpx2 with sync/async clients#18
dbritto-dev wants to merge 13 commits into
mainfrom
chore/httpx2-migration

Conversation

@dbritto-dev

Copy link
Copy Markdown
Owner

Summary

  • Replaces requests/requests_aws4auth with httpx2, restructured around a unified Client (sync) / AsyncClient (async) pair — Anthropic/OpenAI SDK style — exposing reports and vendor.orders as resources. The async resource classes are hand-written; the sync ones are generated from them via unasync (scripts/generate_sync.py, checked in CI).
  • AWS SigV4 + LWA auth reimplemented as an httpx2.Auth subclass (_auth.py); AWS STS credential refresh is offloaded to a thread on the async path so it never blocks the event loop.
  • New automatic retries on 429/5xx and connection/timeout errors (honors Retry-After, exponential backoff + jitter, max_retries=0 to opt out), a typed APIStatusError/APIConnectionError/APITimeoutError hierarchy, and a with_raw_response accessor on every resource method.
  • Models upgraded from pydantic v1 to v2.
  • Old amzn_selling_partner.reports.Client() / amzn_selling_partner.vendor.orders.Client() still work, now as deprecated shims that build a Client internally and delegate.
  • Test suite rewritten from responses to httpx2.MockTransport, parametrized across sync and async (tests/conftest.py's client_factory + maybe_await).

Full breaking-change list, rationale, and a known limitation (async STS calls running in a thread) are in MIGRATION.md.

Test plan

  • uv run nox -s lint — ruff check + format
  • uv run nox -s type_checkty check src/amzn_selling_partner
  • uv run nox -s test — 88 tests pass, 93% coverage (gate: 90%)
  • uv run nox -s unasync_check — generated _sync.py files match regeneration
  • uv run nox -s security_test — bandit + safety clean
  • Manual sandbox smoke test against the real SP-API (not run in this environment — no sandbox credentials available)

🤖 Generated with Claude Code

Replace the requests-based SDK with httpx2, restructured around a unified
Client/AsyncClient (Anthropic/OpenAI SDK style) exposing reports and
vendor.orders as resources. Adds automatic retries, a typed error
hierarchy, with_raw_response, and upgrades models to pydantic v2. Old
per-resource Client() entry points remain as deprecated shims. See
MIGRATION.md for the full list of breaking and behavior changes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 7, 2026 15:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

dbritto-dev and others added 12 commits September 7, 2026 10:23
Drop the unasync-based generation step. OpenAI's Python SDK (the
architecture this migration follows) doesn't derive its sync classes
from async ones via a script — both come out of its own spec-driven
codegen as independent, hand-maintained-looking code. This repo has no
such pipeline, so the faithful equivalent is to hand-maintain
reports/_sync.py and vendor/orders/_sync.py directly alongside their
_async.py counterparts, rather than regenerating them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace automatic aiohttp-transport detection with OpenAI's actual
pattern: explicit DefaultHttpxClient/DefaultAsyncHttpxClient/
DefaultAioHttpClient classes that users opt into via http_client=,
never selected implicitly just because the aiohttp extra happens to
be installed.

This also fixes a real latent bug the change surfaced: a
caller-supplied http_client= previously bypassed this SDK's auth and
base URL entirely, since both were only ever applied at the
underlying httpx2 client's own construction time. Auth and URL
resolution now happen explicitly in BaseClient, per request, so they
apply correctly regardless of whether the httpx2 client was built by
this SDK or handed in by the caller.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds tests/test_sigv4.py, which independently recomputes the expected
AWS SigV4 signature from AWS's published algorithm using only stdlib
hashlib/hmac -- no botocore, no requests_aws4auth -- and checks it
against SPAPIAuth._sign()'s actual output for POST-with-body and
GET-without-body requests on both resource paths.

This was cross-checked manually against the old requests_aws4auth
implementation for a fixed request: the computed body-hash matched
exactly, confirming the new botocore-backed signer is a faithful
replacement, not just independently self-consistent.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This release removes requests/requests_aws4auth, upgrades models to
pydantic v2, and removes amzn_selling_partner.client.auth entirely --
all documented as breaking changes in MIGRATION.md. Warrants a major
version bump under semver.

Note: release.yml's auto-release workflow always does a patch-only
version bump on every push to main (uv version --bump patch), so on
merge this will actually be tagged and published as 1.0.1, not 1.0.0
exactly -- setting the base to 1.0.0 here is what gets that bump into
the 1.x line at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
uv version --bump patch only ever increments the patch digit, so it
can never land on a version ending in .0 -- no starting value in
pyproject.toml could make the previous unconditional patch-bump
produce 1.0.0. Revert the manual version bump (0.1.9, matching main)
and instead make release.yml bump major only while the current major
version is still 0; a 0.x major bump always resets to X.0.0, so
0.1.9 -> 1.0.0 exactly on merge.

This is self-limiting: once the version is 1.x or higher, every
future push falls through to the normal patch bump again, so no
manual revert of this workflow change is needed after this release.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The 0.x -> 1.0.0 major-bump transition is unchanged. For everything
after that, release.yml now runs `uv version --bump minor` instead of
`--bump patch` on every push to main.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Also sets pyproject.toml's version to 1.0.0 directly, since a plain
unconditional minor bump can never produce a version ending in .0 on
its own -- on merge this becomes 1.1.0, not 1.0.0 exactly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Mirrors the OpenAI Python SDK's _utils/_path.py: percent-encodes each
interpolated value for safe use in a URL path segment, and rejects a
resulting path containing a dot-segment (., .., or their
percent-encoded forms), so a caller-supplied id can never redirect a
request via ../ traversal. Plain f-string interpolation did neither.

Wires it into the three call sites that interpolate a caller-supplied
value into a URL: get_report, get_report_document, and
get_purchase_order (both resources, sync and async).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds tests for every previously-uncovered branch: STS credential
construction/failure (only ever exercised via the autouse dummy
mock before now), LWA access_token guard, retry backoff without a
Retry-After header, Retry-After as an HTTP-date and as an
unparseable value, connection-error retry/exhaustion,
non-JSON error bodies, unmapped non-5xx status codes, with_raw_response
variants that weren't called anywhere, invalid-id guards on
get_report_document/get_report_document_content/
download_report_document_content, the vendor.orders deprecated
shim's endpoint helpers, and DefaultAioHttpClient's ImportError
fallback (via a sys.modules hide-and-reload).

Also removes one genuinely dead branch in _parse_retry_after:
email.utils.parsedate_to_datetime always raises ValueError for
unparseable input on Python 3.10+ (verified empirically), so it
never returns None -- the pre-existing None-check was unreachable
given this package's Python floor. Marks the one branch that can't
be deterministically forced (async double-checked-locking race,
since asyncio.Lock.acquire() never suspends when uncontended) with
an explained pragma: no branch instead of a misleading fake test.

Raises the coverage gate from 90% to 100% to hold the line going
forward.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The 100% coverage achieved in the previous commit stands, but the
gate itself stays at 90% rather than being raised to enforce it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Was hardcoded as 'danilo-poc/0.0.1 (Language=...; Platform=...)', a
leftover POC placeholder carried over from the original requests-based
client. OpenAI's SDK uses a simple '{Name}/Python {version}' format
with no embedded platform details (those go in separate headers,
which this SDK doesn't have and isn't adding here -- out of scope for
'update the user agent'). Now: 'amzn-selling-partner/Python {version}',
reading the installed package version dynamically.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants