Release 1.1.0: first-class async client support - #334
Merged
Conversation
Move MLB envelope parsing into reusable helpers so endpoint methods can build Pydantic models in one place.
The list parsers expect the response envelope, not the inner array. Wire people, team, and schedule endpoints through those helpers and align parser tests with the real model required fields.
Reuse collection parsers for person and team responses, handle empty schedules, and align parser tests with real API payloads.
…onse-parsers Feature/299 shared response parsers
…ging build: add optional HTTPX async dependency
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
…adapter Feature/301 async data adapter
#312: Update CI to run async coverage and preserve sync-only installation checks
Bring the existing Claude Code and Claude Code Review GitHub Actions workflows over from main so they run for pull requests targeting release/1.1.0. Both files are byte-identical to their versions on main; no other files are touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XhnfQFr66k3NRTN3GG7DEF
…ows-release-1.1.0 Add Claude workflows to release/1.1.0
Covers the remaining HTTP/result and error/warning contract gaps from #298 for AsyncMlbDataAdapter, without duplicating the suite added in #301/#314: - final non-404 4xx (403) raises MlbHttpError under strict_http=True, with structured status/reason/URL/method context - final non-404 4xx under strict_http=False emits one MlbHttpCompatibilityWarning and returns the historical empty MlbResult - compatibility warnings do not leak response bodies or headers - compatibility warnings are attributed to the awaiting caller's call site - a failure while extracting optional error-response context degrades that field instead of replacing the original MlbHttpError Test-only change. Existing helpers (run_async, _ScriptedHandler, _response, _owned_adapter, SLEEP_TARGET) and httpx.MockTransport are reused; no live MLB API requests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHYA36k526xUzTUxx4iPSZ
The default retry policy uses total=3, connect=3 and status=3, so the existing exhaustion tests that observe four attempts cannot tell those budgets apart, and the generic timeout/request-error branches had no coverage at all. Narrow one budget per test so the observed attempt count is uniquely attributable to it: - generic failures (pool timeout, read error) spend the total budget and still surface MlbTimeoutError / MlbTransportError with the original cause - a connection failure spends the connect budget, not the total one - a retryable status spends the status budget, not the total one Test-only change; no production behavior was modified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHYA36k526xUzTUxx4iPSZ
httpx.ReadError currently falls through to the generic RequestError branch and so spends the total budget, but #298 does not define that mapping, and asserting it would freeze an implementation detail as public contract. The read budget already has deterministic coverage through ReadTimeout, and the pool timeout case is enough to prove the generic timeout path spends the total budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHYA36k526xUzTUxx4iPSZ
The suite already proved two concurrent requests can share one adapter and that cancelling one does not cancel another. These lock down the remaining #298 concurrency promises: - concurrent requests keep their own ep_params and their own response, now asserted against the query the transport actually observed - a request that exhausts its retry budget and raises MlbHttpError leaves an unrelated concurrent request untouched, on its single attempt - a second request completes while the first is parked inside its retry backoff, proven with asyncio.Event synchronization rather than wall-clock timing, and bounded so a serializing regression fails fast instead of hanging CI Test-only change; no production behavior was modified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHYA36k526xUzTUxx4iPSZ
test_library_owned_client_closes only covers an adapter that never issued a request, so nothing asserted that a used adapter is still closable. Cover the three states a request can leave behind: - after a successful request, aclose() closes the library-owned client - after a request that raised MlbHttpError, cleanup succeeds and the error's public fields are unchanged - after an in-flight request is cancelled, CancelledError stays the caller's outcome and cleanup still closes the client The cancellation test waits on an event set inside the transport handler, so the request is genuinely in flight before it is cancelled. Test-only change; no production behavior was modified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHYA36k526xUzTUxx4iPSZ
The async suite proved the 5xx raise only under the default strict adapter; every strict_http=False test targeted a 4xx. Nothing stopped a regression that widened compatibility-mode suppression from the 4xx branch into the 5xx branch, which would have returned a warned empty MlbResult with the suite still green. A persistent 503 against a strict_http=False adapter still raises MlbHttpError after the full status retry budget, and emits no MlbHttpCompatibilityWarning. Test-only change; no production behavior was modified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHYA36k526xUzTUxx4iPSZ
Wraps the reusable async logic in a plain function so it stays valid Python without prescribing a main() entry point; the asyncio.run() wrapper is now clearly marked as just one way to invoke it. Co-authored-by: Matthew Spah <2068393+Mattsface@users.noreply.github.com>
Wrapping every advanced async example in a main()/asyncio.run() entry point isn't practical for readers integrating into an existing app. Show a script entry point alongside patterns for an already-running event loop, FastAPI, and interactive/notebook use with top-level await. Co-authored-by: Matthew Spah <2068393+Mattsface@users.noreply.github.com>
- README now states plainly that AsyncMlb mirrors the full Mlb endpoint surface, matching docs/async.md and docs/public-api.md, instead of hedged wording that implied partial coverage. - Drop the README callout singling out get_schedule as available on both clients, since that only made sense under partial coverage. - Fix get_awards, get_season, get_seasons, and get_people_id signatures in docs/methods.md to match mlbstatsapi/mlb_api.py. Co-authored-by: Matthew Spah <2068393+Mattsface@users.noreply.github.com>
PR #323 moved async retries into MlbAsyncRetryTransport, mounted via AsyncClient(transport=...). HTTPX only builds its own env-proxy mounts when the caller leaves transport=None (allow_env_proxies = trust_env and transport is None in Client.__init__), so passing a transport silently disabled HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY support, leaving callers behind a proxy with an unexplained hang. create_library_async_client() now rebuilds that proxy discovery from the stdlib (mlbstatsapi/_env_proxies.py, no private httpx APIs) and passes it through HTTPX's public mounts= argument, wrapping every proxy transport in the same retry transport used for direct requests so retries still apply behind a proxy. A caller-injected client is untouched. Fixes #324.
Test and docs fixes only, no changes to _env_proxies.py logic or create_library_async_client() wiring: - The aclose() test's fixture had no NO_PROXY entry, so its mount map had no None value and the "none closed twice" assertion held regardless of whether the bypass branch mounted None or reused `direct`. Added a NO_PROXY fixture and pinned proxy_mounts to an explicit length so the test now fails if that branch regresses (verified locally, then reverted). - The proxied-retry test asserted nothing about which transport actually served the request; both the direct transport and the https:// mount wrapped the same MockTransport, so resolution could have silently fallen back to direct. Added an explicit _transport_for_url() assertion before the request. - Split the module: tests/test_env_proxies.py now covers only environment_proxy_map()'s pure stdlib parsing and carries no httpx import, so it runs in the no-httpx CI job instead of skipping with everything else. tests/test_async_env_proxies.py keeps the client-wiring, cleanup, and differential tests behind the module-level httpx importorskip guard. - Renamed the "No async support in this section" heading in docs/http-transport.md to "Scope of this document". - Reworded the _env_proxies.py docstring to point at the differential test as the automated drift check, rather than implying a manual recheck is needed. - Closed the httpx.AsyncClient left open in the injected-client test.
Two review findings from #331, no proxy logic or test coverage changed: - docs/releases/1.1.0.md and its release-validation classification are #307's responsibility (version bump, release notes, final release validation), not #324's. Removed the file and reverted tests/test_release_validation.py to its release/1.1.0 state so 1.1.0 is not prematurely classified as historical release notes. - create_library_async_client(*, trust_env=True) only threaded trust_env into environment_proxy_map(); it never reached AsyncClient or AsyncHTTPTransport, so it did not represent full HTTPX trust_env semantics and wasn't exposed by any public constructor. Removed the parameter; the factory now always runs environment discovery, matching the trust_env=True default a caller gets from a plain httpx.AsyncClient(). environment_proxy_map() keeps its own trust_env parameter and test, since it is a pure helper.
…port fix: honor environment proxies for library-created async clients
Member
Author
|
@claude look over this release please |
|
Claude finished @Mattsface's task in 6m 31s —— View job Reviewing release/1.1.0 → mainTodo list
Overall this is a well-executed, thoroughly documented release. The sync/async parity work is careful and the async transport design is genuinely nice engineering. A few notes, nothing blocking: Strengths
Minor observations (non-blocking)
None of these affect the core claims in the PR description (sync backward compatibility, async parity, packaging correctness) — I was not able to execute |
The async installed-artifact smoke test exercised AsyncMlbDataAdapter only indirectly through AsyncMlb(), which owns and passes its own client. Add a direct check that constructs the public standalone AsyncMlbDataAdapter() with client=None, confirming its default strict_http=True, its library-owned httpx.AsyncClient, and safe/ idempotent cleanup via aclose().
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
Release
python-mlb-statsapi1.1.0.The primary goal of 1.1.0 is to add first-class asynchronous access to the MLB Stats API while preserving the existing synchronous 1.x contract.
This release introduces
AsyncMlbandAsyncMlbDataAdapter, expands the async client to the full supported endpoint surface, and establishes deterministic sync/async parity, transport, lifecycle, cancellation, concurrency, packaging, and release-validation coverage.Existing 1.0.x users of
MlbandMlbDataAdaptershould require zero code changes when upgrading to 1.1.0.What
Async API
AsyncMlbAsyncMlbDataAdapter[async]installation extra backed by HTTPXAsyncMlbaclose()httpx.AsyncClientinstancesSync/async parity
HTTP transport
strict_http=Trueas the defaultstrict_http=Falsethroughout the 1.x release series as the documented compatibility pathPackaging and release validation
1.1.0[async]installation separatelyTests
The current
release/1.1.0head passes the full deterministic Offline CI workflow.Validated in CI:
[async]artifact installstwine checkagainst built artifactsRelease artifact smoke validation is deterministic and does not contact the live MLB API.
Risk and impact
Risk level: Normal
This is a substantial minor release that adds a new public asynchronous API and refactors some internal parsing/transport behavior to support sync/async parity.
The primary compatibility requirement is that the existing synchronous 1.x API remains unchanged for callers. That contract has extensive deterministic coverage, and the base installation remains sync-only without requiring HTTPX.
The new async functionality is opt-in through the
[async]extra, which limits the impact on existing users.If something does go wrong, the highest-impact failures would be:
The release validation added for 1.1.0 specifically exercises the built artifacts in clean sync and async environments to reduce those packaging and compatibility risks.
Closes #307