diff --git a/docs/public-api.md b/docs/public-api.md index 93bbdfd..6a3d6a7 100644 --- a/docs/public-api.md +++ b/docs/public-api.md @@ -272,13 +272,17 @@ promised. `AsyncMlb` constructs internal adapters for both `v1` and `v1.1` that share one HTTPX client, mirroring `Mlb`'s shared-Session pattern. Most endpoint -methods use `v1`. `get_game` uses the `v1.1` live feed endpoint. The `v1` -adapter resolves and owns the shared client (library-created when the caller -passes none to `AsyncMlb`, otherwise the caller's own); the `v1.1` adapter -borrows that same client and never closes it directly. Retry eligibility -follows the shared client's ownership on both adapters, not which adapter -version issues a given request, matching `Mlb`'s single retry policy mounted -on the shared `Session`. +methods use `v1`. `get_game` uses the `v1.1` live feed endpoint. `AsyncMlb` +owns the shared client, exactly as `Mlb` owns the shared `Session`: it creates +one when the caller passes none, closes only a client it created, and hands +the same client to both adapters. + +Retries are a property of that client, not of either adapter. A +library-created client is built with the library retry transport mounted on +it, the way a library-created `Session` is built with the library retry +adapters mounted on it, so both API versions retry identically without either +adapter holding retry state. A caller-injected client keeps whatever transport +its caller mounted. ### Endpoint methods diff --git a/mlbstatsapi/_async_transport.py b/mlbstatsapi/_async_transport.py new file mode 100644 index 0000000..816e34e --- /dev/null +++ b/mlbstatsapi/_async_transport.py @@ -0,0 +1,165 @@ +"""Retry-aware HTTPX transport for the async client. + +The synchronous side does not implement retries. It *configures* them: ``Mlb`` +mounts an ``HTTPAdapter`` carrying the library ``Retry`` policy onto the +Session it creates, and from that point on every ``session.get()`` retries +without any caller — ``MlbDataAdapter`` included — knowing retries exist. + +HTTPX has the same seam. ``AsyncClient(transport=...)`` accepts any +``AsyncBaseTransport``, which is the position ``HTTPAdapter`` occupies in +Requests. Putting the retry loop there instead of inside +``AsyncMlbDataAdapter`` gives the async side the sync structure: + +* Adapters call ``client.get()`` and are unaware of retries. +* The retry policy travels with the client, so two adapters sharing one client + share one policy by construction. Neither adapter holds retry state, so + neither can disagree with the other about it. +* A caller-injected client keeps whatever transport its caller mounted, so + "the library does not touch an injected client" needs no flag to enforce. + +A caller who wants library retry behavior on a client they own mounts this +transport themselves, mirroring the documented sync recipe for +``create_retry_policy()``. +""" + +import asyncio + +from ._async_support import import_httpx +from .mlb_dataadapter import _build_user_agent, create_retry_policy + +httpx = import_httpx() + + +class MlbAsyncRetryTransport(httpx.AsyncBaseTransport): + """Wrap an HTTPX transport with the library's bounded retry policy. + + Failures spend the same retry budget the sync policy spends: + + ReadTimeout -> read budget + ConnectTimeout -> connect budget + ConnectError -> connect budget + other TimeoutException -> total budget + other RequestError -> total budget + retryable HTTP status -> status budget + + Exhausting a budget re-raises the underlying HTTPX exception. Translating + those into the library's public exception types stays with the adapter, so + this class satisfies the transport contract HTTPX documents: transports + raise HTTPX errors. + """ + + def __init__( + self, + inner: httpx.AsyncBaseTransport | None = None, + *, + retry_policy=None, + ): + self._inner = inner if inner is not None else httpx.AsyncHTTPTransport() + self._retry_policy = ( + retry_policy if retry_policy is not None else create_retry_policy() + ) + + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + policy = self._retry_policy + + attempt = 0 + while True: + attempt += 1 + try: + response = await self._inner.handle_async_request(request) + + except httpx.ReadTimeout: + if attempt > policy.read: + raise + await self._backoff(attempt=attempt, response=None) + continue + + except httpx.ConnectTimeout: + # Caught before httpx.TimeoutException: a connect timeout is a + # timeout for the caller, but it spends the connect budget so + # the retry accounting matches the sync policy. + if attempt > policy.connect: + raise + await self._backoff(attempt=attempt, response=None) + continue + + except httpx.ConnectError: + if attempt > policy.connect: + raise + await self._backoff(attempt=attempt, response=None) + continue + + except httpx.TimeoutException: + if attempt > policy.total: + raise + await self._backoff(attempt=attempt, response=None) + continue + + except httpx.RequestError: + if attempt > policy.total: + raise + await self._backoff(attempt=attempt, response=None) + continue + + if ( + response.status_code not in policy.status_forcelist + or attempt > policy.status + ): + return response + + # The response is discarded, so release it before another attempt + # rather than leaving a connection checked out of the pool. + delay = self._delay_for(attempt=attempt, response=response) + await response.aclose() + if delay > 0: + await asyncio.sleep(delay) + + async def _backoff( + self, + *, + attempt: int, + response: httpx.Response | None, + ) -> None: + delay = self._delay_for(attempt=attempt, response=response) + if delay > 0: + await asyncio.sleep(delay) + + def _delay_for( + self, + *, + attempt: int, + response: httpx.Response | None, + ) -> float: + policy = self._retry_policy + + if policy.respect_retry_after_header and response is not None: + retry_after = policy.get_retry_after(response) + if retry_after: + return retry_after + + # Mirrors urllib3's Retry.get_backoff_time(): no delay before the + # first retry, exponential thereafter, capped at backoff_max. + if attempt <= 1: + return 0.0 + + return min( + policy.backoff_factor * (2 ** (attempt - 1)), + policy.backoff_max, + ) + + async def aclose(self) -> None: + await self._inner.aclose() + + +def create_library_async_client() -> httpx.AsyncClient: + """Build the async client the library creates and owns. + + The counterpart of ``_configure_library_session()`` on the sync side: + library defaults are applied here, at creation, and only to clients the + library creates. Passing headers to the constructor replaces just the + User-Agent, so HTTPX's other default headers survive. + """ + return httpx.AsyncClient( + headers={"User-Agent": _build_user_agent()}, + transport=MlbAsyncRetryTransport(), + ) diff --git a/mlbstatsapi/async_mlb.py b/mlbstatsapi/async_mlb.py index 5ff4973..94a479e 100644 --- a/mlbstatsapi/async_mlb.py +++ b/mlbstatsapi/async_mlb.py @@ -5,6 +5,7 @@ import logging from typing import TYPE_CHECKING +from ._async_transport import create_library_async_client from ._helpers.id_lookup import find_ids_by_key from ._helpers.schedule import build_schedule_params from ._parsers.attendance import parse_attendance @@ -63,17 +64,23 @@ def __init__( ): self._logger = logger or logging.getLogger(__name__) - # One client is shared by the v1 and v1.1 adapters, mirroring Mlb's - # shared-Session pattern. The v1 adapter resolves and owns the client - # (library-created when the caller passes none); the v1.1 adapter - # borrows that same client and never closes it itself, but still - # retries exactly when the shared client is library-owned. + # One client is shared by the v1 and v1.1 adapters, and this client + # owns it, mirroring Mlb's shared-Session pattern. The library closes + # only clients it creates; caller-injected clients remain caller-owned. + # The versioned User-Agent and the retry transport are applied only to + # library-created clients. + self._owns_client = client is None + if client is None: + self._client = create_library_async_client() + else: + self._client = client + self._closed = False self._mlb_adapter_v1 = AsyncMlbDataAdapter( hostname=hostname, ver="v1", logger=self._logger, timeout=timeout, - client=client, + client=self._client, strict_http=strict_http, ) self._mlb_adapter_v1_1 = AsyncMlbDataAdapter( @@ -81,19 +88,18 @@ def __init__( ver="v1.1", logger=self._logger, timeout=timeout, - client=self._mlb_adapter_v1._client, + client=self._client, strict_http=strict_http, ) - # AsyncMlb, not either adapter, actually owns this shared transport, - # so it is the one that knows whether the client is library-owned. - # The v1.1 adapter received a non-None client above, so it would - # otherwise conclude it's using a caller-injected client and disable - # retries even when the client is really library-owned via v1. - self._mlb_adapter_v1_1._set_retries_enabled(self._mlb_adapter_v1._owns_client) async def aclose(self) -> None: - """Close library-owned async resources.""" - await self._mlb_adapter_v1.aclose() + """Close the HTTP client when this client owns it. + + Safe to call more than once. Caller-injected clients are left alone. + """ + if self._owns_client and not self._closed: + await self._client.aclose() + self._closed = True async def __aenter__(self) -> "AsyncMlb": return self diff --git a/mlbstatsapi/async_mlb_dataadapter.py b/mlbstatsapi/async_mlb_dataadapter.py index 8e18065..ec67631 100644 --- a/mlbstatsapi/async_mlb_dataadapter.py +++ b/mlbstatsapi/async_mlb_dataadapter.py @@ -1,9 +1,9 @@ -import asyncio import logging from typing import Dict from ._async_support import import_httpx +from ._async_transport import create_library_async_client from .exceptions import ( MlbDecodeError, MlbTimeoutError, @@ -13,8 +13,6 @@ DEFAULT_TIMEOUT, MlbResult, TimeoutType, - _build_user_agent, - create_retry_policy, ) from ._http import ( @@ -49,25 +47,16 @@ def __init__( self._timeout = timeout self._strict_http = strict_http self._owns_client = client is None - # Retry eligibility follows client ownership by default, like the - # sync adapter (retries are mounted on the Session, not per - # MlbDataAdapter version). This is not a constructor knob: a caller - # that owns this adapter's transport (AsyncMlb, for its v1.1 adapter - # sharing v1's client) may call _set_retries_enabled() after - # construction, since it — not this adapter — is the one that knows - # whether the shared client is actually library-owned. - self._retries_enabled = self._owns_client - self._retry_policy = create_retry_policy() if client is None: - # Only a library-owned client gets the package User-Agent. Passing - # it to the constructor replaces just that header, so httpx's other - # default headers (Accept, Accept-Encoding, Connection) survive. - self._client = httpx.AsyncClient( - headers={"User-Agent": _build_user_agent()}, - ) + # A library-created client carries the package User-Agent and the + # library retry transport. Retries are a property of the client, + # not of this adapter, exactly as they are a property of the + # Session on the sync side. + self._client = create_library_async_client() else: - # An injected client stays exactly as the caller configured it. + # An injected client stays exactly as the caller configured it, + # retry transport included or not. self._client = client self._closed = False @@ -100,8 +89,20 @@ async def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> ) ) - self._logger.debug(logline_post) - response = await self._request_with_retries(full_url, ep_params) + try: + self._logger.debug(logline_post) + response = await self._client.get( + url=full_url, + params=ep_params, + timeout=self._translate_timeout(self._timeout), + ) + + except httpx.TimeoutException as exc: + self._logger.error(msg=(str(exc))) + raise MlbTimeoutError("Request failed") from exc + except httpx.RequestError as exc: + self._logger.error(msg=(str(exc))) + raise MlbTransportError("Request failed") from exc status_code = response.status_code @@ -181,129 +182,6 @@ async def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> data=response_data, ) - async def _request_with_retries( - self, - full_url: str, - ep_params: Dict, - ) -> httpx.Response: - """Issue the GET call, retrying with bounded backoff when this - adapter owns its httpx.AsyncClient. - - An injected client is called exactly once; its retry behavior stays - under caller control, matching the sync adapter's session-ownership - rule. - - Failures spend the retry budget the sync policy would spend, and - surface the public exception the sync adapter raises: - - 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 - """ - policy = self._retry_policy - - attempt = 0 - while True: - attempt += 1 - try: - response = await self._client.get( - url=full_url, - params=ep_params, - timeout=self._translate_timeout(self._timeout), - ) - - except httpx.ReadTimeout as exc: - max_attempts = policy.read + 1 if self._retries_enabled else 1 - - if attempt >= max_attempts: - self._logger.error(msg=str(exc)) - raise MlbTimeoutError("Request failed") from exc - - await self._sleep_before_retry(attempt=attempt, response=None) - continue - - except httpx.ConnectTimeout as exc: - # Caught before httpx.TimeoutException: a connect timeout is a - # timeout for the caller, but it spends the connect budget so - # the retry accounting matches the sync policy. - max_attempts = policy.connect + 1 if self._retries_enabled else 1 - - if attempt >= max_attempts: - self._logger.error(msg=str(exc)) - raise MlbTimeoutError("Request failed") from exc - - await self._sleep_before_retry(attempt=attempt, response=None) - continue - - except httpx.ConnectError as exc: - max_attempts = policy.connect + 1 if self._retries_enabled else 1 - - if attempt >= max_attempts: - self._logger.error(msg=str(exc)) - raise MlbTransportError("Request failed") from exc - - await self._sleep_before_retry( - attempt=attempt, - response=None, - ) - continue - - except httpx.TimeoutException as exc: - max_attempts = policy.total + 1 if self._retries_enabled else 1 - - if attempt >= max_attempts: - raise MlbTimeoutError("Request failed") from exc - - await self._sleep_before_retry( - attempt=attempt, - response=None, - ) - continue - - except httpx.RequestError as exc: - max_attempts = policy.total + 1 if self._retries_enabled else 1 - - if attempt >= max_attempts: - self._logger.error(msg=str(exc)) - raise MlbTransportError("Request failed") from exc - - await self._sleep_before_retry(attempt=attempt, response=None) - continue - - max_attempts = policy.status + 1 if self._retries_enabled else 1 - - if response.status_code not in policy.status_forcelist or attempt >= max_attempts: - return response - - await self._sleep_before_retry(attempt=attempt, response=response) - - async def _sleep_before_retry( - self, - *, - attempt: int, - response: httpx.Response | None, - ) -> None: - policy = self._retry_policy - - if policy.respect_retry_after_header and response is not None: - retry_after = policy.get_retry_after(response) - if retry_after: - await asyncio.sleep(retry_after) - return - - # Mirrors urllib3's Retry.get_backoff_time(): no delay before the - # first retry, exponential thereafter, capped at backoff_max. - delay = 0.0 if attempt <= 1 else min( - policy.backoff_factor * (2 ** (attempt - 1)), - policy.backoff_max, - ) - - if delay > 0: - await asyncio.sleep(delay) - @staticmethod def _translate_timeout(timeout: TimeoutType) -> httpx.Timeout: if isinstance(timeout, tuple): @@ -322,14 +200,3 @@ async def aclose(self) -> None: if self._owns_client and not self._closed: await self._client.aclose() self._closed = True - - def _set_retries_enabled(self, enabled: bool) -> None: - """Override retry eligibility for a borrowed, non-owned client. - - Internal coordination hook, not public API: only a caller that - actually owns this adapter's transport (AsyncMlb, wiring up its v1.1 - adapter to share the v1 adapter's client) should call this. Standalone - use never needs it; retry eligibility already follows client - ownership by default. - """ - self._retries_enabled = enabled diff --git a/tests/test_async_mlb.py b/tests/test_async_mlb.py index 08a98d2..afa87fe 100644 --- a/tests/test_async_mlb.py +++ b/tests/test_async_mlb.py @@ -36,6 +36,7 @@ httpx = pytest.importorskip("httpx", reason="requires the async extra (HTTPX)") from mlbstatsapi import Mlb # noqa: E402 +from mlbstatsapi._async_transport import MlbAsyncRetryTransport # noqa: E402 from mlbstatsapi.async_mlb import AsyncMlb # noqa: E402 from mlbstatsapi.mlb_dataadapter import MlbResult # noqa: E402 from mlbstatsapi.models.attendances import Attendance # noqa: E402 @@ -387,29 +388,23 @@ def request(self) -> httpx.Request: async def async_mlb(handler: _Handler): """Yield an AsyncMlb whose own client talks to ``handler``, then close it. - AsyncMlb builds its adapter and client through the production path; only - the transport is swapped. Teardown closes the adapter's client directly - rather than calling AsyncMlb.aclose(), so the lifecycle tests that replace - aclose with a mock still get their real client closed. + AsyncMlb builds its client, its retry transport and its adapters through + the production path; only the innermost network transport is swapped. + Teardown closes the client directly rather than calling AsyncMlb.aclose(), + so the lifecycle tests that replace aclose with a mock still get their real + client closed. """ - real_async_client = httpx.AsyncClient - - def mock_transport_client(**client_kwargs) -> httpx.AsyncClient: - return real_async_client( - transport=httpx.MockTransport(handler), **client_kwargs - ) - with pytest.MonkeyPatch.context() as monkeypatch: monkeypatch.setattr( - "mlbstatsapi.async_mlb_dataadapter.httpx.AsyncClient", - mock_transport_client, + "mlbstatsapi._async_transport.httpx.AsyncHTTPTransport", + lambda **kwargs: httpx.MockTransport(handler), ) mlb = AsyncMlb() try: yield mlb finally: - await mlb._mlb_adapter_v1._client.aclose() + await mlb._client.aclose() def sync_request_for(method: str, *args, **kwargs) -> tuple[str, dict, str]: @@ -495,7 +490,7 @@ async def scenario(): def test_context_exit_closes_the_owned_client(): async def scenario(): async with async_mlb(_Handler(_json(TEAM_PAYLOAD))) as mlb: - client = mlb._mlb_adapter_v1._client + client = mlb._client async with mlb: await mlb.get_team(133) @@ -508,7 +503,7 @@ async def scenario(): def test_context_exit_closes_the_owned_client_when_the_body_raises(): async def scenario(): async with async_mlb(_Handler(_json(TEAM_PAYLOAD))) as mlb: - client = mlb._mlb_adapter_v1._client + client = mlb._client with pytest.raises(ValueError, match="boom"): async with mlb: @@ -528,7 +523,7 @@ def test_cleanup_failure_does_not_replace_the_original_exception(): async def scenario(): async with async_mlb(_Handler(_json(TEAM_PAYLOAD))) as mlb: - mlb._mlb_adapter_v1.aclose = AsyncMock( + mlb.aclose = AsyncMock( side_effect=RuntimeError("cleanup failed") ) @@ -548,7 +543,7 @@ def test_cancellation_is_preserved_through_cleanup(): async def scenario(): async with async_mlb(_Handler(_json(TEAM_PAYLOAD))) as mlb: - client = mlb._mlb_adapter_v1._client + client = mlb._client async def worker(): async with mlb: @@ -583,42 +578,47 @@ async def scenario(): asyncio.run(scenario()) -def test_v1_and_v1_1_adapters_share_one_client(): - """One client is shared by both adapters, mirroring Mlb's shared Session.""" +def test_v1_and_v1_1_adapters_share_the_client_this_client_owns(): + """One client is shared by both adapters and owned by AsyncMlb itself, + mirroring Mlb's shared Session.""" async def scenario(): async with async_mlb(_Handler(_json(TEAM_PAYLOAD))) as mlb: - assert mlb._mlb_adapter_v1._client is mlb._mlb_adapter_v1_1._client - # Only v1 tracks close-ownership of the shared client; v1.1 must - # never double-close it. - assert mlb._mlb_adapter_v1._owns_client is True + assert mlb._mlb_adapter_v1._client is mlb._client + assert mlb._mlb_adapter_v1_1._client is mlb._client + # Close-ownership lives on AsyncMlb, so neither adapter can close + # the shared client out from under the other. + assert mlb._owns_client is True + assert mlb._mlb_adapter_v1._owns_client is False assert mlb._mlb_adapter_v1_1._owns_client is False asyncio.run(scenario()) -def test_v1_1_adapter_retries_when_the_shared_client_is_library_owned(): - """Retry eligibility follows the shared client's ownership, not which - adapter version issues the request (matching Mlb, which configures - retries once on the shared Session).""" +def test_both_api_versions_retry_because_the_shared_client_carries_the_policy(): + """Retries belong to the shared client's transport, not to an adapter, so + the two versions cannot disagree about them (matching Mlb, which mounts + one retry policy on the shared Session).""" async def scenario(): async with async_mlb(_Handler(_json(TEAM_PAYLOAD))) as mlb: - assert mlb._mlb_adapter_v1._retries_enabled is True - assert mlb._mlb_adapter_v1_1._retries_enabled is True + assert isinstance(mlb._client._transport, MlbAsyncRetryTransport) asyncio.run(scenario()) -def test_v1_1_adapter_does_not_retry_with_a_caller_injected_client(): +def test_caller_injected_client_keeps_its_own_transport(): + """The library mounts nothing on a client it did not create, so an + injected client retries exactly as much as its caller configured.""" handler = _Handler(_json(TEAM_PAYLOAD)) - client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + transport = httpx.MockTransport(handler) + client = httpx.AsyncClient(transport=transport) async def scenario(): try: async with AsyncMlb(client=client) as mlb: - assert mlb._mlb_adapter_v1._retries_enabled is False - assert mlb._mlb_adapter_v1_1._retries_enabled is False + assert mlb._client is client + assert mlb._client._transport is transport finally: await client.aclose() diff --git a/tests/test_async_mlb_dataadapter.py b/tests/test_async_mlb_dataadapter.py index f2ef0f5..9073732 100644 --- a/tests/test_async_mlb_dataadapter.py +++ b/tests/test_async_mlb_dataadapter.py @@ -41,6 +41,10 @@ MlbTimeoutError, MlbTransportError, ) +from mlbstatsapi._async_transport import ( # noqa: E402 + MlbAsyncRetryTransport, + create_library_async_client, +) from mlbstatsapi.async_mlb_dataadapter import AsyncMlbDataAdapter # noqa: E402 from mlbstatsapi.mlb_dataadapter import PACKAGE_DISTRIBUTION_NAME # noqa: E402 @@ -54,11 +58,13 @@ BASE_URL = "https://statsapi.mlb.com/api/v1/" -SLEEP_TARGET = "mlbstatsapi.async_mlb_dataadapter.asyncio.sleep" +SLEEP_TARGET = "mlbstatsapi._async_transport.asyncio.sleep" # Patched only while a test adapter is constructed, so the adapter creates its -# own library-owned client the way production does, over a MockTransport. -CLIENT_TARGET = "mlbstatsapi.async_mlb_dataadapter.httpx.AsyncClient" +# own library-owned client the way production does. Only the innermost network +# transport is swapped, so the library retry transport under test is the real +# one, wrapping a MockTransport instead of a socket. +INNER_TRANSPORT_TARGET = "mlbstatsapi._async_transport.httpx.AsyncHTTPTransport" # Matches tests/test_mlb_session.py, so both adapters assert the same contract. MOCKED_PACKAGE_VERSION = "9.8.7" @@ -117,26 +123,23 @@ def _owned_adapter(handler, **kwargs) -> AsyncMlbDataAdapter: """Build an adapter that owns its client, so retries are active. The adapter still builds its own client through the production path — only - the transport is swapped for a MockTransport — so ownership, headers, and - retry behavior are exactly what the library does at runtime, and no client - is constructed and then discarded. Call this from inside a run_async() - scenario; run_async() closes what it creates. + the innermost network transport is swapped for a MockTransport — so + ownership, headers, and retry behavior are exactly what the library does at + runtime, and no client is constructed and then discarded. Call this from + inside a run_async() scenario; run_async() closes what it creates. """ - real_async_client = httpx.AsyncClient - - def mock_transport_client(**client_kwargs) -> httpx.AsyncClient: - return real_async_client( - transport=httpx.MockTransport(handler), - **client_kwargs, - ) - - with patch(CLIENT_TARGET, mock_transport_client): + with patch(INNER_TRANSPORT_TARGET, lambda **kwargs: httpx.MockTransport(handler)): adapter = AsyncMlbDataAdapter(**kwargs) _ADAPTERS_TO_CLOSE.append(adapter) return adapter +def _retry_policy_of(adapter: AsyncMlbDataAdapter): + """Read the retry policy the adapter's client actually uses.""" + return adapter._client._transport._retry_policy + + def _injected_adapter(handler, **kwargs) -> AsyncMlbDataAdapter: """Build an adapter with a caller-supplied client, so retries are bypassed.""" client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) @@ -145,34 +148,43 @@ def _injected_adapter(handler, **kwargs) -> AsyncMlbDataAdapter: def test_retry_policy_matches_library_default(): adapter = AsyncMlbDataAdapter() - assert_library_retry_policy(adapter._retry_policy) + assert_library_retry_policy(_retry_policy_of(adapter)) -def test_set_retries_enabled_does_not_change_client_ownership(): - """The private coordination hook AsyncMlb uses for its v1.1 adapter only - overrides retry eligibility; it must never grant close-ownership over a - client this adapter did not create.""" - handler = _ScriptedHandler(_response(200)) - adapter = _injected_adapter(handler) - assert adapter._retries_enabled is False +def test_library_created_client_mounts_the_retry_transport(): + """Retries are configured onto the client at creation, the way the sync + side mounts them onto a library-created Session.""" + client = create_library_async_client() + + assert isinstance(client._transport, MlbAsyncRetryTransport) - adapter._set_retries_enabled(True) - assert adapter._retries_enabled is True +def test_injected_client_transport_is_left_alone(): + """The library mounts nothing on a client it did not create, so an + injected client keeps exactly the retry behavior its caller gave it.""" + transport = httpx.MockTransport(_ScriptedHandler(_response(200))) + client = httpx.AsyncClient(transport=transport) + adapter = AsyncMlbDataAdapter(client=client) + + assert adapter._client._transport is transport assert adapter._owns_client is False -def test_set_retries_enabled_true_makes_an_injected_client_retry(): - """An injected client normally gets zero retries; overriding the flag - must actually change retry behavior, not just the stored value.""" +def test_mounting_the_retry_transport_makes_an_injected_client_retry(): + """The supported way for a caller to opt their own client into library + retry behavior, mirroring the sync create_retry_policy() recipe.""" handler = _ScriptedHandler(_response(503), _response(200)) async def scenario(): - adapter = _injected_adapter(handler) - adapter._set_retries_enabled(True) - - with patch(SLEEP_TARGET, new_callable=AsyncMock): - return await adapter.get(endpoint="sports") + client = httpx.AsyncClient( + transport=MlbAsyncRetryTransport(httpx.MockTransport(handler)), + ) + adapter = AsyncMlbDataAdapter(client=client) + try: + with patch(SLEEP_TARGET, new_callable=AsyncMock): + return await adapter.get(endpoint="sports") + finally: + await client.aclose() result = run_async(scenario()) assert result.status_code == 200 @@ -765,7 +777,7 @@ def test_connect_timeout_spends_the_connect_retry_budget(): async def scenario(): adapter = _owned_adapter(handler) - adapter._retry_policy.connect = 1 + _retry_policy_of(adapter).connect = 1 with patch(SLEEP_TARGET, new_callable=AsyncMock): with pytest.raises(MlbTimeoutError): await adapter.get(endpoint="sports") @@ -825,7 +837,7 @@ def test_generic_failures_spend_the_total_retry_budget(failure, expected_excepti async def scenario(): adapter = _owned_adapter(handler) - adapter._retry_policy.total = 1 + _retry_policy_of(adapter).total = 1 with patch(SLEEP_TARGET, new_callable=AsyncMock): with pytest.raises(expected_exception) as exc_info: await adapter.get(endpoint="sports") @@ -849,7 +861,7 @@ def test_connect_error_spends_the_connect_retry_budget(): async def scenario(): adapter = _owned_adapter(handler) - adapter._retry_policy.connect = 1 + _retry_policy_of(adapter).connect = 1 with patch(SLEEP_TARGET, new_callable=AsyncMock): with pytest.raises(MlbTransportError): await adapter.get(endpoint="sports") @@ -864,7 +876,7 @@ def test_retryable_status_spends_the_status_retry_budget(): async def scenario(): adapter = _owned_adapter(handler) - adapter._retry_policy.status = 1 + _retry_policy_of(adapter).status = 1 with patch(SLEEP_TARGET, new_callable=AsyncMock): with pytest.raises(MlbHttpError) as exc_info: await adapter.get(endpoint="sports") @@ -921,9 +933,9 @@ async def scenario(): def test_retry_sleep_is_async_and_non_blocking(): """A real (unmocked) backoff wait must yield the event loop. - If _sleep_before_retry ever used a blocking call (e.g. time.sleep) - instead of `await asyncio.sleep(...)`, the whole event loop would - freeze for the wait's duration and the concurrently running marker + If the retry transport's backoff ever used a blocking call (e.g. + time.sleep) instead of `await asyncio.sleep(...)`, the whole event loop + would freeze for the wait's duration and the concurrently running marker task below would make zero progress during it. """ handler = _ScriptedHandler(_response(500), _response(500), _response(200)) @@ -931,7 +943,7 @@ def test_retry_sleep_is_async_and_non_blocking(): async def scenario(): adapter = _owned_adapter(handler) # Small but real backoff so the test stays fast without mocking sleep. - adapter._retry_policy.backoff_factor = 0.05 + _retry_policy_of(adapter).backoff_factor = 0.05 marker_ticks = 0