Feature/301 async data adapter - #314
Merged
Merged
Conversation
Add an httpx-based async transport while sharing HTTP error and compatibility handling with the synchronous adapter.
Remove obsolete adapter imports and update the async adapter to build HTTP errors through the shared transport-neutral helpers.
Keep HTTP error construction resilient to unexpected response parsing failures and update exception tests for the shared HTTP helpers.
Adds a hand-rolled retry loop for AsyncMlbDataAdapter.get(), since httpx has no transport-level equivalent to urllib3's Retry mounted on the sync adapter's session. Reuses create_retry_policy() for total/backoff_factor/ status_forcelist/respect_retry_after_header so async stays consistent with the sync adapter's retry config, honors Retry-After, and only retries when the adapter owns its client. Closes #301. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Splits AsyncMlbDataAdapter's retry loop into read/connect/timeout/status budgets sourced from create_retry_policy(), matching the sync adapter's independent connect/read/status counters instead of a single uniform total bound. ConnectTimeout now correctly falls through to MlbTimeoutError rather than being bundled with ConnectError's MlbTransportError path. Also adds two regression tests: a plain 200 makes exactly one call with no retry, and the backoff wait actually yields the event loop (verified by temporarily swapping it for a blocking call and confirming the test catches it). Note: the ConnectError and TimeoutException exhaustion branches don't log via self._logger.error before raising, unlike the ReadTimeout and RequestError branches - worth a follow-up for logging consistency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…currency Adds regression coverage for gaps found during review: actual JSON payload parsing on 2xx, an explicit empty 204 response, structured MlbHttpError context (reason/url/method/response_data/body_excerpt), library-owned client close plus aclose() idempotence, injected clients staying open, scalar and tuple timeout translation to httpx.Timeout, multiple concurrent requests on one adapter, and cancelling one in-flight request leaving a sibling request on the same adapter unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A library-created httpx.AsyncClient now identifies itself as python-mlb-statsapi/<installed-version>, reusing _build_user_agent() from the sync adapter so the version lookup and the "unknown" source-only fallback stay defined in one place. Passing the header to the AsyncClient constructor replaces only User-Agent, leaving httpx's other defaults intact. A caller-injected client is used exactly as given: its headers are never read, replaced, or reconfigured. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFiLe3NhRL75YPrFCmQVZG
httpx.ConnectTimeout subclasses httpx.TimeoutException, so it fell
through to the generic timeout handler and spent the total retry
budget. It now has its own branch, ahead of TimeoutException, that
spends the connect budget while still raising MlbTimeoutError, matching
the sync retry contract:
ReadTimeout -> read budget -> MlbTimeoutError
ConnectTimeout -> connect budget -> MlbTimeoutError
ConnectError -> connect budget -> MlbTransportError
other TimeoutException -> total budget -> MlbTimeoutError
other RequestError -> total budget -> MlbTransportError
retryable HTTP status -> status budget
A failing connect error is now logged like the other exhausted retry
paths.
The _owned_adapter test helper created the adapter's library-owned
AsyncClient and then replaced it, leaving the original open. It now
swaps only the transport while the adapter builds its own client
through the production path, so exactly one client exists, ownership
and header behavior are unchanged, and run_async() closes it inside the
event loop that used it.
New focused coverage:
- connect timeout exhausts retries and raises MlbTimeoutError
- connect timeout spends the connect budget, not the total budget
- a final non-2xx outside 4xx/5xx raises MlbHttpError
- an injected client's timeout configuration is not mutated
- the package User-Agent leaves httpx's other default headers intact
- a JSON decode failure keeps the underlying error as its cause
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FaoU7oRx5LGn9ZKMufGbzd
AsyncMlbDataAdapter is public API per #298, but mlbstatsapi/__init__.py did not export it, and adding a plain import there would have made `import mlbstatsapi` require HTTPX for every sync-only install. Resolve the package-root async symbol lazily (PEP 562 module __getattr__ plus __dir__) and route the HTTPX import through a private boundary helper. A missing optional dependency now surfaces as an ImportError naming `pip install "python-mlb-statsapi[async]"`, chained from the original ModuleNotFoundError, and only when async functionality is requested. - add mlbstatsapi/_async_support.import_httpx() for the one actionable message - import HTTPX through it in async_mlb_dataadapter, so importing that module directly hits the same boundary - lazily export AsyncMlbDataAdapter from the package root and keep it in dir() - add tests/test_async_optional_dependency.py; every "HTTPX is missing" case runs in a child interpreter that blocks the import at sys.meta_path, so the results do not depend on sys.modules state from earlier tests - document the boundary in docs/public-api.md HTTPX remains optional and is not re-exported. Retry, timeout, User-Agent, and all synchronous behavior are unchanged. Refs #301 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXEufcdjaRsRM89BvaJuq5
Split the frozen package-root manifest so "public API" and "available without optional dependencies" are separate statements: * SUPPORTED_PACKAGE_ROOT_SYMBOLS is the always-available surface that sync-only environments freeze against * OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS holds the public async surface that needs the async extra * SUPPORTED_PACKAGE_ROOT_API is their union, the whole supported 1.x package-root API Tests now prove all three parts of the contract: the always-available symbols still import without HTTPX, AsyncMlbDataAdapter is public and importable when HTTPX is present, and it stays discoverable and reported against the async manifest in a sync-only install. The docs classification table gains an availability column and an AsyncMlbDataAdapter row, checked against the manifests so the two cannot drift. Also tighten the optional-dependency boundary: only a missing top-level httpx is rewritten into the install message. An installed but broken HTTPX fails on some other module and now reports its own error instead of pointing at an extra that would not fix it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014DLLrrKnwptxpJq3bVmNK5
HTTPX is optional, but both #301 test modules imported it at module scope, so `pytest tests/` errored during collection on a sync-only install instead of running the tests that do not need the extra. test_async_mlb_dataadapter.py exercises the HTTPX-backed adapter from end to end, so it now skips as a module via pytest.importorskip before importing AsyncMlbDataAdapter. Ordering is pytest, then the HTTPX check, then the async imports. test_async_optional_dependency.py deliberately does not skip: most of it asserts how an install without HTTPX behaves, which is exactly what a sync-only environment can prove. Its module-level async adapter import is gone; the two cases that need a real HTTPX skip individually and import the adapter inside the test. Sync-only environments now collect the whole offline suite and run every optional-dependency contract test, including the missing-HTTPX subprocess cases. No production behavior changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014DLLrrKnwptxpJq3bVmNK5
Mattsface
marked this pull request as draft
August 18, 2026 03:50
Mattsface
marked this pull request as ready for review
August 18, 2026 03:51
Member
Author
|
This looks good to me. Nothing super complicated. |
14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This PR adds the transport layer needed for first-class async support in v1.1.
The existing library only had a synchronous MlbDataAdapter, so async support needed its own adapter without changing the current sync API or making HTTPX a required dependency for everyone.
The goal was to make the async transport behave as closely as possible to the existing sync transport while keeping HTTPX optional.
What
Tests
Added focused offline coverage for:
The async adapter tests use httpx.MockTransport, so no live MLB API calls are required.
The existing synchronous behavior was also kept covered to make sure adding async support did not change the current API.
Risk and impact
Risk: Normal
This is a fairly large new transport path with retry, timeout, lifecycle, and error-handling behavior, so there is more surface area than a small feature change.
The risk to existing users is lower because the async implementation is additive. HTTPX remains optional, the synchronous API is unchanged, and sync-only installs do not import the async dependency.
If something does go wrong, the most likely impact would be incorrect behavior for users of the new async adapter, such as retry, timeout, cleanup, or error-mapping differences.
Existing synchronous users should be largely isolated from those failures because the async transport is separate and loaded only when requested.