diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 0cd4bcd..4842bd3 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -4,11 +4,11 @@ on: pull_request: branches: - main - - release/0.8.0 + - release/0.9.0 push: branches: - main - - release/0.8.0 + - release/0.9.0 workflow_dispatch: permissions: @@ -49,7 +49,7 @@ jobs: --ignore=tests/external_tests build-package: - name: Build package + name: Build and validate package needs: offline-tests runs-on: ubuntu-latest @@ -67,4 +67,11 @@ jobs: - name: Install dependencies run: poetry install --no-interaction - name: Build package - run: poetry build + run: | + rm -rf dist + poetry build + # Validates the built artifacts and a clean wheel installation. + # Runs outside the Poetry environment so the smoke test cannot import + # the repository checkout instead of the installed distribution. + - name: Validate release artifacts + run: python scripts/validate_release.py diff --git a/README.md b/README.md index 0d4998a..1e10bbc 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ For detailed documentation, check out the [Wiki](https://github.com/zero-sum-sea
## Installation -```python +```bash python3 -m pip install python-mlb-statsapi ``` @@ -65,9 +65,13 @@ Ty France Seattle Mariners Seattle ``` -## HTTP Sessions, Timeouts, and Retries +## HTTP Sessions, Timeouts, Retries, and Error Behavior + +Version 0.8.0 added shared HTTP Sessions, explicit timeouts, optional Session injection, bounded retries, and structured transport exceptions. Version 0.9.0 builds on that transport with configurable HTTP behavior: a public retry policy, richer `MlbHttpError` context, an optional strict mode, compatibility warnings, and a versioned User-Agent. + +The `Mlb` client remains synchronous. Shared Sessions pool reusable connections; they do not cache MLB response bodies, and the client does not enable response caching by default. -Version 0.8.0 adds shared HTTP Sessions, explicit timeouts, optional Session injection, bounded retries, and structured transport exceptions. The `Mlb` client remains synchronous. Shared Sessions pool reusable connections; they do not cache MLB response bodies, and the client does not enable response caching by default. +For the complete reference see the [HTTP transport documentation](docs/http-transport.md). For what changed in this release see the [0.9.0 release notes](docs/releases/0.9.0.md). ### Recommended context-manager usage @@ -83,18 +87,67 @@ with mlbstatsapi.Mlb() as mlb: One `Mlb` client uses one shared `requests.Session`. The v1 and v1.1 adapters share that Session, so repeated requests can reuse pooled connections. A Session manages a pool of reusable connections; it is not one permanent network connection. -### Existing construction remains valid +Callers who do not use a context manager may call `mlb.close()` instead. Repeated `close()` calls are safe. Closing a client only closes a Session the library created; a caller-injected Session is left open for its owner. + +### Compatibility mode is the default + +Existing construction continues to work unchanged: + +```python +import mlbstatsapi + +with mlbstatsapi.Mlb() as mlb: + player = mlb.get_person(664034) +``` + +That is equivalent to: + +```python +mlb = mlbstatsapi.Mlb( + strict_http=False, +) +``` + +Compatibility mode remains the default in version 0.9.0. A final non-404 4xx response still returns the historical empty result instead of raising, so existing applications keep working after upgrading. + +### Optional strict HTTP mode + +Applications that would rather fail loudly can opt in to strict mode: + +```python +import mlbstatsapi + +with mlbstatsapi.Mlb( + strict_http=True, +) as mlb: + player = mlb.get_person(664034) +``` + +In strict mode: + +* A final non-404 4xx response raises `MlbHttpError` +* A final 5xx response raises `MlbHttpError`, as it already did in compatibility mode +* A 404 keeps the existing endpoint-specific behavior and does not raise -Existing construction continues to work: +"Final" means after the bounded retry policy has been exhausted. Strict mode is opt-in; it is not the default. + +### Compatibility warnings + +When compatibility mode converts a final non-404 4xx response into the historical empty result, the library emits `MlbHttpCompatibilityWarning`. The warning marks a response that strict mode would have raised on, so it doubles as migration guidance. + +The category inherits from `FutureWarning`, so it stays visible under default Python warning filters. Applications can promote only this package category to an error: ```python +import warnings import mlbstatsapi -mlb = mlbstatsapi.Mlb() -player = mlb.get_person(664034) +warnings.filterwarnings( + "error", + category=mlbstatsapi.MlbHttpCompatibilityWarning, +) ``` -Callers who do not use a context manager may call `mlb.close()`. Repeated `close()` calls are safe. +Filter on `mlbstatsapi.MlbHttpCompatibilityWarning` specifically rather than disabling all warnings or all `FutureWarning` instances, which would also hide unrelated notices from other libraries. No warning is emitted for successful responses, 404 responses, intermediate retries, final 5xx responses, or in strict mode. ### Custom timeouts @@ -156,12 +209,50 @@ Ownership rules: ```text Library-created Session - The library owns and closes it + The library configures and closes it Caller-injected Session - The caller owns and closes it + The caller configures and closes it +``` + +`Mlb.close()` does not close a caller-injected Session, and exiting `with Mlb(session=session)` does not close the injected Session either. The library does not replace or reconfigure adapters or headers on an injected Session. Callers control custom retry, TLS, proxy, header, and adapter configuration. + +### Reusing the retry policy on a caller-managed Session + +`create_retry_policy()` is public in version 0.9.0. It returns a new instance of the same tested policy the library mounts on Sessions it creates, so a caller-managed Session can opt in to identical retry behavior: + +```python +import requests +import mlbstatsapi + +session = requests.Session() +adapter = requests.adapters.HTTPAdapter( + max_retries=mlbstatsapi.create_retry_policy(), +) +session.mount("https://", adapter) +session.mount("http://", adapter) + +try: + with mlbstatsapi.Mlb(session=session) as mlb: + player = mlb.get_person(664034) +finally: + session.close() ``` -`Mlb.close()` does not close a caller-injected Session, and exiting `with Mlb(session=session)` does not close the injected Session either. The library does not replace or reconfigure adapters on an injected Session. Callers control custom retry, TLS, proxy, and adapter configuration. +* The caller mounts the adapters +* The caller closes the injected Session +* The library never reconfigures an injected Session + +### Versioned User-Agent + +A Session created by the library sends a package-specific User-Agent: + +```text +python-mlb-statsapi/ +``` + +For this release that resolves to `python-mlb-statsapi/0.9.0`. The version is read from the installed distribution metadata, so it always matches the installed release. Only the `User-Agent` header is set; other Requests defaults such as `Accept-Encoding` remain intact, and the header carries no identifiers beyond the package name and version. + +Headers on a caller-injected Session are left untouched, so applications that set their own User-Agent keep it. ### Structured exception handling @@ -176,11 +267,12 @@ except mlbstatsapi.MlbTimeoutError: except mlbstatsapi.MlbTransportError: print("The request could not reach the MLB API") except mlbstatsapi.MlbHttpError as exc: - print( - exc.status_code, - exc.reason, - exc.url, - ) + print(exc.method) + print(exc.status_code) + print(exc.reason) + print(exc.url) + print(exc.response_data) + print(exc.body_excerpt) except mlbstatsapi.MlbDecodeError: print("The MLB API returned invalid JSON") ``` @@ -190,6 +282,8 @@ except mlbstatsapi.MlbDecodeError: * `MlbHttpError` represents an unexpected final HTTP response * `MlbDecodeError` represents invalid JSON in a successful response +Version 0.9.0 adds `method`, `response_data`, and `body_excerpt` to `MlbHttpError` alongside the existing `status_code`, `reason`, and `url`. `response_data` holds the decoded JSON dictionary or list when the error body contains one, and is `None` otherwise. `body_excerpt` is a bounded excerpt of the response text, capped at 500 characters. Complete response bodies are never automatically logged, and `str(exc)` stays concise. + ### Backward-compatible exception handling All new transport exceptions inherit from `TheMlbStatsApiException`, so existing broad exception handling remains compatible: @@ -224,11 +318,11 @@ Backoff factor: 0.5 Retry-After respected: yes ``` -Only GET requests are retried, and retries are bounded. Ordinary client errors such as 400, 401, 403, and 404 are not retried. Invalid JSON and Pydantic validation failures are not retried. Retries improve resilience for transient failures, but they do not guarantee success. +Only GET requests are retried, and retries are bounded. Ordinary client errors such as 400, 401, 403, and 404 are not retried. Invalid JSON and Pydantic validation failures are not retried. Retries improve resilience for transient failures, but they do not guarantee success. The retry values are unchanged from version 0.8.0. ### Existing 404 compatibility -Version 0.8.0 preserves existing endpoint-specific not-found behavior. Depending on the endpoint, a 404 may still produce: +Version 0.9.0 preserves existing endpoint-specific not-found behavior in both compatibility mode and strict mode. Depending on the endpoint, a 404 may still produce: ```text None @@ -236,9 +330,19 @@ None {} ``` -Not every 404 raises `MlbHttpError`. +Not every 404 raises `MlbHttpError`, and strict mode does not change that. -See the [HTTP transport documentation](docs/http-transport.md) for the complete retry policy, Session ownership rules, cleanup behavior, and exception hierarchy. +### HTTP behavior at a glance + +| Final response | Compatibility mode (default) | Strict mode | +| -------------- | ---------------------------- | ----------- | +| Successful 2xx | Normal result | Normal result | +| Non-404 4xx | Warning and historical empty result | `MlbHttpError` | +| 404 | Existing endpoint behavior | Existing endpoint behavior | +| Final 429 | Warning and historical empty result | `MlbHttpError` | +| Final 5xx | `MlbHttpError` | `MlbHttpError` | + +See the [HTTP transport documentation](docs/http-transport.md) for the complete retry policy, Session ownership rules, warning behavior, cleanup behavior, and exception hierarchy, and the [0.9.0 release notes](docs/releases/0.9.0.md) for the release summary and migration guidance. ## Working with Pydantic Models @@ -376,9 +480,13 @@ Full local validation: ```bash poetry run pytest tests/ +rm -rf dist poetry build +python3 scripts/validate_release.py ``` +`scripts/validate_release.py` is the same release check offline CI runs. It inspects the built wheel and source distribution, installs the wheel into a temporary virtual environment, and runs a public-import smoke test against the installed package. It never contacts the MLB API. + Offline CI is the normal pull-request gate. External tests are available manually, on a weekly schedule, and before releases. ### Pull Request Guidelines diff --git a/docs/http-transport.md b/docs/http-transport.md index 9150f64..df04dc0 100644 --- a/docs/http-transport.md +++ b/docs/http-transport.md @@ -1,9 +1,34 @@ # HTTP Transport -This document describes the HTTP transport behavior introduced in version 0.8.0. +This document describes the HTTP transport behavior of the current release, version 0.9.0. + +Version 0.8.0 introduced shared Sessions, explicit timeouts, bounded retries, and structured exceptions. Version 0.9.0 keeps all of that and adds configurable HTTP behavior: a public retry policy, richer `MlbHttpError` context, an optional strict mode, compatibility warnings, and a versioned User-Agent. The public client remains synchronous. Ordinary usage does not need to configure sessions or retries. +See [the 0.9.0 release notes](releases/0.9.0.md) for a shorter summary of what changed. + +## Public transport API + +Everything this document describes is reachable from the package root: + +```python +from mlbstatsapi import ( + Mlb, + MlbDataAdapter, + MlbDecodeError, + MlbHttpCompatibilityWarning, + MlbHttpError, + MlbTimeoutError, + MlbTransportError, + TheMlbStatsApiException, + create_retry_policy, +) +``` + +Names that are not exported from `mlbstatsapi` are internal and may change without a +deprecation cycle. + ## Existing usage Existing construction continues to work: @@ -15,7 +40,7 @@ mlb = mlbstatsapi.Mlb() player = mlb.get_person(664034) ``` -The client remains synchronous. Async support is not part of version 0.8.0. +The client remains synchronous. Async support is not part of version 0.9.0. ## Context manager @@ -53,8 +78,8 @@ Every request uses an explicit timeout. The default is: -```python -DEFAULT_TIMEOUT = (3.05, 30.0) +```text +(3.05, 30.0) ``` That means: @@ -105,6 +130,27 @@ A Session is not: * One guaranteed permanent TCP connection * An async transport +## Session ownership + +Session ownership is the single most important rule in this document. Whoever creates the +Session configures it and closes it. + +```text +Library-created Session + Configured and closed by the library + Receives retry adapters + Receives the package User-Agent + +Caller-injected Session + Configured and closed by the caller + Existing adapters remain untouched + Existing headers remain untouched +``` + +The library never installs adapters, replaces headers, or closes a Session it did not +create. `Mlb.close()` and exiting `with Mlb(session=session)` both leave an injected +Session open. + ## Session injection Advanced callers may inject a Session: @@ -122,23 +168,71 @@ finally: session.close() ``` -Ownership rules: +Callers who inject a Session control its retry, TLS, proxy, header, and adapter +configuration. See [Reusing the retry policy on a caller-managed +Session](#reusing-the-retry-policy-on-a-caller-managed-session) for opting in to the +library's tested retry policy. + +## User-Agent + +Library-created Sessions send a package-specific User-Agent: ```text -Library-created Session - The library owns and closes it +python-mlb-statsapi/ +``` -Caller-injected Session - The caller owns and closes it +For this release that resolves to: + +```text +python-mlb-statsapi/0.9.0 ``` -The library does not install or replace retry adapters on caller-injected Sessions. +The version comes from the installed package metadata, so it always matches the +installed release without a separately maintained version string. -Callers who inject a Session control its retry, TLS, proxy, and adapter configuration. +Notes: + +* The header helps identify package traffic while debugging +* Other Requests default headers such as `Accept-Encoding`, `Accept`, and `Connection` remain intact +* Only `User-Agent` is set; the full header mapping is never replaced +* Caller-injected Sessions are never modified +* Applications using an injected Session may set their own User-Agent +* The header contains no machine identifiers, installation identifiers, hostnames, or user tracking data +* This is not telemetry and sends no analytics + +If the distribution metadata is unavailable, for example in an unusual +source-only environment, the header falls back to: + +```text +python-mlb-statsapi/unknown +``` + +Callers who inject a Session control the User-Agent themselves: + +```python +import requests +import mlbstatsapi + +session = requests.Session() +session.headers.update( + { + "User-Agent": "my-baseball-project/1.0", + } +) + +try: + with mlbstatsapi.Mlb(session=session) as mlb: + player = mlb.get_person(664034) +finally: + session.close() +``` ## Default retry policy -Library-created Sessions mount a bounded retry policy for GET requests. +Library-created Sessions mount a bounded retry policy for GET requests automatically. + +Caller-injected Sessions are never automatically reconfigured. Retry settings on an +injected Session remain under the caller's control unless the caller opts in. ```text Initial request: 1 @@ -176,11 +270,220 @@ Additional rules: * Pydantic validation failures are not retried * Application parsing failures are not retried * A final 404 preserves existing not-found behavior -* A final 429 preserves existing 4xx compatibility +* A final 429 preserves existing 4xx compatibility by default and warns * A final 5xx raises `MlbHttpError` +* In strict mode, a final non-404 4xx (including a final 429) raises `MlbHttpError` Retries improve resilience for transient failures. They do not guarantee success. +## HTTP compatibility modes + +Compatibility mode remains the default: + +```python +mlb = mlbstatsapi.Mlb() +``` + +or: + +```python +mlb = mlbstatsapi.Mlb( + strict_http=False, +) +``` + +Callers may explicitly enable strict mode: + +```python +mlb = mlbstatsapi.Mlb( + strict_http=True, +) +``` + +Behavior: + +| Final response | Compatibility mode | Strict mode | +| -------------- | ----------------------------------- | -------------------------- | +| Successful 2xx | Normal result | Normal result | +| Non-404 4xx | Warning and historical empty result | `MlbHttpError` | +| 404 | Existing endpoint behavior | Existing endpoint behavior | +| Final 429 | Warning and historical empty result | `MlbHttpError` | +| Final 5xx | `MlbHttpError` | `MlbHttpError` | + +Every row describes the *final* response. A retryable status such as 429, 500, 502, 503, +or 504 is evaluated against this table only after the bounded retry policy has been +exhausted; intermediate retried responses neither raise nor warn. + +Notes: + +* Compatibility mode remains the default +* Strict mode is explicitly opt-in +* Strict mode applies only after retries are exhausted +* Strict mode does not make 404 raise +* Strict mode does not change transport or decode exceptions +* Strict-mode exceptions include the richer context from `MlbHttpError` +* Existing constructor usage remains valid +* Compatibility mode emits `MlbHttpCompatibilityWarning` for a suppressed non-404 4xx + +Example: + +```python +import mlbstatsapi + +try: + with mlbstatsapi.Mlb(strict_http=True) as mlb: + player = mlb.get_person(664034) +except mlbstatsapi.MlbHttpError as exc: + print(exc.method) + print(exc.status_code) + print(exc.reason) + print(exc.url) + print(exc.response_data) + print(exc.body_excerpt) +``` + +## Compatibility warnings + +Version 0.9.0 emits `MlbHttpCompatibilityWarning` when compatibility mode returns the +historical empty result for a final non-404 4xx response. + +The warning means strict mode would have raised `MlbHttpError` for the same response. + +```python +import mlbstatsapi + +mlb = mlbstatsapi.Mlb() +sports = mlb.get_sports() +``` + +A representative warning looks like: + +```text +HTTP 403 for https://statsapi.mlb.com/api/v1/sports was handled through +compatibility mode and returned the historical empty result. Pass +strict_http=True to raise MlbHttpError. This compatibility behavior may +change in version 1.0. +``` + +The category inherits from `FutureWarning` so the migration notice stays visible under +default Python warning filters. + +A warning is emitted only when all three of the following are true: + +* Compatibility mode is active +* The final response is a non-404 4xx +* Strict mode would have raised `MlbHttpError` for the same response + +No warning is emitted for: + +```text +Successful responses +404 +Strict mode +Intermediate retries +Final 5xx +Timeouts +Transport failures +Decode failures +Pydantic validation failures +``` + +When the warning is emitted: + +| Response | Warning | +| ------------------------- | ------- | +| Non-404 4xx, compatibility mode | Yes | +| Final 429, compatibility mode, after retries | Yes | +| Non-404 4xx, strict mode | No, `MlbHttpError` is raised instead | +| 404, either mode | No | +| Successful 2xx | No | +| Final 5xx | No, `MlbHttpError` is raised in both modes | +| Timeout, transport, decode, or validation failure | No | + +Additional rules: + +* The warning never changes the return value; compatibility mode still returns the + historical empty result in version 0.9.0 +* A final 404 remains warning-free and keeps existing `None` / `[]` / `{}` behavior +* Warnings are emitted only after retries are exhausted, so a retried 429 warns once +* Strict mode does not warn because it raises `MlbHttpError` directly +* Warning messages contain only the status code and request URL, never response + bodies, headers, or credentials +* Stricter defaults may be introduced in version 1.0 + +Enabling strict mode is the recommended migration: + +```python +import mlbstatsapi + +mlb = mlbstatsapi.Mlb( + strict_http=True, +) +``` + +Applications may also turn only this package warning into an exception: + +```python +import warnings +import mlbstatsapi + +warnings.filterwarnings( + "error", + category=mlbstatsapi.MlbHttpCompatibilityWarning, +) +``` + +Or silence only this category: + +```python +import warnings +import mlbstatsapi + +warnings.filterwarnings( + "ignore", + category=mlbstatsapi.MlbHttpCompatibilityWarning, +) +``` + +Prefer enabling strict mode over permanently ignoring the warning when the application +wants explicit HTTP failures. Filter by `mlbstatsapi.MlbHttpCompatibilityWarning` rather +than disabling all `FutureWarning` or all warnings, which would also hide unrelated +notices from other libraries. + +## Reusing the retry policy on a caller-managed Session + +`create_retry_policy()` returns a new instance of the same tested retry policy used +internally for library-created Sessions. + +Callers who inject a Session must mount the policy themselves. The library does not +install or replace adapters on caller-injected Sessions. + +Callers retain control over connection-pool sizes and other `HTTPAdapter` options. +The caller remains responsible for closing an injected Session. + +```python +import requests +import mlbstatsapi + +session = requests.Session() +adapter = requests.adapters.HTTPAdapter( + max_retries=mlbstatsapi.create_retry_policy(), + pool_connections=10, + pool_maxsize=20, +) +session.mount("https://", adapter) +session.mount("http://", adapter) + +try: + with mlbstatsapi.Mlb(session=session) as mlb: + player = mlb.get_person(664034) +finally: + session.close() +``` + +Mounting the same adapter instance for both schemes is valid. Callers may also mount +separate adapters when they need different settings for HTTP and HTTPS. + ## Structured exceptions ```text @@ -243,16 +546,53 @@ Notes: `MlbHttpError` exposes: ```text +method status_code reason url +response_data +body_excerpt ``` -It does not expose a response body or Response object. +Existing attributes remain compatible: + +```text +status_code +reason +url +``` + +Additional context: + +* `method` is the HTTP method when the adapter raises the error (`GET` today). Manually constructed exceptions without a method leave `method` as `None`. +* `response_data` contains a decoded JSON dictionary or list when the error body is valid JSON of that shape. +* `response_data` is `None` for invalid JSON, HTML, plain text, empty bodies, or JSON scalars such as strings, numbers, booleans, or null. +* `body_excerpt` contains at most 500 characters of the response text for non-empty bodies. +* `body_excerpt` is `None` for an empty body. +* Error-context extraction is best-effort. A parsing or decoding failure while collecting context must not replace the original HTTP error. +* Complete `requests.Response` objects are not exposed. +* Complete large response bodies are not preserved beyond the excerpt limit. +* Response bodies are not automatically logged. +* `str(exc)` remains concise, for example `500: Internal Server Error`, and does not include the response body, excerpt, or `response_data`. + +Usage: + +```python +try: + player = mlb.get_person(664034) +except mlbstatsapi.MlbHttpError as exc: + print(exc.method) + print(exc.status_code) + print(exc.reason) + print(exc.url) + print(exc.response_data) + print(exc.body_excerpt) +``` ## Existing 404 behavior -Version 0.8.0 preserves endpoint-specific not-found behavior. +Version 0.9.0 preserves endpoint-specific not-found behavior in both compatibility mode +and strict mode. Depending on the endpoint, a 404 may become: @@ -262,7 +602,23 @@ None {} ``` -Not every 404 raises `MlbHttpError`. +Not every 404 raises `MlbHttpError`. Strict mode does not change this, and a 404 never +emits `MlbHttpCompatibilityWarning`. + +## Version 1.0 migration direction + +Version 0.9.0 keeps compatibility mode as the default. + +The `MlbHttpCompatibilityWarning` notices exist to give applications advance migration +guidance: each warning marks a response that strict mode would already have raised on. + +A future 1.0 release may make stricter non-404 4xx behavior the default. No final 1.0 +decision is implemented here, and nothing about the current return shapes changes in +version 0.9.0. + +Applications that want the future-facing behavior today can enable strict mode, and +applications that want to find affected call sites early can turn +`MlbHttpCompatibilityWarning` into an error. ## No response caching @@ -274,4 +630,4 @@ The client has no default response cache. The client remains synchronous. -Async support is not part of version 0.8.0. +Async support is not part of version 0.9.0. diff --git a/docs/releases/0.9.0.md b/docs/releases/0.9.0.md new file mode 100644 index 0000000..a169d49 --- /dev/null +++ b/docs/releases/0.9.0.md @@ -0,0 +1,235 @@ +# python-mlb-statsapi 0.9.0 + +Version 0.9.0 is the configurable HTTP behavior release. + +Version 0.8.0 made the network layer reliable. Version 0.9.0 makes it explainable and +adjustable: callers can now reuse the library's retry policy, inspect much more context +on a failed request, and choose whether an unexpected HTTP response raises or falls back +to the historical empty result. + +Default return values and endpoint behavior remain compatible. Compatibility mode is still the default, but final non-404 4xx responses now emit `MlbHttpCompatibilityWarning`. Applications that treat warnings as errors may need to handle or selectively filter this warning, or opt into `strict_http=True`. + +Most applications do not need code changes to upgrade because compatibility mode remains the default. Applications that treat warnings as errors should handle or selectively filter `MlbHttpCompatibilityWarning`, or opt into `strict_http=True`. + +## Highlights + +### Public retry policy + +`create_retry_policy()` is now part of the public API: + +```python +import requests +import mlbstatsapi + +session = requests.Session() +adapter = requests.adapters.HTTPAdapter( + max_retries=mlbstatsapi.create_retry_policy(), +) +session.mount("https://", adapter) +session.mount("http://", adapter) + +try: + with mlbstatsapi.Mlb(session=session) as mlb: + player = mlb.get_person(664034) +finally: + session.close() +``` + +Each call returns a new `urllib3.util.retry.Retry` instance configured exactly like the +policy the library mounts on Sessions it creates, so a caller-managed Session can opt in +to the same tested behavior instead of reinventing it. + +Injected Sessions are still never modified automatically. The caller mounts the adapters +and the caller closes the Session. + +The retry values themselves are unchanged from version 0.8.0: up to three retries for +GET requests, a 0.5 backoff factor, `Retry-After` respected, and a retryable status list +of 429, 500, 502, 503, and 504. + +### Richer HTTP errors + +`MlbHttpError` now carries enough context to diagnose a failure without re-running the +request: + +```text +method +status_code +reason +url +response_data +body_excerpt +``` + +```python +try: + with mlbstatsapi.Mlb() as mlb: + player = mlb.get_person(664034) +except mlbstatsapi.MlbHttpError as exc: + print(exc.method) + print(exc.status_code) + print(exc.reason) + print(exc.url) + print(exc.response_data) + print(exc.body_excerpt) +``` + +`response_data` holds the decoded JSON dictionary or list when the error body contains +one, and is `None` for HTML, plain text, empty bodies, and JSON scalars. `body_excerpt` +is a bounded excerpt of the response text, capped at 500 characters. + +Context collection is best-effort and never replaces the original HTTP error. Complete +response bodies are not retained beyond the excerpt and are not automatically logged, and +`str(exc)` stays concise. + +`status_code`, `reason`, and `url` behave exactly as they did in version 0.8.0. + +### Optional strict HTTP mode + +Applications that prefer explicit failures can opt in: + +```python +import mlbstatsapi + +with mlbstatsapi.Mlb( + strict_http=True, +) as mlb: + player = mlb.get_person(664034) +``` + +In strict mode a final non-404 4xx response raises `MlbHttpError` instead of returning +the historical empty result. Final 5xx responses raise in both modes, as they already +did. + +Strict mode is evaluated only after the bounded retry policy is exhausted, it does not +change 404 handling, and it does not affect timeout, transport, decode, or Pydantic +validation failures. + +### Compatibility warnings + +Compatibility mode remains the default, but it is no longer silent. When a final non-404 +4xx response is converted into the historical empty result, the library emits +`MlbHttpCompatibilityWarning`: + +```text +HTTP 403 for https://statsapi.mlb.com/api/v1/sports was handled through +compatibility mode and returned the historical empty result. Pass +strict_http=True to raise MlbHttpError. This compatibility behavior may +change in version 1.0. +``` + +The category inherits from `FutureWarning` so it stays visible under default filters, and +it can be targeted precisely: + +```python +import warnings +import mlbstatsapi + +warnings.filterwarnings( + "error", + category=mlbstatsapi.MlbHttpCompatibilityWarning, +) +``` + +Warning messages contain only the status code and the request URL. No warning is emitted +for successful responses, 404 responses, intermediate retries, final 5xx responses, or in +strict mode. + +### Versioned User-Agent + +A Session created by the library now identifies itself: + +```text +python-mlb-statsapi/0.9.0 +``` + +The version is read from the installed distribution metadata rather than a duplicated +source constant, so the header always matches the installed release. Only the +`User-Agent` header is set; the remaining Requests defaults are preserved. + +Caller-injected Session headers are left untouched. This header carries nothing beyond +the package name and version. + +### Session ownership remains explicit + +```text +Library-created Session + Configured and closed by the library + Receives retry adapters + Receives the package User-Agent + +Caller-injected Session + Configured and closed by the caller + Existing adapters remain untouched + Existing headers remain untouched +``` + +Ownership rules are unchanged from version 0.8.0. Version 0.9.0 only makes the +library-created side more capable, and the injected side is still never reconfigured or +closed by the library. + +### Preserved compatibility + +This release preserves: + +* The synchronous `Mlb` client and every existing endpoint method +* Existing constructor arguments, with `strict_http` added as a keyword-only option +* Existing endpoint return types +* Endpoint-specific 404 results such as `None`, `[]`, and `{}` +* Existing `MlbHttpError` attributes +* Existing broad handling through `TheMlbStatsApiException` +* Existing retry values +* Existing Session ownership behavior + +### Testing and release validation + +The release adds deterministic offline coverage for the HTTP contract, compatibility +warnings, the public retry policy, exception context, and Session ownership. Warnings are +asserted directly rather than suppressed. + +Packaging is validated by `scripts/validate_release.py`, which builds on the artifacts in +`dist/` and checks the wheel and source distribution, the reported distribution name and +version, a clean-virtual-environment installation, the public imports, the retry policy +return type, both HTTP modes, the User-Agent produced from installed metadata, and that +injected Session headers survive untouched. The validator never contacts the MLB API and +runs in offline CI. + +## Migration guidance + +No action is required to upgrade. Compatibility mode is the default and existing code +keeps its current behavior. + +If `MlbHttpCompatibilityWarning` appears in your logs, it marks a call site where the MLB +API returned a non-404 4xx and the library returned an empty result anyway. That is a +real request failure being hidden, and it is worth handling. + +The recommended path is: + +1. Leave the default in place and watch for the warning +2. Handle or investigate the call sites the warning identifies +3. Turn the warning into an error in tests to keep new occurrences from creeping in +4. Enable `strict_http=True` once those call sites handle `MlbHttpError` + +Version 0.9.0 keeps compatibility mode as the default, and the warnings provide advance +migration guidance. A future 1.0 release may make stricter non-404 4xx behavior the +default. No final 1.0 decision is being made in this release. + +## Installation + +Once version 0.9.0 is published: + +```bash +python3 -m pip install --upgrade python-mlb-statsapi +``` + +## Not included + +Version 0.9.0 does not add: + +* Async support +* Response caching +* New MLB endpoints +* Global rate limiting +* Strict behavior for 404 responses +* Strict mode as the default +* New retry values +* Telemetry diff --git a/mlbstatsapi/__init__.py b/mlbstatsapi/__init__.py index a2d6669..3a046df 100644 --- a/mlbstatsapi/__init__.py +++ b/mlbstatsapi/__init__.py @@ -1,5 +1,5 @@ from .mlb_api import Mlb -from .mlb_dataadapter import MlbDataAdapter, MlbResult +from .mlb_dataadapter import MlbDataAdapter, MlbResult, create_retry_policy from .exceptions import ( MlbDecodeError, MlbHttpError, @@ -7,6 +7,7 @@ MlbTransportError, TheMlbStatsApiException, ) +from .warnings import MlbHttpCompatibilityWarning from .mlb_module import ( return_splits, diff --git a/mlbstatsapi/exceptions.py b/mlbstatsapi/exceptions.py index 7270de3..d174e21 100644 --- a/mlbstatsapi/exceptions.py +++ b/mlbstatsapi/exceptions.py @@ -18,10 +18,17 @@ def __init__( status_code: int, reason: str, url: str | None = None, + *, + method: str | None = None, + response_data: dict | list | None = None, + body_excerpt: str | None = None, ): self.status_code = int(status_code) self.reason = str(reason) self.url = url + self.method = method.upper() if method is not None else None + self.response_data = response_data + self.body_excerpt = body_excerpt super().__init__( f"{self.status_code}: {self.reason}" diff --git a/mlbstatsapi/mlb_api.py b/mlbstatsapi/mlb_api.py index 26921e6..1e8a6d5 100644 --- a/mlbstatsapi/mlb_api.py +++ b/mlbstatsapi/mlb_api.py @@ -26,7 +26,7 @@ DEFAULT_TIMEOUT, MlbDataAdapter, TimeoutType, - _configure_retry_adapters, + _configure_library_session, ) # from .exceptions import TheMlbStatsApiException from . import mlb_module @@ -51,24 +51,29 @@ def __init__( logger: logging.Logger | None = None, timeout: TimeoutType = DEFAULT_TIMEOUT, session: requests.Session | None = None, + *, + strict_http: bool = False, ): # One session is shared by the v1 and v1.1 adapters. The library closes # only sessions it creates; caller-injected sessions remain caller-owned. - # Retry adapters are installed only on library-created Sessions. + # The versioned User-Agent and retry adapters are applied only to + # library-created Sessions. self._owns_session = session is None if session is None: self._session = requests.Session() - _configure_retry_adapters(self._session) + _configure_library_session(self._session) else: self._session = session self._closed = False self._timeout = timeout + self._strict_http = strict_http self._mlb_adapter_v1 = MlbDataAdapter( hostname, 'v1', logger, timeout=timeout, session=self._session, + strict_http=strict_http, ) self._mlb_adapter_v1_1 = MlbDataAdapter( hostname, @@ -76,6 +81,7 @@ def __init__( logger, timeout=timeout, session=self._session, + strict_http=strict_http, ) self._logger = logger or logging.getLogger(__name__) self._logger.setLevel(logging.DEBUG) diff --git a/mlbstatsapi/mlb_dataadapter.py b/mlbstatsapi/mlb_dataadapter.py index f9a416c..4bc66d1 100644 --- a/mlbstatsapi/mlb_dataadapter.py +++ b/mlbstatsapi/mlb_dataadapter.py @@ -1,3 +1,4 @@ +from importlib.metadata import PackageNotFoundError, version as package_version from typing import Dict from .exceptions import ( @@ -6,7 +7,9 @@ MlbTimeoutError, MlbTransportError, ) +from .warnings import MlbHttpCompatibilityWarning import logging +import warnings import requests from requests.adapters import HTTPAdapter @@ -17,8 +20,114 @@ DEFAULT_TIMEOUT = (3.05, 30.0) TimeoutType = int | float | tuple[float, float] +# Distribution name published on PyPI; the User-Agent version is read from its +# installed metadata so no release version is duplicated in source. +PACKAGE_DISTRIBUTION_NAME = "python-mlb-statsapi" +UNKNOWN_PACKAGE_VERSION = "unknown" -def _build_retry_policy() -> Retry: +# Bounded excerpt for error response bodies attached to MlbHttpError. +HTTP_ERROR_BODY_EXCERPT_LIMIT = 500 + +# Frames from warnings.warn() out to whoever called MlbDataAdapter.get(), so the +# warning points at application code rather than the helper below. +COMPATIBILITY_WARNING_STACKLEVEL = 3 + + +def _warn_http_compatibility( + *, + status_code: int, + url: str, +) -> None: + """Warn that compatibility mode suppressed an error strict mode would raise. + + Only the status code and URL are reported; response bodies, headers, and + credentials must never reach a warning message. + """ + warnings.warn( + ( + f"HTTP {status_code} for {url} was handled through compatibility mode " + "and returned the historical empty result. Pass strict_http=True to " + "raise MlbHttpError. This compatibility behavior may change in " + "version 1.0." + ), + MlbHttpCompatibilityWarning, + stacklevel=COMPATIBILITY_WARNING_STACKLEVEL, + ) + + +def _extract_error_response_data( + response: requests.Response, +) -> dict | list | None: + """Best-effort JSON object/list extraction from an error response. + + Returns None for empty bodies, invalid JSON, scalars, or unexpected failures. + Must not raise; context extraction cannot replace the original HTTP error. + """ + try: + if not response.content: + return None + data = response.json() + except Exception: + return None + + if isinstance(data, (dict, list)): + return data + return None + + +def _extract_error_body_excerpt( + response: requests.Response, +) -> str | None: + """Best-effort bounded text excerpt from an error response body. + + Returns None for empty bodies or unexpected text-decoding failures. + Must not raise; context extraction cannot replace the original HTTP error. + """ + try: + if not response.content: + return None + text = response.text + except Exception: + return None + + if not text: + return None + return text[:HTTP_ERROR_BODY_EXCERPT_LIMIT] + + +def _build_http_error( + response: requests.Response, + *, + method: str, + fallback_url: str, +) -> MlbHttpError: + """Build an MlbHttpError with best-effort response context. + + Extraction failures must not prevent raising MlbHttpError with status, + reason, URL, and method. + """ + try: + response_data = _extract_error_response_data(response) + except Exception: + response_data = None + + try: + body_excerpt = _extract_error_body_excerpt(response) + except Exception: + body_excerpt = None + + return MlbHttpError( + status_code=response.status_code, + reason=response.reason, + url=response.url or fallback_url, + method=method, + response_data=response_data, + body_excerpt=body_excerpt, + ) + + +def create_retry_policy() -> Retry: + """Create a new instance of the default MLB HTTP retry policy.""" return Retry( total=3, connect=3, @@ -49,17 +158,45 @@ def _configure_retry_adapters( session.mount( "https://", HTTPAdapter( - max_retries=_build_retry_policy() + max_retries=create_retry_policy(), ), ) session.mount( "http://", HTTPAdapter( - max_retries=_build_retry_policy() + max_retries=create_retry_policy(), ), ) +def _build_user_agent() -> str: + """Build the package User-Agent from installed distribution metadata. + + Falls back to an "unknown" version for source-only environments where the + distribution metadata is not installed. This must never raise, because it + runs while a library-created Session is being constructed. + """ + try: + installed_version = package_version(PACKAGE_DISTRIBUTION_NAME) + except PackageNotFoundError: + installed_version = UNKNOWN_PACKAGE_VERSION + return f"{PACKAGE_DISTRIBUTION_NAME}/{installed_version}" + + +def _configure_library_session( + session: requests.Session, +) -> None: + """Apply library defaults to a Session the library created and owns. + + Caller-injected Sessions must not be passed here; their headers and + adapters stay under the caller's control. + """ + # Only the User-Agent is replaced so the remaining Requests default + # headers (Accept-Encoding, Accept, Connection) are preserved. + session.headers["User-Agent"] = _build_user_agent() + _configure_retry_adapters(session) + + class MlbResult: """ A class that holds data, status_code, and message returned from statsapi.mlb.com @@ -109,14 +246,17 @@ def __init__( logger: logging.Logger | None = None, timeout: TimeoutType = DEFAULT_TIMEOUT, session: requests.Session | None = None, + *, + strict_http: bool = False, ): self.url = f'https://{hostname}/api/{ver}/' self._logger = logger or logging.getLogger(__name__) self._timeout = timeout + self._strict_http = strict_http self._owns_session = session is None if session is None: self._session = requests.Session() - _configure_retry_adapters(self._session) + _configure_library_session(self._session) else: self._session = session self._closed = False @@ -167,6 +307,19 @@ def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> MlbRe response.reason, response.url, )) + # Strict mode raises for final non-404 4xx after retries are exhausted. + # 404 stays an empty MlbResult so endpoints keep None / [] / {} behavior. + if self._strict_http and status_code != 404: + raise _build_http_error( + response, + method="GET", + fallback_url=full_url, + ) + if status_code != 404: + _warn_http_compatibility( + status_code=status_code, + url=response.url or full_url, + ) return MlbResult( status_code=status_code, message=response.reason, @@ -180,17 +333,17 @@ def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> MlbRe response.reason, response.url, )) - raise MlbHttpError( - status_code=status_code, - reason=response.reason, - url=response.url, + raise _build_http_error( + response, + method="GET", + fallback_url=full_url, ) if not 200 <= status_code <= 299: - raise MlbHttpError( - status_code=status_code, - reason=response.reason, - url=response.url, + raise _build_http_error( + response, + method="GET", + fallback_url=full_url, ) self._logger.debug(msg=logline_post.format( diff --git a/mlbstatsapi/warnings.py b/mlbstatsapi/warnings.py new file mode 100644 index 0000000..4a681d0 --- /dev/null +++ b/mlbstatsapi/warnings.py @@ -0,0 +1,13 @@ +"""Package warning categories. + +Kept in a dedicated module so callers can filter package warnings by class +without importing the HTTP adapter internals. +""" + + +class MlbHttpCompatibilityWarning(FutureWarning): + """A compatibility-mode HTTP response that strict mode would raise. + + FutureWarning is used instead of DeprecationWarning because this notice is + aimed at application users and must stay visible under default filters. + """ diff --git a/pyproject.toml b/pyproject.toml index 94705db..ef962dc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "python-mlb-statsapi" -version = "0.8.0" +version = "0.9.0" description = "mlbstatsapi python wrapper" authors = [ "Matthew Spah ", diff --git a/scripts/validate_release.py b/scripts/validate_release.py new file mode 100644 index 0000000..ff45dfc --- /dev/null +++ b/scripts/validate_release.py @@ -0,0 +1,343 @@ +"""Validate the built python-mlb-statsapi distributions before a release. + +Checks the artifacts in ``dist/``, then installs the wheel into a throwaway +virtual environment and runs a public-import smoke test against the *installed* +package. + +The smoke test deliberately runs from a temporary directory so the repository +checkout cannot shadow the installed distribution. + +Nothing here contacts the MLB API. + +Usage:: + + python scripts/validate_release.py + python scripts/validate_release.py --dist dist --expected-version 0.9.0 +""" + +from __future__ import annotations + +import argparse +import re +import subprocess +import sys +import tarfile +import tempfile +import venv +import zipfile +from email.parser import Parser +from pathlib import Path + +DISTRIBUTION_NAME = "python-mlb-statsapi" +NORMALIZED_DISTRIBUTION_NAME = "python_mlb_statsapi" +EXPECTED_REQUIRES_PYTHON = ">=3.10" + +# Paths every source distribution must carry so the project can be rebuilt and +# read from the sdist alone. +REQUIRED_SDIST_PATHS = ( + "README.md", + "pyproject.toml", + "mlbstatsapi/__init__.py", +) + +SMOKE_TEST_SOURCE = ''' +"""Public import smoke test for an installed python-mlb-statsapi wheel.""" + +import importlib.metadata +import inspect +import sys +from pathlib import Path + +import requests +from urllib3.util.retry import Retry + +import mlbstatsapi +from mlbstatsapi import ( + Mlb, + MlbDataAdapter, + MlbDecodeError, + MlbHttpCompatibilityWarning, + MlbHttpError, + MlbTimeoutError, + MlbTransportError, + TheMlbStatsApiException, + create_retry_policy, +) + +expected_version = sys.argv[1] + +package_file = Path(mlbstatsapi.__file__).resolve() +assert "site-packages" in package_file.parts, ( + f"mlbstatsapi was imported from {package_file}, not from the installed wheel" +) + +installed_version = importlib.metadata.version("python-mlb-statsapi") +assert installed_version == expected_version, ( + f"installed metadata reports {installed_version}, expected {expected_version}" +) + +assert callable(create_retry_policy) +retry_policy = create_retry_policy() +assert isinstance(retry_policy, Retry), type(retry_policy) +assert create_retry_policy() is not retry_policy, ( + "create_retry_policy() must return a new Retry instance per call" +) + +assert issubclass(MlbHttpCompatibilityWarning, FutureWarning) +assert issubclass(MlbHttpError, TheMlbStatsApiException) +assert issubclass(MlbTimeoutError, MlbTransportError) +assert issubclass(MlbTransportError, TheMlbStatsApiException) +assert issubclass(MlbDecodeError, TheMlbStatsApiException) + +# Compatibility mode is the default in this release. +assert ( + inspect.signature(Mlb.__init__).parameters["strict_http"].default is False +) +assert ( + inspect.signature(MlbDataAdapter.__init__).parameters["strict_http"].default + is False +) + +# A library-created Session is library-owned, so reading its User-Agent through +# the private attribute is acceptable for internal release validation only. +expected_user_agent = f"python-mlb-statsapi/{expected_version}" +with Mlb() as mlb: + user_agent = mlb._session.headers["User-Agent"] + assert user_agent == expected_user_agent, user_agent + +# Strict mode is constructible and injected Session headers stay untouched. +session = requests.Session() +session.headers.update( + { + "User-Agent": "release-smoke-test/1.0", + "X-Release-Test": "preserved", + } +) +try: + with Mlb(session=session, strict_http=True): + pass + assert session.headers["User-Agent"] == "release-smoke-test/1.0" + assert session.headers["X-Release-Test"] == "preserved" +finally: + session.close() + +print(f"smoke test passed for python-mlb-statsapi {installed_version}") +''' + + +class ValidationError(Exception): + """A release validation check failed.""" + + +def _log(message: str) -> None: + print(message, flush=True) + + +def _read_expected_version(project_root: Path) -> str: + """Read the declared project version from pyproject.toml. + + Uses tomllib when available and falls back to a narrow regex so the + validator also runs on Python 3.10, which the package still supports. + """ + pyproject = project_root / "pyproject.toml" + text = pyproject.read_text(encoding="utf-8") + + try: + import tomllib + except ModuleNotFoundError: + match = re.search( + r'^version\s*=\s*"([^"]+)"', + text, + flags=re.MULTILINE, + ) + if match is None: + raise ValidationError(f"could not find a version in {pyproject}") + return match.group(1) + + data = tomllib.loads(text) + project_version = data.get("project", {}).get("version") + poetry_version = data.get("tool", {}).get("poetry", {}).get("version") + version = project_version or poetry_version + if not version: + raise ValidationError(f"could not find a version in {pyproject}") + return version + + +def _find_single(dist_dir: Path, pattern: str, label: str) -> Path: + matches = sorted(dist_dir.glob(pattern)) + if not matches: + raise ValidationError( + f"no {label} matching {pattern!r} in {dist_dir}; run `poetry build` first" + ) + if len(matches) > 1: + names = ", ".join(path.name for path in matches) + raise ValidationError( + f"expected exactly one {label} in {dist_dir}, found: {names}. " + "Remove stale artifacts and rebuild." + ) + return matches[0] + + +def _check_wheel_metadata(wheel: Path, expected_version: str) -> None: + with zipfile.ZipFile(wheel) as archive: + metadata_names = [ + name + for name in archive.namelist() + if name.endswith(".dist-info/METADATA") + ] + if len(metadata_names) != 1: + raise ValidationError( + f"expected one METADATA file in {wheel.name}, found {metadata_names}" + ) + raw_metadata = archive.read(metadata_names[0]).decode("utf-8") + + metadata = Parser().parsestr(raw_metadata) + + name = metadata.get("Name") + if name != DISTRIBUTION_NAME: + raise ValidationError(f"wheel Name is {name!r}, expected {DISTRIBUTION_NAME!r}") + + version = metadata.get("Version") + if version != expected_version: + raise ValidationError( + f"wheel Version is {version!r}, expected {expected_version!r}" + ) + + requires_python = metadata.get("Requires-Python") + if requires_python != EXPECTED_REQUIRES_PYTHON: + raise ValidationError( + f"wheel Requires-Python is {requires_python!r}, " + f"expected {EXPECTED_REQUIRES_PYTHON!r}" + ) + + _log( + f" wheel metadata: Name={name} Version={version} " + f"Requires-Python={requires_python}" + ) + + +def _check_sdist_contents(sdist: Path) -> None: + with tarfile.open(sdist, "r:gz") as archive: + members = archive.getnames() + + # Every path inside an sdist is prefixed with the versioned root directory. + relative_paths = {name.split("/", 1)[1] for name in members if "/" in name} + + missing = [path for path in REQUIRED_SDIST_PATHS if path not in relative_paths] + if missing: + raise ValidationError( + f"source distribution {sdist.name} is missing: {', '.join(missing)}" + ) + + _log(f" sdist contains: {', '.join(REQUIRED_SDIST_PATHS)}") + + +def _venv_python(venv_dir: Path) -> Path: + candidates = ( + venv_dir / "bin" / "python", + venv_dir / "Scripts" / "python.exe", + ) + for candidate in candidates: + if candidate.exists(): + return candidate + raise ValidationError(f"no interpreter found in {venv_dir}") + + +def _run(command: list[str], *, cwd: Path) -> None: + result = subprocess.run(command, cwd=cwd, check=False) + if result.returncode != 0: + printable = " ".join(command) + raise ValidationError(f"command failed ({result.returncode}): {printable}") + + +def _check_clean_install(wheel: Path, expected_version: str) -> None: + with tempfile.TemporaryDirectory(prefix="python-mlb-statsapi-release-") as tmp: + workspace = Path(tmp) + venv_dir = workspace / "venv" + + _log(f" creating clean virtual environment in {venv_dir}") + venv.EnvBuilder(with_pip=True, clear=True).create(venv_dir) + python = _venv_python(venv_dir) + + _run( + [str(python), "-m", "pip", "install", "--upgrade", "--quiet", "pip"], + cwd=workspace, + ) + _log(f" installing {wheel.name}") + _run( + [str(python), "-m", "pip", "install", "--quiet", str(wheel.resolve())], + cwd=workspace, + ) + + smoke_test = workspace / "release_smoke_test.py" + smoke_test.write_text(SMOKE_TEST_SOURCE, encoding="utf-8") + + # Run from the temporary directory so the repository checkout is not on + # sys.path and cannot shadow the installed distribution. + _log(" running public import smoke test against the installed wheel") + _run([str(python), str(smoke_test), expected_version], cwd=workspace) + + +def validate(dist_dir: Path, expected_version: str) -> None: + _log(f"Validating release {expected_version} in {dist_dir}") + + wheel = _find_single( + dist_dir, + f"{NORMALIZED_DISTRIBUTION_NAME}-{expected_version}-*.whl", + "wheel", + ) + _log(f" wheel: {wheel.name}") + + sdist = _find_single( + dist_dir, + f"{NORMALIZED_DISTRIBUTION_NAME}-{expected_version}.tar.gz", + "source distribution", + ) + _log(f" source distribution: {sdist.name}") + + _check_wheel_metadata(wheel, expected_version) + _check_sdist_contents(sdist) + _check_clean_install(wheel, expected_version) + + _log(f"Release validation passed for {DISTRIBUTION_NAME} {expected_version}") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--project-root", + type=Path, + default=Path(__file__).resolve().parent.parent, + help="repository root containing pyproject.toml", + ) + parser.add_argument( + "--dist", + type=Path, + default=None, + help="directory holding the built artifacts (default: /dist)", + ) + parser.add_argument( + "--expected-version", + default=None, + help="version to validate (default: the version declared in pyproject.toml)", + ) + args = parser.parse_args(argv) + + project_root = args.project_root.resolve() + dist_dir = (args.dist or project_root / "dist").resolve() + + try: + expected_version = args.expected_version or _read_expected_version(project_root) + if not dist_dir.is_dir(): + raise ValidationError( + f"{dist_dir} does not exist; run `poetry build` first" + ) + validate(dist_dir, expected_version) + except ValidationError as exc: + print(f"Release validation failed: {exc}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/http_contract_support.py b/tests/http_contract_support.py new file mode 100644 index 0000000..df1de81 --- /dev/null +++ b/tests/http_contract_support.py @@ -0,0 +1,87 @@ +"""Shared offline HTTP status matrices and retry-policy assertions. + +These status groups document the version 0.8.0 compatibility baseline and are +intended for reuse by later version 0.9.0 strict-mode and retry-policy tests. + +They must not contact the live MLB API. +""" + +from __future__ import annotations + +from urllib3.util.retry import Retry + + +# Final non-404 client errors that currently return an empty MlbResult by +# default rather than raising MlbHttpError. Later strict-mode work should +# reuse this matrix when asserting the opposite behavior. +COMPATIBILITY_CLIENT_ERRORS = ( + 400, + 401, + 403, + 405, + 422, + 429, +) + +NOT_FOUND_STATUS = 404 + +SERVER_ERRORS = ( + 500, + 502, + 503, + 504, +) + +# Statuses retried by the library-created Session policy. +RETRYABLE_STATUS_CODES = ( + 429, + 500, + 502, + 503, + 504, +) + +# Ordinary client errors that must not be retried. Includes 404 and the +# non-429 compatibility client errors. +NON_RETRYABLE_CLIENT_ERRORS = ( + 400, + 401, + 403, + 404, + 405, + 422, +) + +HTTP_REASON_BY_STATUS = { + 400: "Bad Request", + 401: "Unauthorized", + 403: "Forbidden", + 404: "Not Found", + 405: "Method Not Allowed", + 422: "Unprocessable Entity", + 429: "Too Many Requests", + 500: "Internal Server Error", + 502: "Bad Gateway", + 503: "Service Unavailable", + 504: "Gateway Timeout", +} + + +def assert_library_retry_policy(retry: Retry) -> None: + """Assert the default library retry policy values. + + Kept in tests so retry and contract modules share one assertion helper + for the public `create_retry_policy()` configuration. + """ + assert retry.total == 3 + assert retry.connect == 3 + assert retry.read == 2 + assert retry.status == 3 + assert retry.backoff_factor == 0.5 + assert set(retry.status_forcelist) == set(RETRYABLE_STATUS_CODES) + assert retry.allowed_methods == frozenset({"GET"}) + assert retry.respect_retry_after_header is True + assert retry.raise_on_status is False + assert "POST" not in retry.allowed_methods + assert "PATCH" not in retry.allowed_methods + assert "DELETE" not in retry.allowed_methods diff --git a/tests/test_http_contract.py b/tests/test_http_contract.py new file mode 100644 index 0000000..2193d54 --- /dev/null +++ b/tests/test_http_contract.py @@ -0,0 +1,634 @@ +"""High-level offline HTTP compatibility and strict-mode contract for version 0.9.0. + +Protects existing version 0.8.0 public compatibility behavior and the opt-in +strict HTTP mode introduced for version 0.9.0. +""" + +from __future__ import annotations + +import json +from unittest.mock import MagicMock + +import pytest +import requests + +from mlbstatsapi import ( + Mlb, + MlbDataAdapter, + MlbDecodeError, + MlbHttpError, + MlbResult, + MlbTimeoutError, + MlbTransportError, + TheMlbStatsApiException, +) +from mlbstatsapi.mlb_dataadapter import DEFAULT_TIMEOUT + +from http_contract_support import ( + COMPATIBILITY_CLIENT_ERRORS, + HTTP_REASON_BY_STATUS, + NOT_FOUND_STATUS, + SERVER_ERRORS, + assert_library_retry_policy, +) + +# Non-404 client errors asserted under strict mode (final 429 covered in retries). +STRICT_NON_404_CLIENT_ERRORS = tuple( + code for code in COMPATIBILITY_CLIENT_ERRORS if code != 429 +) + + +def _response( + *, + status_code: int, + reason: str, + url: str, + content: bytes = b"", + payload=None, + text: str | None = None, +): + """Build a fake requests Response for offline adapter tests.""" + response = MagicMock() + response.status_code = status_code + response.reason = reason + response.url = url + response.content = content + if text is not None: + response.text = text + elif content: + response.text = content.decode("utf-8", errors="replace") + else: + response.text = "" + if payload is None: + response.json.side_effect = ValueError("no json") + else: + response.json.return_value = payload + return response + + +class RecordingSession: + """Minimal session stand-in that records get() and close() calls.""" + + def __init__(self): + self.calls = [] + self.close_calls = 0 + self.headers = requests.structures.CaseInsensitiveDict() + + def get(self, url, params=None, timeout=None, **kwargs): + self.calls.append( + { + "url": url, + "params": params, + "timeout": timeout, + "kwargs": kwargs, + } + ) + return _response( + status_code=200, + reason="OK", + url=url, + content=b'{"sports": []}', + payload={"sports": []}, + ) + + def close(self): + self.close_calls += 1 + + +# --- Status matrices remain available for later strict-mode work --- + + +def test_status_matrices_document_compatibility_baseline(): + """Keep the shared status groups stable for later strict-mode tests.""" + assert COMPATIBILITY_CLIENT_ERRORS == (400, 401, 403, 405, 422, 429) + assert STRICT_NON_404_CLIENT_ERRORS == (400, 401, 403, 405, 422) + assert NOT_FOUND_STATUS == 404 + assert SERVER_ERRORS == (500, 502, 503, 504) + assert 404 not in COMPATIBILITY_CLIENT_ERRORS + assert 429 in COMPATIBILITY_CLIENT_ERRORS + assert set(SERVER_ERRORS).isdisjoint(COMPATIBILITY_CLIENT_ERRORS) + + +# --- Default / explicit mode wiring --- + + +def test_mlb_default_uses_compatibility_mode(): + """Mlb() defaults to compatibility mode on the client and both adapters.""" + mlb = Mlb() + try: + assert mlb._strict_http is False + assert mlb._mlb_adapter_v1._strict_http is False + assert mlb._mlb_adapter_v1_1._strict_http is False + finally: + mlb.close() + + +def test_mlb_explicit_compatibility_mode_matches_default(): + """Mlb(strict_http=False) matches the default compatibility wiring.""" + mlb = Mlb(strict_http=False) + try: + assert mlb._strict_http is False + assert mlb._mlb_adapter_v1._strict_http is False + assert mlb._mlb_adapter_v1_1._strict_http is False + finally: + mlb.close() + + +def test_mlb_explicit_strict_mode_wires_both_adapters(): + """Mlb(strict_http=True) enables strict mode on both internal adapters.""" + mlb = Mlb(strict_http=True) + try: + assert mlb._strict_http is True + assert mlb._mlb_adapter_v1._strict_http is True + assert mlb._mlb_adapter_v1_1._strict_http is True + finally: + mlb.close() + + +# --- Default 4xx compatibility (empty MlbResult, no MlbHttpError) --- + + +@pytest.mark.parametrize("status_code", COMPATIBILITY_CLIENT_ERRORS) +def test_compatibility_client_errors_return_empty_mlb_result(status_code): + """Non-404 4xx responses return an empty MlbResult via the Mlb adapters.""" + reason = HTTP_REASON_BY_STATUS[status_code] + url = "https://statsapi.mlb.com/api/v1/sports" + session = MagicMock() + session.get.return_value = _response( + status_code=status_code, + reason=reason, + url=url, + ) + mlb = Mlb(session=session) + + result = mlb._mlb_adapter_v1.get(endpoint="sports") + + assert isinstance(result, MlbResult) + assert result.status_code == status_code + assert result.data == {} + + +@pytest.mark.parametrize("status_code", COMPATIBILITY_CLIENT_ERRORS) +def test_compatibility_client_errors_do_not_raise_mlb_http_error(status_code): + """Compatibility-mode client errors must not raise MlbHttpError.""" + reason = HTTP_REASON_BY_STATUS[status_code] + session = MagicMock() + session.get.return_value = _response( + status_code=status_code, + reason=reason, + url="https://statsapi.mlb.com/api/v1/sports", + ) + adapter = MlbDataAdapter(session=session) + + result = adapter.get(endpoint="sports") + + assert result.status_code == status_code + assert result.data == {} + + +@pytest.mark.parametrize("status_code", COMPATIBILITY_CLIENT_ERRORS) +def test_explicit_compatibility_mode_client_errors_return_empty_mlb_result( + status_code, +): + """Mlb(strict_http=False) preserves empty MlbResult for non-404 4xx.""" + reason = HTTP_REASON_BY_STATUS[status_code] + url = "https://statsapi.mlb.com/api/v1/sports" + session = MagicMock() + session.get.return_value = _response( + status_code=status_code, + reason=reason, + url=url, + ) + mlb = Mlb(session=session, strict_http=False) + + result = mlb._mlb_adapter_v1.get(endpoint="sports") + + assert isinstance(result, MlbResult) + assert result.status_code == status_code + assert result.data == {} + + +# --- Strict non-404 4xx raises enriched MlbHttpError --- + + +@pytest.mark.parametrize("status_code", STRICT_NON_404_CLIENT_ERRORS) +def test_strict_non_404_client_errors_raise_enriched_mlb_http_error(status_code): + """Strict mode raises MlbHttpError with richer context for non-404 4xx.""" + reason = HTTP_REASON_BY_STATUS[status_code] + url = "https://statsapi.mlb.com/api/v1/sports" + payload = {"message": "client error", "code": status_code} + body = json.dumps(payload).encode("utf-8") + session = MagicMock() + session.get.return_value = _response( + status_code=status_code, + reason=reason, + url=url, + content=body, + payload=payload, + ) + mlb = Mlb(session=session, strict_http=True) + + with pytest.raises(MlbHttpError) as exc_info: + mlb._mlb_adapter_v1.get(endpoint="sports") + + exc = exc_info.value + assert isinstance(exc, TheMlbStatsApiException) + assert exc.status_code == status_code + assert exc.reason == reason + assert exc.url == url + assert exc.method == "GET" + assert exc.response_data == payload + assert exc.body_excerpt is not None + assert "client error" in exc.body_excerpt + + +@pytest.mark.parametrize("status_code", STRICT_NON_404_CLIENT_ERRORS) +def test_strict_adapter_non_404_client_errors_raise_mlb_http_error(status_code): + """Standalone adapters honor strict_http for non-404 4xx responses.""" + reason = HTTP_REASON_BY_STATUS[status_code] + url = "https://statsapi.mlb.com/api/v1/sports" + payload = {"error": reason} + body = json.dumps(payload).encode("utf-8") + session = MagicMock() + session.get.return_value = _response( + status_code=status_code, + reason=reason, + url=url, + content=body, + payload=payload, + ) + adapter = MlbDataAdapter(session=session, strict_http=True) + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert exc.status_code == status_code + assert exc.reason == reason + assert exc.url == url + assert exc.method == "GET" + assert exc.response_data == payload + assert exc.body_excerpt is not None + + +# --- Endpoint-specific 404 return shapes via public Mlb methods --- + + +def test_mlb_get_person_404_returns_none(): + """Single-object endpoints keep returning None on 404.""" + session = MagicMock() + session.get.return_value = _response( + status_code=NOT_FOUND_STATUS, + reason=HTTP_REASON_BY_STATUS[NOT_FOUND_STATUS], + url="https://statsapi.mlb.com/api/v1/people/999999", + ) + mlb = Mlb(session=session) + + assert mlb.get_person(999999) is None + + +def test_mlb_get_teams_404_returns_empty_list(): + """Collection endpoints keep returning an empty list on 404.""" + session = MagicMock() + session.get.return_value = _response( + status_code=NOT_FOUND_STATUS, + reason=HTTP_REASON_BY_STATUS[NOT_FOUND_STATUS], + url="https://statsapi.mlb.com/api/v1/teams", + ) + mlb = Mlb(session=session) + + assert mlb.get_teams() == [] + + +def test_mlb_get_player_stats_404_returns_empty_mapping(): + """Mapping endpoints keep returning an empty dict on 404.""" + session = MagicMock() + session.get.return_value = _response( + status_code=NOT_FOUND_STATUS, + reason=HTTP_REASON_BY_STATUS[NOT_FOUND_STATUS], + url="https://statsapi.mlb.com/api/v1/people/999999/stats", + ) + mlb = Mlb(session=session) + + assert mlb.get_player_stats(999999, ["season"], ["hitting"]) == {} + + +def test_mlb_endpoint_404_does_not_raise_mlb_http_error(): + """Public 404 handling stays domain-level empty results, not MlbHttpError.""" + session = MagicMock() + session.get.return_value = _response( + status_code=NOT_FOUND_STATUS, + reason=HTTP_REASON_BY_STATUS[NOT_FOUND_STATUS], + url="https://statsapi.mlb.com/api/v1/people/999999", + ) + mlb = Mlb(session=session) + + assert mlb.get_person(999999) is None + assert mlb.get_teams() == [] + assert mlb.get_player_stats(999999, ["season"], ["hitting"]) == {} + + +@pytest.mark.parametrize("strict_http", [False, True]) +def test_404_preserves_endpoint_shapes_in_both_modes(strict_http): + """404 keeps None / [] / {} endpoint shapes in compatibility and strict modes.""" + session = MagicMock() + session.get.return_value = _response( + status_code=NOT_FOUND_STATUS, + reason=HTTP_REASON_BY_STATUS[NOT_FOUND_STATUS], + url="https://statsapi.mlb.com/api/v1/people/999999", + ) + mlb = Mlb(session=session, strict_http=strict_http) + + assert mlb.get_person(999999) is None + assert mlb.get_teams() == [] + assert mlb.get_player_stats(999999, ["season"], ["hitting"]) == {} + + +@pytest.mark.parametrize("strict_http", [False, True]) +def test_adapter_404_returns_mlb_result_in_both_modes(strict_http): + """Adapters return a 404 MlbResult rather than raising in either mode.""" + url = "https://statsapi.mlb.com/api/v1/people/999999" + session = MagicMock() + session.get.return_value = _response( + status_code=NOT_FOUND_STATUS, + reason=HTTP_REASON_BY_STATUS[NOT_FOUND_STATUS], + url=url, + ) + adapter = MlbDataAdapter(session=session, strict_http=strict_http) + + result = adapter.get(endpoint="people/999999") + + assert isinstance(result, MlbResult) + assert result.status_code == NOT_FOUND_STATUS + assert result.data == {} + + +# --- Final 5xx raises MlbHttpError with existing attributes --- + + +@pytest.mark.parametrize("status_code", SERVER_ERRORS) +def test_mlb_server_errors_raise_mlb_http_error_with_existing_attributes(status_code): + """Persistent 5xx failures raise MlbHttpError with status_code, reason, and url.""" + reason = HTTP_REASON_BY_STATUS[status_code] + url = "https://statsapi.mlb.com/api/v1/people/664034" + session = MagicMock() + session.get.return_value = _response( + status_code=status_code, + reason=reason, + url=url, + ) + mlb = Mlb(session=session) + + with pytest.raises(MlbHttpError) as exc_info: + mlb.get_person(664034) + + exc = exc_info.value + assert isinstance(exc, TheMlbStatsApiException) + assert exc.status_code == status_code + assert exc.reason == reason + assert exc.url == url + # Protect useful message content without freezing the full exception string; + # issue #268 may enrich MlbHttpError while keeping these attributes. + assert str(status_code) in str(exc) + assert reason in str(exc) + + +@pytest.mark.parametrize("status_code", [500, 502]) +@pytest.mark.parametrize("strict_http", [False, True]) +def test_server_errors_raise_enriched_mlb_http_error_in_both_modes( + status_code, + strict_http, +): + """Final 5xx responses raise enriched MlbHttpError in both HTTP modes.""" + reason = HTTP_REASON_BY_STATUS[status_code] + url = "https://statsapi.mlb.com/api/v1/sports" + payload = {"message": "server error", "status": status_code} + body = json.dumps(payload).encode("utf-8") + session = MagicMock() + session.get.return_value = _response( + status_code=status_code, + reason=reason, + url=url, + content=body, + payload=payload, + ) + mlb = Mlb(session=session, strict_http=strict_http) + + with pytest.raises(MlbHttpError) as exc_info: + mlb._mlb_adapter_v1.get(endpoint="sports") + + exc = exc_info.value + assert exc.status_code == status_code + assert exc.reason == reason + assert exc.url == url + assert exc.method == "GET" + assert exc.response_data == payload + assert exc.body_excerpt is not None + assert "server error" in exc.body_excerpt + + +# --- Transport and decode errors remain unchanged in strict mode --- + + +def test_strict_mode_timeout_still_raises_mlb_timeout_error(): + """Strict mode does not convert timeouts into MlbHttpError.""" + original = requests.exceptions.Timeout("timed out") + session = MagicMock() + session.get.side_effect = original + adapter = MlbDataAdapter(session=session, strict_http=True) + + with pytest.raises(MlbTimeoutError, match=r"^Request failed$") as exc_info: + adapter.get(endpoint="sports") + + assert isinstance(exc_info.value, MlbTransportError) + assert not isinstance(exc_info.value, MlbHttpError) + assert exc_info.value.__cause__ is original + + +def test_strict_mode_connection_failure_still_raises_mlb_transport_error(): + """Strict mode does not convert connection failures into MlbHttpError.""" + original = requests.exceptions.ConnectionError("connection refused") + session = MagicMock() + session.get.side_effect = original + adapter = MlbDataAdapter(session=session, strict_http=True) + + with pytest.raises(MlbTransportError, match=r"^Request failed$") as exc_info: + adapter.get(endpoint="sports") + + assert not isinstance(exc_info.value, MlbTimeoutError) + assert not isinstance(exc_info.value, MlbHttpError) + assert exc_info.value.__cause__ is original + + +def test_strict_mode_invalid_json_still_raises_mlb_decode_error(): + """Strict mode does not convert 2xx JSON decode failures into MlbHttpError.""" + url = "https://statsapi.mlb.com/api/v1/sports" + session = MagicMock() + session.get.return_value = _response( + status_code=200, + reason="OK", + url=url, + content=b'{"bad": json', + text='{"bad": json', + ) + # Force json() to raise like a real Response with malformed body. + session.get.return_value.json.side_effect = ValueError("Expecting value") + adapter = MlbDataAdapter(session=session, strict_http=True) + + with pytest.raises(MlbDecodeError, match=r"^Bad JSON in response$") as exc_info: + adapter.get(endpoint="sports") + + assert not isinstance(exc_info.value, MlbHttpError) + + +# --- Library-created retry policy (contract-level guard) --- + + +def test_library_created_mlb_session_retains_retry_policy_contract(): + """Library-created Mlb Sessions keep the existing retry policy values.""" + mlb = Mlb() + try: + for scheme in ("https://", "http://"): + assert_library_retry_policy(mlb._session.get_adapter(scheme).max_retries) + finally: + mlb.close() + + +# --- Constructor compatibility / positional argument order --- + + +def test_mlb_constructors_remain_compatible(): + """Existing Mlb() positional constructor usage continues to work.""" + mlb_default = Mlb() + try: + assert isinstance(mlb_default._session, requests.Session) + assert mlb_default._timeout == DEFAULT_TIMEOUT + assert mlb_default._strict_http is False + finally: + mlb_default.close() + + mlb_host = Mlb("statsapi.mlb.com") + try: + assert mlb_host._mlb_adapter_v1.url.startswith( + "https://statsapi.mlb.com/api/v1/" + ) + finally: + mlb_host.close() + + logger = MagicMock() + logger.level = 0 + mlb_logger = Mlb("statsapi.mlb.com", logger) + try: + assert mlb_logger._logger is logger + finally: + mlb_logger.close() + + +def test_mlb_positional_timeout_remains_third_argument(): + """Guard against inserting new positional args before timeout.""" + session = RecordingSession() + mlb = Mlb("statsapi.mlb.com", None, 10, session) + + mlb._mlb_adapter_v1.get(endpoint="sports") + mlb._mlb_adapter_v1_1.get(endpoint="game") + + assert session.calls[0]["timeout"] == 10 + assert session.calls[1]["timeout"] == 10 + assert mlb._session is session + assert mlb._strict_http is False + + +def test_mlb_strict_http_is_keyword_only(): + """strict_http must not shift existing positional constructor arguments.""" + logger = MagicMock() + logger.level = 0 + session = RecordingSession() + + mlb = Mlb("statsapi.mlb.com", logger, 10, session) + assert mlb._session is session + assert mlb._timeout == 10 + assert mlb._strict_http is False + + mlb_strict = Mlb( + "statsapi.mlb.com", + logger, + 10, + session, + strict_http=True, + ) + assert mlb_strict._session is session + assert mlb_strict._timeout == 10 + assert mlb_strict._strict_http is True + assert mlb_strict._mlb_adapter_v1._strict_http is True + assert mlb_strict._mlb_adapter_v1_1._strict_http is True + + with pytest.raises(TypeError): + Mlb("statsapi.mlb.com", logger, 10, session, True) + + +def test_adapter_positional_construction_remains_compatible(): + """Existing MlbDataAdapter positional construction order stays intact.""" + logger = MagicMock() + session = RecordingSession() + adapter = MlbDataAdapter("statsapi.mlb.com", "v1.1", logger, (5.0, 60.0), session) + + assert adapter.url == "https://statsapi.mlb.com/api/v1.1/" + assert adapter._logger is logger + assert adapter._timeout == (5.0, 60.0) + assert adapter._session is session + assert adapter._strict_http is False + + adapter.get(endpoint="game") + assert session.calls[0]["timeout"] == (5.0, 60.0) + + +def test_adapter_strict_http_is_keyword_only(): + """Adapter strict_http is keyword-only and does not shift positional args.""" + logger = MagicMock() + session = RecordingSession() + adapter = MlbDataAdapter( + "statsapi.mlb.com", + "v1.1", + logger, + (5.0, 60.0), + session, + strict_http=True, + ) + assert adapter._strict_http is True + + with pytest.raises(TypeError): + MlbDataAdapter( + "statsapi.mlb.com", + "v1.1", + logger, + (5.0, 60.0), + session, + True, + ) + + +# --- Timeout forwarding at the Mlb client level --- + + +def test_mlb_timeout_constructors_forward_to_both_adapters(): + """Default, scalar, and tuple timeouts reach both v1 and v1.1 adapters.""" + default_session = RecordingSession() + mlb_default = Mlb(session=default_session) + mlb_default._mlb_adapter_v1.get(endpoint="sports") + mlb_default._mlb_adapter_v1_1.get(endpoint="game") + assert default_session.calls[0]["timeout"] == DEFAULT_TIMEOUT + assert default_session.calls[1]["timeout"] == DEFAULT_TIMEOUT + + scalar_session = RecordingSession() + mlb_scalar = Mlb(session=scalar_session, timeout=10) + mlb_scalar._mlb_adapter_v1.get(endpoint="sports") + mlb_scalar._mlb_adapter_v1_1.get(endpoint="game") + assert scalar_session.calls[0]["timeout"] == 10 + assert scalar_session.calls[1]["timeout"] == 10 + + tuple_session = RecordingSession() + mlb_tuple = Mlb(session=tuple_session, timeout=(5.0, 60.0)) + mlb_tuple._mlb_adapter_v1.get(endpoint="sports") + mlb_tuple._mlb_adapter_v1_1.get(endpoint="game") + assert tuple_session.calls[0]["timeout"] == (5.0, 60.0) + assert tuple_session.calls[1]["timeout"] == (5.0, 60.0) diff --git a/tests/test_http_warnings.py b/tests/test_http_warnings.py new file mode 100644 index 0000000..f19e9bb --- /dev/null +++ b/tests/test_http_warnings.py @@ -0,0 +1,437 @@ +"""Offline tests for the compatibility-mode HTTP warning added in version 0.9.0. + +Kept separate from the strict/compatibility contract module so warning behavior +stays readable. These tests must not contact the live MLB API. +""" + +from __future__ import annotations + +import contextlib +import json +import warnings +from unittest.mock import MagicMock + +import pytest +import requests + +import mlbstatsapi +from mlbstatsapi import ( + Mlb, + MlbDataAdapter, + MlbDecodeError, + MlbHttpCompatibilityWarning, + MlbHttpError, + MlbTimeoutError, + MlbTransportError, +) + +from http_contract_support import ( + COMPATIBILITY_CLIENT_ERRORS, + HTTP_REASON_BY_STATUS, + NOT_FOUND_STATUS, + SERVER_ERRORS, +) + +# Final 429 is retried first, so it is covered in tests/test_mlb_retries.py. +WARNING_CLIENT_ERRORS = tuple( + code for code in COMPATIBILITY_CLIENT_ERRORS if code != 429 +) + +SPORTS_URL = "https://statsapi.mlb.com/api/v1/sports" + + +def _response( + *, + status_code: int, + reason: str, + url: str, + content: bytes = b"", + payload=None, + text: str | None = None, +): + """Build a fake requests Response for offline adapter tests.""" + response = MagicMock() + response.status_code = status_code + response.reason = reason + response.url = url + response.content = content + if text is not None: + response.text = text + elif content: + response.text = content.decode("utf-8", errors="replace") + else: + response.text = "" + if payload is None: + response.json.side_effect = ValueError("no json") + else: + response.json.return_value = payload + return response + + +def _session_returning(response) -> MagicMock: + session = MagicMock() + session.get.return_value = response + return session + + +def _session_for_status( + status_code: int, + *, + url: str = SPORTS_URL, + payload=None, +) -> MagicMock: + content = b"" if payload is None else json.dumps(payload).encode("utf-8") + return _session_returning( + _response( + status_code=status_code, + reason=HTTP_REASON_BY_STATUS[status_code], + url=url, + content=content, + payload=payload, + ) + ) + + +@contextlib.contextmanager +def _recorded_compatibility_warnings(): + """Record every MlbHttpCompatibilityWarning raised inside the block.""" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", MlbHttpCompatibilityWarning) + yield caught + + +def _compatibility_warnings(caught) -> list: + return [ + item + for item in caught + if issubclass(item.category, MlbHttpCompatibilityWarning) + ] + + +# --- Public warning class --- + + +def test_compatibility_warning_is_publicly_importable(): + """The warning category is reachable from the package namespace.""" + assert ( + mlbstatsapi.MlbHttpCompatibilityWarning + is MlbHttpCompatibilityWarning + ) + + +def test_compatibility_warning_inherits_future_warning(): + """FutureWarning keeps this migration notice visible by default.""" + assert issubclass(MlbHttpCompatibilityWarning, FutureWarning) + assert issubclass(MlbHttpCompatibilityWarning, Warning) + assert not issubclass(MlbHttpCompatibilityWarning, DeprecationWarning) + + +# --- Compatibility-mode non-404 4xx warns exactly once --- + + +@pytest.mark.parametrize("status_code", WARNING_CLIENT_ERRORS) +def test_compatibility_client_errors_warn_once_and_return_empty_result(status_code): + """Non-404 4xx warns once while preserving the historical empty result.""" + session = _session_for_status(status_code) + adapter = MlbDataAdapter(session=session) + + with pytest.warns(MlbHttpCompatibilityWarning) as warning_info: + result = adapter.get(endpoint="sports") + + assert result.status_code == status_code + assert result.message == HTTP_REASON_BY_STATUS[status_code] + assert result.data == {} + assert len(warning_info) == 1 + + +@pytest.mark.parametrize("status_code", WARNING_CLIENT_ERRORS) +def test_compatibility_warning_message_contains_migration_guidance(status_code): + """The message carries status, URL, mode, migration, and version guidance.""" + session = _session_for_status(status_code) + adapter = MlbDataAdapter(session=session) + + with pytest.warns(MlbHttpCompatibilityWarning) as warning_info: + adapter.get(endpoint="sports") + + message = str(warning_info[0].message) + assert str(status_code) in message + assert SPORTS_URL in message + assert "compatibility mode" in message + assert "strict_http=True" in message + assert "MlbHttpError" in message + assert "version 1.0" in message + + +def test_compatibility_warning_excludes_response_body(): + """Response bodies must never leak into the warning message.""" + payload = {"message": "secret client error detail"} + session = _session_for_status(403, payload=payload) + adapter = MlbDataAdapter(session=session) + + with pytest.warns(MlbHttpCompatibilityWarning) as warning_info: + adapter.get(endpoint="sports") + + message = str(warning_info[0].message) + assert "secret client error detail" not in message + assert json.dumps(payload) not in message + + +def test_compatibility_warning_falls_back_to_request_url(): + """A response without a URL still reports the requested endpoint.""" + response = _response( + status_code=403, + reason=HTTP_REASON_BY_STATUS[403], + url=None, + ) + adapter = MlbDataAdapter(session=_session_returning(response)) + + with pytest.warns(MlbHttpCompatibilityWarning) as warning_info: + adapter.get(endpoint="sports") + + assert SPORTS_URL in str(warning_info[0].message) + + +# --- Client wiring: default and explicit compatibility mode --- + + +@pytest.mark.parametrize("status_code", WARNING_CLIENT_ERRORS) +def test_default_mlb_client_warns_for_non_404_client_errors(status_code): + """Mlb() stays in compatibility mode and receives the migration notice.""" + session = _session_for_status(status_code) + mlb = Mlb(session=session) + + with pytest.warns(MlbHttpCompatibilityWarning) as warning_info: + result = mlb._mlb_adapter_v1.get(endpoint="sports") + + assert result.status_code == status_code + assert result.data == {} + assert len(warning_info) == 1 + + +def test_default_mlb_client_public_endpoint_warns_and_keeps_return_shape(): + """A public endpoint keeps returning None for 4xx while warning once.""" + session = _session_for_status( + 403, + url="https://statsapi.mlb.com/api/v1/people/664034", + ) + mlb = Mlb(session=session) + + with pytest.warns(MlbHttpCompatibilityWarning) as warning_info: + person = mlb.get_person(664034) + + assert person is None + assert len(warning_info) == 1 + + +@pytest.mark.parametrize("status_code", WARNING_CLIENT_ERRORS) +def test_explicit_compatibility_mode_warns_and_preserves_result(status_code): + """Mlb(strict_http=False) warns without changing the compatibility result.""" + session = _session_for_status(status_code) + mlb = Mlb(session=session, strict_http=False) + + with pytest.warns(MlbHttpCompatibilityWarning) as warning_info: + result = mlb._mlb_adapter_v1.get(endpoint="sports") + + assert result.status_code == status_code + assert result.message == HTTP_REASON_BY_STATUS[status_code] + assert result.data == {} + assert len(warning_info) == 1 + + +# --- Strict mode raises instead of warning --- + + +@pytest.mark.parametrize("status_code", WARNING_CLIENT_ERRORS) +def test_strict_mode_raises_without_compatibility_warning(status_code): + """Strict mode raises MlbHttpError and emits no compatibility warning.""" + payload = {"error": HTTP_REASON_BY_STATUS[status_code]} + session = _session_for_status(status_code, payload=payload) + mlb = Mlb(session=session, strict_http=True) + + with _recorded_compatibility_warnings() as caught: + with pytest.raises(MlbHttpError) as exc_info: + mlb._mlb_adapter_v1.get(endpoint="sports") + + exc = exc_info.value + assert exc.status_code == status_code + assert exc.reason == HTTP_REASON_BY_STATUS[status_code] + assert exc.url == SPORTS_URL + assert exc.method == "GET" + assert exc.response_data == payload + assert _compatibility_warnings(caught) == [] + + +# --- 404 stays warning-free in both modes --- + + +@pytest.mark.parametrize("strict_http", [False, True]) +def test_adapter_404_does_not_warn(strict_http): + """A final 404 returns an MlbResult without warning in either mode.""" + session = _session_for_status( + NOT_FOUND_STATUS, + url="https://statsapi.mlb.com/api/v1/people/999999", + ) + adapter = MlbDataAdapter(session=session, strict_http=strict_http) + + with _recorded_compatibility_warnings() as caught: + result = adapter.get(endpoint="people/999999") + + assert result.status_code == NOT_FOUND_STATUS + assert result.data == {} + assert _compatibility_warnings(caught) == [] + + +@pytest.mark.parametrize("strict_http", [False, True]) +def test_public_404_return_shapes_do_not_warn(strict_http): + """404 keeps None / [] / {} endpoint shapes without emitting a warning.""" + session = _session_for_status( + NOT_FOUND_STATUS, + url="https://statsapi.mlb.com/api/v1/people/999999", + ) + mlb = Mlb(session=session, strict_http=strict_http) + + with _recorded_compatibility_warnings() as caught: + assert mlb.get_person(999999) is None + assert mlb.get_teams() == [] + assert mlb.get_player_stats(999999, ["season"], ["hitting"]) == {} + + assert _compatibility_warnings(caught) == [] + + +# --- Successful responses stay warning-free --- + + +@pytest.mark.parametrize("strict_http", [False, True]) +def test_successful_response_does_not_warn(strict_http): + """A 200 response parses normally and emits no compatibility warning.""" + payload = {"sports": [{"id": 1}]} + session = _session_returning( + _response( + status_code=200, + reason="OK", + url=SPORTS_URL, + content=json.dumps(payload).encode("utf-8"), + payload=payload, + ) + ) + adapter = MlbDataAdapter(session=session, strict_http=strict_http) + + with _recorded_compatibility_warnings() as caught: + result = adapter.get(endpoint="sports") + + assert result.status_code == 200 + assert result.data == payload + assert _compatibility_warnings(caught) == [] + + +# --- Final 5xx already raises, so it is not compatibility-suppressed --- + + +@pytest.mark.parametrize("status_code", SERVER_ERRORS) +@pytest.mark.parametrize("strict_http", [False, True]) +def test_final_server_errors_do_not_warn(status_code, strict_http): + """Final 5xx raises MlbHttpError in both modes without a compatibility warning.""" + session = _session_for_status(status_code) + adapter = MlbDataAdapter(session=session, strict_http=strict_http) + + with _recorded_compatibility_warnings() as caught: + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + assert exc_info.value.status_code == status_code + assert _compatibility_warnings(caught) == [] + + +# --- Transport, timeout, and decode failures stay warning-free --- + + +@pytest.mark.parametrize("strict_http", [False, True]) +def test_timeout_does_not_warn(strict_http): + """Timeouts raise MlbTimeoutError without a compatibility warning.""" + session = MagicMock() + session.get.side_effect = requests.exceptions.Timeout("timed out") + adapter = MlbDataAdapter(session=session, strict_http=strict_http) + + with _recorded_compatibility_warnings() as caught: + with pytest.raises(MlbTimeoutError): + adapter.get(endpoint="sports") + + assert _compatibility_warnings(caught) == [] + + +@pytest.mark.parametrize("strict_http", [False, True]) +def test_connection_failure_does_not_warn(strict_http): + """Connection failures raise MlbTransportError without a compatibility warning.""" + session = MagicMock() + session.get.side_effect = requests.exceptions.ConnectionError("refused") + adapter = MlbDataAdapter(session=session, strict_http=strict_http) + + with _recorded_compatibility_warnings() as caught: + with pytest.raises(MlbTransportError): + adapter.get(endpoint="sports") + + assert _compatibility_warnings(caught) == [] + + +@pytest.mark.parametrize("strict_http", [False, True]) +def test_decode_failure_does_not_warn(strict_http): + """Malformed JSON on a 200 raises MlbDecodeError without warning.""" + response = _response( + status_code=200, + reason="OK", + url=SPORTS_URL, + content=b'{"bad": json', + text='{"bad": json', + ) + response.json.side_effect = ValueError("Expecting value") + adapter = MlbDataAdapter( + session=_session_returning(response), + strict_http=strict_http, + ) + + with _recorded_compatibility_warnings() as caught: + with pytest.raises(MlbDecodeError): + adapter.get(endpoint="sports") + + assert _compatibility_warnings(caught) == [] + + +# --- Caller-controlled warning filters --- + + +def test_caller_can_turn_the_warning_into_an_exception(): + """A caller may promote only this category to an error.""" + session = _session_for_status(403) + adapter = MlbDataAdapter(session=session) + + with warnings.catch_warnings(): + warnings.simplefilter("error", MlbHttpCompatibilityWarning) + with pytest.raises(MlbHttpCompatibilityWarning): + adapter.get(endpoint="sports") + + +def test_caller_can_ignore_only_this_warning_category(): + """Ignoring the category keeps the historical compatibility result.""" + session = _session_for_status(403) + adapter = MlbDataAdapter(session=session) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + warnings.simplefilter("ignore", MlbHttpCompatibilityWarning) + result = adapter.get(endpoint="sports") + + assert result.status_code == 403 + assert result.data == {} + assert _compatibility_warnings(caught) == [] + + +def test_each_request_emits_its_own_warning(): + """Every final non-404 4xx warns; deduplication is left to warning filters.""" + session = _session_for_status(429) + adapter = MlbDataAdapter(session=session) + + with _recorded_compatibility_warnings() as caught: + adapter.get(endpoint="sports") + adapter.get(endpoint="sports") + + assert len(_compatibility_warnings(caught)) == 2 diff --git a/tests/test_mlb_exceptions.py b/tests/test_mlb_exceptions.py index 66f59b0..effbff3 100644 --- a/tests/test_mlb_exceptions.py +++ b/tests/test_mlb_exceptions.py @@ -1,5 +1,7 @@ """Offline tests for the structured MLB Stats API exception hierarchy.""" +from unittest.mock import MagicMock, patch + import pytest import requests @@ -12,6 +14,10 @@ MlbTransportError, TheMlbStatsApiException, ) +from mlbstatsapi.mlb_dataadapter import ( + HTTP_ERROR_BODY_EXCERPT_LIMIT, + _build_http_error, +) BASE_URL = "https://statsapi.mlb.com/api/v1/" @@ -56,6 +62,33 @@ def test_mlb_http_error_attributes_and_message(): assert str(exc) == "500: Internal Server Error" +def test_mlb_http_error_positional_construction_preserves_compat(): + exc = MlbHttpError( + 500, + "Internal Server Error", + "https://example.test", + ) + + assert exc.status_code == 500 + assert exc.reason == "Internal Server Error" + assert exc.url == "https://example.test" + assert exc.method is None + assert exc.response_data is None + assert exc.body_excerpt is None + assert isinstance(exc, TheMlbStatsApiException) + assert str(exc) == "500: Internal Server Error" + + +def test_mlb_http_error_method_normalized_to_uppercase(): + exc = MlbHttpError( + 500, + "Internal Server Error", + method="get", + ) + + assert exc.method == "GET" + + def test_timeout_raises_mlb_timeout_error(): original = requests.exceptions.Timeout("timed out") session = requests.Session() @@ -153,10 +186,288 @@ def test_server_error_raises_mlb_http_error(status_code, reason, requests_mock): assert exc_info.value.status_code == status_code assert exc_info.value.reason == reason assert exc_info.value.url == url + assert exc_info.value.method == "GET" assert str(exc_info.value) == f"{status_code}: {reason}" adapter.close() +def test_json_object_error_populates_response_data(requests_mock): + payload = { + "message": "Internal error occurred", + "code": "SERVICE_FAILURE", + } + url = f"{BASE_URL}sports" + requests_mock.get( + url, + json=payload, + status_code=500, + reason="Internal Server Error", + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert exc.method == "GET" + assert exc.response_data == payload + assert exc.body_excerpt is not None + assert len(exc.body_excerpt) <= HTTP_ERROR_BODY_EXCERPT_LIMIT + assert "Internal error occurred" in exc.body_excerpt + assert str(exc) == "500: Internal Server Error" + adapter.close() + + +def test_json_list_error_populates_response_data(requests_mock): + payload = [{"message": "error"}, {"message": "another"}] + requests_mock.get( + f"{BASE_URL}sports", + json=payload, + status_code=500, + reason="Internal Server Error", + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + assert exc_info.value.response_data == payload + assert exc_info.value.method == "GET" + adapter.close() + + +def test_invalid_json_error_keeps_mlb_http_error(requests_mock): + malformed = '{"message": broken' + requests_mock.get( + f"{BASE_URL}sports", + text=malformed, + status_code=500, + reason="Internal Server Error", + headers={"Content-Type": "application/json"}, + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert not isinstance(exc, MlbDecodeError) + assert exc.response_data is None + assert exc.body_excerpt == malformed + assert str(exc) == "500: Internal Server Error" + adapter.close() + + +def test_html_error_response_context(requests_mock): + html = "Internal Server Error" + requests_mock.get( + f"{BASE_URL}sports", + text=html, + status_code=500, + reason="Internal Server Error", + headers={"Content-Type": "text/html"}, + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert exc.response_data is None + assert html in (exc.body_excerpt or "") + adapter.close() + + +def test_plain_text_error_response_context(requests_mock): + text = "temporary failure, try again later" + requests_mock.get( + f"{BASE_URL}sports", + text=text, + status_code=503, + reason="Service Unavailable", + headers={"Content-Type": "text/plain"}, + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert exc.response_data is None + assert text in (exc.body_excerpt or "") + adapter.close() + + +def test_empty_error_response_context(requests_mock): + requests_mock.get( + f"{BASE_URL}sports", + text="", + status_code=500, + reason="Internal Server Error", + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert exc.response_data is None + assert exc.body_excerpt is None + adapter.close() + + +def test_large_error_body_excerpt_is_bounded(requests_mock): + body = "x" * (HTTP_ERROR_BODY_EXCERPT_LIMIT + 250) + requests_mock.get( + f"{BASE_URL}sports", + text=body, + status_code=500, + reason="Internal Server Error", + headers={"Content-Type": "text/plain"}, + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert len(exc.body_excerpt) <= HTTP_ERROR_BODY_EXCERPT_LIMIT + assert exc.body_excerpt == body[:HTTP_ERROR_BODY_EXCERPT_LIMIT] + assert body not in str(exc) + assert str(exc) == "500: Internal Server Error" + adapter.close() + + +def test_json_scalar_error_response_data_is_none(requests_mock): + requests_mock.get( + f"{BASE_URL}sports", + text='"server unavailable"', + status_code=500, + reason="Internal Server Error", + headers={"Content-Type": "application/json"}, + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert exc.response_data is None + assert "server unavailable" in (exc.body_excerpt or "") + adapter.close() + + +def test_non_ascii_error_body_excerpt(requests_mock): + text = "エラー: サーバー障害 — café" + requests_mock.get( + f"{BASE_URL}sports", + text=text, + status_code=500, + reason="Internal Server Error", + headers={"Content-Type": "text/plain; charset=utf-8"}, + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + assert text in (exc_info.value.body_excerpt or "") + adapter.close() + + +def test_unexpected_status_raises_mlb_http_error_with_context(requests_mock): + requests_mock.get( + f"{BASE_URL}sports", + text="redirect loop", + status_code=308, + reason="Permanent Redirect", + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value + assert exc.status_code == 308 + assert exc.method == "GET" + assert exc.response_data is None + assert "redirect loop" in (exc.body_excerpt or "") + adapter.close() + + +def test_url_fallback_when_response_url_missing(): + response = MagicMock() + response.status_code = 500 + response.reason = "Internal Server Error" + response.url = "" + response.content = b'{"message": "boom"}' + response.json.return_value = {"message": "boom"} + response.text = '{"message": "boom"}' + + exc = _build_http_error( + response, + method="GET", + fallback_url=f"{BASE_URL}sports", + ) + + assert exc.url == f"{BASE_URL}sports" + assert exc.method == "GET" + assert exc.response_data == {"message": "boom"} + + +def test_best_effort_extraction_failure_still_raises_mlb_http_error(requests_mock): + requests_mock.get( + f"{BASE_URL}sports", + text="failure body", + status_code=500, + reason="Internal Server Error", + ) + adapter = MlbDataAdapter() + + with ( + patch( + "mlbstatsapi.mlb_dataadapter._extract_error_response_data", + side_effect=RuntimeError("unexpected json failure"), + ), + patch( + "mlbstatsapi.mlb_dataadapter._extract_error_body_excerpt", + side_effect=RuntimeError("unexpected text failure"), + ), + pytest.raises(MlbHttpError) as exc_info, + ): + adapter.get(endpoint="sports") + + exc = exc_info.value + assert exc.status_code == 500 + assert exc.reason == "Internal Server Error" + assert exc.url == f"{BASE_URL}sports" + assert exc.method == "GET" + assert exc.response_data is None + assert exc.body_excerpt is None + assert str(exc) == "500: Internal Server Error" + adapter.close() + + +def test_error_response_body_is_not_logged(requests_mock, caplog): + secret = "DO-NOT-LOG-THIS-BODY" + requests_mock.get( + f"{BASE_URL}sports", + text=secret, + status_code=500, + reason="Internal Server Error", + headers={"Content-Type": "text/plain"}, + ) + adapter = MlbDataAdapter() + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + assert secret in (exc_info.value.body_excerpt or "") + logged = " ".join(record.getMessage() for record in caplog.records) + assert secret not in logged + adapter.close() + + def test_injected_session_exception_wrapping_still_works(): original = requests.exceptions.Timeout("timed out") session = requests.Session() diff --git a/tests/test_mlb_retries.py b/tests/test_mlb_retries.py index 0bae978..6fab006 100644 --- a/tests/test_mlb_retries.py +++ b/tests/test_mlb_retries.py @@ -4,6 +4,7 @@ import json import threading +import warnings from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from typing import Iterable @@ -12,45 +13,74 @@ from requests.adapters import HTTPAdapter from urllib3.util.retry import Retry -from mlbstatsapi import Mlb, MlbDataAdapter, MlbHttpError, MlbResult +import mlbstatsapi +from mlbstatsapi import ( + Mlb, + MlbDataAdapter, + MlbHttpCompatibilityWarning, + MlbHttpError, + MlbResult, + create_retry_policy, +) + +from http_contract_support import ( + NON_RETRYABLE_CLIENT_ERRORS, + RETRYABLE_STATUS_CODES, + SERVER_ERRORS, + assert_library_retry_policy, +) + + +def test_create_retry_policy_is_publicly_importable(): + """create_retry_policy is available through the package public API.""" + assert callable(mlbstatsapi.create_retry_policy) + assert create_retry_policy is mlbstatsapi.create_retry_policy -def _assert_retry_policy(retry: Retry) -> None: - assert retry.total == 3 - assert retry.connect == 3 - assert retry.read == 2 - assert retry.status == 3 - assert retry.backoff_factor == 0.5 - assert set(retry.status_forcelist) == {429, 500, 502, 503, 504} - assert retry.allowed_methods == frozenset({"GET"}) - assert retry.respect_retry_after_header is True - assert retry.raise_on_status is False - assert "POST" not in retry.allowed_methods - assert "PATCH" not in retry.allowed_methods - assert "DELETE" not in retry.allowed_methods +def test_create_retry_policy_returns_retry_instance(): + """create_retry_policy returns an urllib3 Retry with the library policy.""" + policy = create_retry_policy() + assert isinstance(policy, Retry) + assert_library_retry_policy(policy) + + +def test_create_retry_policy_returns_independent_instances(): + """Each call returns a distinct Retry instance with the same configuration.""" + first = create_retry_policy() + second = create_retry_policy() + assert first is not second + assert_library_retry_policy(first) + assert_library_retry_policy(second) def test_library_created_mlb_session_has_retry_policy(): + """Library-created Mlb Sessions mount the default retry policy on http/https.""" mlb = Mlb() try: - for scheme in ("https://", "http://"): - adapter = mlb._session.get_adapter(scheme) - _assert_retry_policy(adapter.max_retries) + https_adapter = mlb._session.get_adapter("https://") + http_adapter = mlb._session.get_adapter("http://") + assert_library_retry_policy(https_adapter.max_retries) + assert_library_retry_policy(http_adapter.max_retries) + assert https_adapter.max_retries is not http_adapter.max_retries finally: mlb.close() def test_library_created_adapter_session_has_retry_policy(): + """Library-created MlbDataAdapter Sessions mount the default retry policy.""" adapter = MlbDataAdapter() try: - for scheme in ("https://", "http://"): - http_adapter = adapter._session.get_adapter(scheme) - _assert_retry_policy(http_adapter.max_retries) + https_adapter = adapter._session.get_adapter("https://") + http_adapter = adapter._session.get_adapter("http://") + assert_library_retry_policy(https_adapter.max_retries) + assert_library_retry_policy(http_adapter.max_retries) + assert https_adapter.max_retries is not http_adapter.max_retries finally: adapter.close() def test_injected_session_adapters_are_not_replaced(): + """Mlb must not replace adapters or retry config on an injected Session.""" session = requests.Session() custom_adapter = HTTPAdapter(max_retries=0) session.mount("https://", custom_adapter) @@ -58,12 +88,14 @@ def test_injected_session_adapters_are_not_replaced(): mlb = Mlb(session=session) try: assert session.get_adapter("https://") is custom_adapter + assert session.get_adapter("https://").max_retries.total == 0 finally: mlb.close() session.close() def test_injected_adapter_session_adapters_are_not_replaced(): + """Standalone adapters must not replace adapters on an injected Session.""" session = requests.Session() custom_adapter = HTTPAdapter(max_retries=0) session.mount("http://", custom_adapter) @@ -71,11 +103,37 @@ def test_injected_adapter_session_adapters_are_not_replaced(): adapter = MlbDataAdapter(session=session) try: assert session.get_adapter("http://") is custom_adapter + assert session.get_adapter("http://").max_retries.total == 0 finally: adapter.close() session.close() +def test_caller_can_opt_in_to_public_retry_policy(): + """Callers may mount create_retry_policy() on their own Session.""" + session = requests.Session() + https_adapter = HTTPAdapter(max_retries=create_retry_policy()) + http_adapter = HTTPAdapter(max_retries=create_retry_policy()) + session.mount("https://", https_adapter) + session.mount("http://", http_adapter) + + mlb = Mlb(session=session) + try: + assert mlb._session is session + assert mlb._owns_session is False + assert session.get_adapter("https://") is https_adapter + assert session.get_adapter("http://") is http_adapter + assert_library_retry_policy(https_adapter.max_retries) + assert_library_retry_policy(http_adapter.max_retries) + assert https_adapter.max_retries is not http_adapter.max_retries + finally: + mlb.close() + # Injected Sessions remain caller-owned after Mlb.close(). + assert session.get_adapter("https://") is https_adapter + assert session.get_adapter("http://") is http_adapter + session.close() + + class _ScriptedHandler(BaseHTTPRequestHandler): """Serve a scripted sequence of HTTP status codes for retry tests.""" @@ -134,8 +192,12 @@ def no_retry_sleep(monkeypatch): monkeypatch.setattr(Retry, "sleep", lambda self, response=None: None) -def _adapter_against_local_server(port: int) -> MlbDataAdapter: - adapter = MlbDataAdapter() +def _adapter_against_local_server( + port: int, + *, + strict_http: bool = False, +) -> MlbDataAdapter: + adapter = MlbDataAdapter(strict_http=strict_http) adapter.url = f"http://127.0.0.1:{port}/api/v1/" return adapter @@ -144,6 +206,7 @@ def test_retries_recover_from_two_500_responses( scripted_http_server, no_retry_sleep, ): + """Transient 500s are retried and a later 200 succeeds.""" configure, port = scripted_http_server configure([500, 500, 200]) adapter = _adapter_against_local_server(port) @@ -159,15 +222,13 @@ def test_retries_recover_from_two_500_responses( assert _ScriptedHandler.request_count == 3 -@pytest.mark.parametrize( - "retryable_status", - [429, 500, 502, 503, 504], -) +@pytest.mark.parametrize("retryable_status", RETRYABLE_STATUS_CODES) def test_retries_retryable_statuses_then_succeed( retryable_status, scripted_http_server, no_retry_sleep, ): + """Each retryable status is retried once and can recover on success.""" configure, port = scripted_http_server configure([retryable_status, 200]) adapter = _adapter_against_local_server(port) @@ -182,15 +243,13 @@ def test_retries_retryable_statuses_then_succeed( assert _ScriptedHandler.request_count == 2 -@pytest.mark.parametrize( - "status_code", - [400, 401, 403, 404], -) +@pytest.mark.parametrize("status_code", NON_RETRYABLE_CLIENT_ERRORS) def test_non_retryable_client_errors_are_not_retried( status_code, scripted_http_server, no_retry_sleep, ): + """Ordinary client errors are returned immediately without retries.""" configure, port = scripted_http_server configure([status_code, 200]) adapter = _adapter_against_local_server(port) @@ -205,12 +264,15 @@ def test_non_retryable_client_errors_are_not_retried( assert _ScriptedHandler.request_count == 1 -def test_bounded_persistent_500_raises_after_four_attempts( +@pytest.mark.parametrize("status_code", SERVER_ERRORS) +def test_bounded_persistent_server_errors_raise_after_four_attempts( + status_code, scripted_http_server, no_retry_sleep, ): + """Persistent server errors raise MlbHttpError after initial try plus 3 retries.""" configure, port = scripted_http_server - configure([500, 500, 500, 500, 500, 500]) + configure([status_code] * 6) adapter = _adapter_against_local_server(port) try: @@ -219,8 +281,7 @@ def test_bounded_persistent_500_raises_after_four_attempts( finally: adapter.close() - assert exc_info.value.status_code == 500 - assert exc_info.value.reason == "Internal Server Error" + assert exc_info.value.status_code == status_code assert _ScriptedHandler.request_count == 4 @@ -228,6 +289,7 @@ def test_final_429_returns_empty_mlb_result( scripted_http_server, no_retry_sleep, ): + """After retry exhaustion, a final 429 still returns an empty MlbResult.""" configure, port = scripted_http_server configure([429, 429, 429, 429, 429, 429]) adapter = _adapter_against_local_server(port) @@ -243,7 +305,81 @@ def test_final_429_returns_empty_mlb_result( assert _ScriptedHandler.request_count == 4 +def test_final_429_warns_once_after_retry_exhaustion( + scripted_http_server, + no_retry_sleep, +): + """Compatibility mode warns only after the final 429, not per retry attempt.""" + configure, port = scripted_http_server + configure([429, 429, 429, 429, 429, 429]) + # Library-created Session keeps the mounted retry adapter; do not inject a mock. + adapter = _adapter_against_local_server(port) + + try: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", MlbHttpCompatibilityWarning) + result = adapter.get(endpoint="sports") + finally: + adapter.close() + + compatibility_warnings = [ + item + for item in caught + if issubclass(item.category, MlbHttpCompatibilityWarning) + ] + + assert _ScriptedHandler.request_count == 4 + assert len(compatibility_warnings) == 1 + assert "429" in str(compatibility_warnings[0].message) + assert isinstance(result, MlbResult) + assert result.status_code == 429 + assert result.data == {} + + +def test_final_429_raises_mlb_http_error_in_strict_mode( + scripted_http_server, + no_retry_sleep, +): + """Strict mode raises MlbHttpError only after retry exhaustion on final 429.""" + configure, port = scripted_http_server + configure([429, 429, 429, 429, 429, 429]) + # Library-created Session keeps the mounted retry adapter; do not inject a mock. + adapter = _adapter_against_local_server(port, strict_http=True) + + try: + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + finally: + adapter.close() + + assert _ScriptedHandler.request_count == 4 + assert exc_info.value.status_code == 429 + assert exc_info.value.method == "GET" + + +def test_final_429_raises_via_strict_mlb_client( + scripted_http_server, + no_retry_sleep, +): + """Mlb(strict_http=True) raises after retries when the final response is 429.""" + configure, port = scripted_http_server + configure([429, 429, 429, 429, 429, 429]) + mlb = Mlb(strict_http=True) + mlb._mlb_adapter_v1.url = f"http://127.0.0.1:{port}/api/v1/" + + try: + with pytest.raises(MlbHttpError) as exc_info: + mlb._mlb_adapter_v1.get(endpoint="sports") + finally: + mlb.close() + + assert _ScriptedHandler.request_count == 4 + assert exc_info.value.status_code == 429 + assert exc_info.value.method == "GET" + + def test_invalid_json_is_not_retried(no_retry_sleep): + """JSON decode failures are not treated as retryable transport errors.""" class BadJsonHandler(_ScriptedHandler): def do_GET(self) -> None: # noqa: N802 with self.lock: diff --git a/tests/test_mlb_session.py b/tests/test_mlb_session.py index 6f0c6fc..9ae7d09 100644 --- a/tests/test_mlb_session.py +++ b/tests/test_mlb_session.py @@ -1,16 +1,27 @@ -"""Offline tests for shared HTTP sessions and configurable timeouts. +"""Offline tests for shared HTTP sessions, timeouts, and the package User-Agent. -These tests cover session injection, ownership, cleanup, sharing, and timeout -forwarding without calling the live MLB API. +These tests cover session injection, ownership, cleanup, sharing, header +handling, and timeout forwarding without calling the live MLB API. """ +from importlib.metadata import PackageNotFoundError from unittest.mock import MagicMock, patch import pytest import requests from mlbstatsapi import Mlb, MlbDataAdapter, MlbHttpError, TheMlbStatsApiException -from mlbstatsapi.mlb_dataadapter import DEFAULT_TIMEOUT +from mlbstatsapi.mlb_dataadapter import ( + DEFAULT_TIMEOUT, + PACKAGE_DISTRIBUTION_NAME, + _configure_library_session, +) + +from http_contract_support import assert_library_retry_policy + + +MOCKED_PACKAGE_VERSION = "9.8.7" +MOCKED_USER_AGENT = f"python-mlb-statsapi/{MOCKED_PACKAGE_VERSION}" class RecordingSession: @@ -200,6 +211,18 @@ def test_mlb_configured_timeout_reaches_both_adapters(): assert session.calls[1]["url"].endswith("/api/v1.1/game") +def test_mlb_scalar_timeout_reaches_both_adapters(): + """Scalar Mlb(timeout=...) values are forwarded unchanged to both adapters.""" + session = RecordingSession() + mlb = Mlb(session=session, timeout=10) + + mlb._mlb_adapter_v1.get(endpoint="sports") + mlb._mlb_adapter_v1_1.get(endpoint="game") + + assert session.calls[0]["timeout"] == 10 + assert session.calls[1]["timeout"] == 10 + + def test_mlb_default_timeout_passed_to_session(): session = RecordingSession() mlb = Mlb(session=session) @@ -216,11 +239,45 @@ def test_injected_session_is_shared_by_both_adapters(): session = RecordingSession() mlb = Mlb(session=session) + assert mlb._session is session assert mlb._mlb_adapter_v1._session is session assert mlb._mlb_adapter_v1_1._session is session assert mlb._mlb_adapter_v1._session is mlb._mlb_adapter_v1_1._session +def test_injected_session_is_not_replaced_with_library_session(): + """Caller-injected Sessions are used as-is; the library creates no replacement.""" + session = requests.Session() + with patch("mlbstatsapi.mlb_api.requests.Session") as session_cls: + mlb = Mlb(session=session) + + session_cls.assert_not_called() + assert mlb._session is session + assert mlb._mlb_adapter_v1._session is session + assert mlb._mlb_adapter_v1_1._session is session + assert mlb._owns_session is False + + mlb.close() + session.close() + + +def test_injected_session_headers_and_user_agent_are_not_modified(): + """Injected Session headers, including User-Agent, stay under caller control.""" + session = requests.Session() + session.headers["User-Agent"] = "caller-agent/1.0" + session.headers["X-Caller-Header"] = "keep-me" + headers_before = dict(session.headers) + + mlb = Mlb(session=session) + try: + assert dict(session.headers) == headers_before + assert session.headers["User-Agent"] == "caller-agent/1.0" + assert session.headers["X-Caller-Header"] == "keep-me" + finally: + mlb.close() + session.close() + + def test_library_created_session_is_shared_by_both_adapters(): with patch("mlbstatsapi.mlb_api.requests.Session") as session_cls: session = MagicMock() @@ -390,3 +447,178 @@ def test_adapter_positional_hostname_version_and_logger(): adapter = MlbDataAdapter("statsapi.mlb.com", "v1.1", logger) assert adapter._logger is logger adapter.close() + + +# --- Versioned User-Agent --- + + +@pytest.fixture +def mocked_package_version(): + """Patch the metadata lookup the production User-Agent helper uses.""" + with patch( + "mlbstatsapi.mlb_dataadapter.package_version", + return_value=MOCKED_PACKAGE_VERSION, + ) as lookup: + yield lookup + + +def test_user_agent_version_comes_from_package_metadata(mocked_package_version): + """The User-Agent version is read from installed distribution metadata.""" + session = requests.Session() + try: + _configure_library_session(session) + + assert session.headers["User-Agent"] == MOCKED_USER_AGENT + finally: + session.close() + + mocked_package_version.assert_called_with(PACKAGE_DISTRIBUTION_NAME) + + +def test_mlb_library_created_session_has_versioned_user_agent(mocked_package_version): + """Library-created Mlb Sessions send the package and version User-Agent.""" + mlb = Mlb() + try: + user_agent = mlb._session.headers["User-Agent"] + + assert "python-mlb-statsapi" in user_agent + assert user_agent == MOCKED_USER_AGENT + finally: + mlb.close() + + +def test_standalone_adapter_session_has_versioned_user_agent(mocked_package_version): + """Library-created standalone adapter Sessions use the same User-Agent.""" + adapter = MlbDataAdapter() + try: + assert adapter._session.headers["User-Agent"] == MOCKED_USER_AGENT + finally: + adapter.close() + + +def test_prepared_request_carries_versioned_user_agent(mocked_package_version): + """The versioned User-Agent reaches the outgoing prepared request.""" + mlb = Mlb() + try: + prepared = mlb._session.prepare_request( + requests.Request("GET", "https://example.test"), + ) + + assert prepared.headers["User-Agent"] == MOCKED_USER_AGENT + finally: + mlb.close() + + +def test_library_created_session_preserves_requests_default_headers(): + """Only User-Agent changes; other Requests default headers are untouched.""" + baseline = requests.Session() + mlb = Mlb() + try: + for header, value in baseline.headers.items(): + if header.lower() == "user-agent": + continue + assert mlb._session.headers[header] == value + + for header in ("Accept-Encoding", "Accept", "Connection"): + assert mlb._session.headers[header] == baseline.headers[header] + + assert mlb._session.headers["User-Agent"] != baseline.headers["User-Agent"] + finally: + mlb.close() + baseline.close() + + +def test_injected_session_user_agent_is_preserved(): + """A caller-provided User-Agent is never overwritten by the library.""" + session = requests.Session() + session.headers["User-Agent"] = "my-baseball-project/1.0" + + mlb = Mlb(session=session) + try: + assert session.headers["User-Agent"] == "my-baseball-project/1.0" + assert mlb._session.headers["User-Agent"] == "my-baseball-project/1.0" + finally: + mlb.close() + + assert session.headers["User-Agent"] == "my-baseball-project/1.0" + session.close() + + +def test_injected_session_custom_headers_are_preserved(): + """Custom headers on an injected Session survive construction and close().""" + session = requests.Session() + session.headers.update( + { + "User-Agent": "my-baseball-project/1.0", + "X-Application": "scoreboard", + } + ) + headers_before = dict(session.headers) + + mlb = Mlb(session=session) + mlb.close() + + assert dict(session.headers) == headers_before + assert session.headers["User-Agent"] == "my-baseball-project/1.0" + assert session.headers["X-Application"] == "scoreboard" + session.close() + + +def test_standalone_adapter_injected_session_headers_are_preserved(): + """A Session injected into MlbDataAdapter keeps all caller headers.""" + session = requests.Session() + session.headers.update( + { + "User-Agent": "my-baseball-project/1.0", + "X-Application": "scoreboard", + } + ) + headers_before = dict(session.headers) + + adapter = MlbDataAdapter(session=session) + try: + assert dict(session.headers) == headers_before + assert session.headers["User-Agent"] == "my-baseball-project/1.0" + assert session.headers["X-Application"] == "scoreboard" + finally: + adapter.close() + session.close() + + +def test_user_agent_falls_back_when_package_metadata_is_missing(): + """Missing distribution metadata yields a safe value instead of raising.""" + with patch( + "mlbstatsapi.mlb_dataadapter.package_version", + side_effect=PackageNotFoundError(PACKAGE_DISTRIBUTION_NAME), + ): + session = requests.Session() + try: + _configure_library_session(session) + + assert session.headers["User-Agent"] == "python-mlb-statsapi/unknown" + finally: + session.close() + + +def test_mlb_construction_succeeds_without_package_metadata(): + """Client construction still works in source-only environments.""" + with patch( + "mlbstatsapi.mlb_dataadapter.package_version", + side_effect=PackageNotFoundError(PACKAGE_DISTRIBUTION_NAME), + ): + mlb = Mlb() + try: + assert mlb._session.headers["User-Agent"] == "python-mlb-statsapi/unknown" + finally: + mlb.close() + + +def test_user_agent_does_not_change_library_retry_adapters(mocked_package_version): + """Setting the User-Agent leaves the mounted retry adapters intact.""" + mlb = Mlb() + try: + assert mlb._session.headers["User-Agent"] == MOCKED_USER_AGENT + assert_library_retry_policy(mlb._session.get_adapter("https://").max_retries) + assert_library_retry_policy(mlb._session.get_adapter("http://").max_retries) + finally: + mlb.close() diff --git a/tests/test_release_validation.py b/tests/test_release_validation.py new file mode 100644 index 0000000..a20f5de --- /dev/null +++ b/tests/test_release_validation.py @@ -0,0 +1,113 @@ +"""Offline checks that the release documentation stays consistent with the package. + +These tests do not build or install anything. Packaging itself is validated by +``scripts/validate_release.py``, which runs against ``dist/`` and a clean +virtual environment. +""" + +import re +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parent.parent +README = PROJECT_ROOT / "README.md" +TRANSPORT_DOC = PROJECT_ROOT / "docs" / "http-transport.md" +RELEASE_NOTES = PROJECT_ROOT / "docs" / "releases" / "0.9.0.md" + +PYTHON_BLOCK_PATTERN = re.compile( + r"^```python\n(.*?)^```", + re.MULTILINE | re.DOTALL, +) + + +def _project_version() -> str: + text = (PROJECT_ROOT / "pyproject.toml").read_text(encoding="utf-8") + match = re.search(r'^version\s*=\s*"([^"]+)"', text, flags=re.MULTILINE) + assert match is not None, "no version found in pyproject.toml" + return match.group(1) + + +def _python_blocks(path: Path) -> list[tuple[int, str]]: + """Return (line number, source) for every non-REPL ```python block.""" + text = path.read_text(encoding="utf-8") + blocks = [] + for match in PYTHON_BLOCK_PATTERN.finditer(text): + source = match.group(1) + stripped = source.lstrip() + # Interactive blocks interleave prompts and output, so they are not + # compilable source and are excluded from syntax validation. + if stripped.startswith(">>>"): + continue + line_number = text.count("\n", 0, match.start()) + 1 + blocks.append((line_number, source)) + return blocks + + +def _documented_paths() -> list[Path]: + return [README, TRANSPORT_DOC, RELEASE_NOTES] + + +@pytest.mark.parametrize( + "path", + _documented_paths(), + ids=lambda path: path.name, +) +def test_documentation_python_examples_are_valid_syntax(path: Path) -> None: + blocks = _python_blocks(path) + assert blocks, f"expected at least one python example in {path.name}" + + for line_number, source in blocks: + compile(source, f"{path.name}:{line_number}", "exec") + + +@pytest.mark.parametrize( + "path", + _documented_paths(), + ids=lambda path: path.name, +) +def test_documentation_examples_use_public_api_only(path: Path) -> None: + """User-facing examples must not reach into private package internals. + + ``scripts/validate_release.py`` is allowed to read ``Mlb._session`` for + release validation; documentation is not. + """ + for line_number, source in _python_blocks(path): + assert "_session" not in source, ( + f"{path.name}:{line_number} uses the private _session attribute" + ) + assert "mlbstatsapi.mlb_dataadapter" not in source, ( + f"{path.name}:{line_number} imports an internal module" + ) + + +def test_release_notes_exist_for_the_declared_version() -> None: + version = _project_version() + notes = PROJECT_ROOT / "docs" / "releases" / f"{version}.md" + assert notes.is_file(), f"missing release notes for version {version}" + assert notes.read_text(encoding="utf-8").startswith( + f"# python-mlb-statsapi {version}" + ) + + +def test_documented_user_agent_matches_the_declared_version() -> None: + """The documented User-Agent must track the version the build will produce.""" + version = _project_version() + expected = f"python-mlb-statsapi/{version}" + + for path in (README, TRANSPORT_DOC, RELEASE_NOTES): + text = path.read_text(encoding="utf-8") + documented = set(re.findall(r"python-mlb-statsapi/[0-9][^\s`\"']*", text)) + assert documented == {expected}, ( + f"{path.name} documents User-Agent versions {sorted(documented)}, " + f"expected only {expected!r}" + ) + + +def test_ci_watches_the_current_release_branch() -> None: + workflow = PROJECT_ROOT / ".github" / "workflows" / "build-and-test.yml" + text = workflow.read_text(encoding="utf-8") + major, minor, _ = _project_version().split(".") + + assert f"release/{major}.{minor}.0" in text + assert "- main" in text