Skip to content

Commit 261d9f3

Browse files
authored
Merge pull request #18 from modern-python/chore/project-hygiene-tidy
chore: project hygiene tidy — publish guard, uv_build band, HTTPStatus, Response.json() charset
2 parents 8dd82b2 + 5593f21 commit 261d9f3

8 files changed

Lines changed: 39 additions & 21 deletions

File tree

Justfile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ test-branch:
2323
@just test --cov-branch
2424

2525
publish:
26+
@test -n "${GITHUB_REF_NAME:-}" || (echo "GITHUB_REF_NAME is required; refusing to run outside CI" >&2; exit 1)
27+
@test -n "${PYPI_TOKEN:-}" || (echo "PYPI_TOKEN is required; refusing to run outside CI" >&2; exit 1)
2628
rm -rf dist
2729
uv version $GITHUB_REF_NAME
2830
uv build

planning/deferred-work.md

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,12 @@ Items raised in reviews that are real but not actionable now.
44

55
## Deferred from: retrospective review of stories 1-1 through 1-5 (2026-05-31)
66

7-
- **`PydanticDecoder.decode` `TypeError` fallback is unreachable through normal usage** — the `except TypeError` branch only triggers when `_get_adapter(model)` raises during `lru_cache` lookup, which requires `model` to be unhashable; every concrete `type[T]` is hashable. The branch is now deliberately exercised via mock at `tests/test_decoders_pydantic.py:141-156`, so the contract is pinned even though no production caller hits it. (`src/httpware/decoders/pydantic.py:22-26`)
87
- **`_get_adapter` `lru_cache` is module-global, not per-decoder instance** — keyed by `model` only; two `PydanticDecoder()` instances with different configurations (none today) would share adapters, and the cache survives across tests unless explicitly cleared. Revisit if/when a configurable `PydanticDecoder(mode=..., strict=...)` lands. (`src/httpware/decoders/pydantic.py:12-14`)
98
- **`extensions=dict(request.extensions)` forwards opaque payloads to httpx2 verbatim**`httpx2` interprets specific keys (e.g. `timeout`, `sni_hostname`); a typo or unknown key silently bypasses our timeout/limits config. The seam now has a real user: `AsyncClient._build_request` writes `extensions["timeout"]` (`src/httpware/client.py:140-142`). Epic 3 timeout middleware will own the extensions contract; introducing an allowlist now risks blocking legitimate forward-compat uses. (`src/httpware/transports/httpx2.py:121`)
10-
- **`Response.json()` raises raw `JSONDecodeError` and ignores declared charset**`json.loads(self.content)` propagates `json.JSONDecodeError` to callers and ignores any declared charset (`text` honors it); inconsistent with `_try_decode_json` in the transport which never raises. `AsyncClient` doesn't call `.json()` (it goes through `decoder.decode`), but end users do. Two-line fix; bundle with the next response-API touch. (`src/httpware/response.py:50-52`)
119

1210
## Deferred from: code review of story-1-5 (2026-05-14)
1311

1412
- **Empty/malformed payload tests**`b""`, `b"null"`, `b"{}"`, invalid UTF-8: current pydantic-core behavior is correct but unpinned; a future pydantic upgrade could change error types undetected. (`tests/test_decoders_pydantic.py`)
15-
- **`PLR2004` per-file-ignores**`# noqa: PLR2004` repeated 5× in this test file; idiomatic fix is `tool.ruff.lint.per-file-ignores` for `tests/*`. Project-wide lint-config tidy. (`tests/test_decoders_pydantic.py:63,67,83,107,153`)
1613

1714
## Deferred from: code review of story-1-4 (2026-05-14)
1815

@@ -41,7 +38,5 @@ Items raised in reviews that are real but not actionable now.
4138

4239
## Deferred from: code review of story-1-1 (2026-05-13)
4340

44-
- **`just publish` lacks env-var validation** — recipe assumes `GITHUB_REF_NAME` and `PYPI_TOKEN` are set; running locally could corrupt the version. Add `test -n "$GITHUB_REF_NAME"` guard before release work. (`Justfile:25-29`)
45-
- **`uv_build>=0.11,<0.12` narrow window** — single-minor band will expire as soon as uv_build 0.12 ships; bump when that happens. (`pyproject.toml:49`)
4641
- **Unpinned `ruff`/`ty` with `select=["ALL"]`** — any new ruff release adds rules and can break CI overnight. Pin major versions or pin specific rules when a regression occurs. (`pyproject.toml` `[dependency-groups] lint`, `[tool.ruff.lint] select`)
47-
- **No `[test]` extra; CI installs all extras**`just install` runs `uv sync --all-extras --group lint`, so every CI run pulls msgspec/otel/niquests even though most tests don't need them. Declare a `test` extra (or move test-only deps into a dedicated dependency-group) and switch CI to the narrower install. Mild YAGNI today; revisit when extras grow heavier. (`pyproject.toml` `[project.optional-dependencies]`, `Justfile:install`)
42+
- **No `[test]` extra; CI installs all extras**`just install` runs `uv sync --all-extras --group lint`, so every CI run pulls msgspec/otel/niquests even though most tests don't need them. Declare a `test` extra (or move test-only deps into a dedicated dependency-group) and switch CI to the narrower install. Mild YAGNI today; revisit when extras grow heavier. (`pyproject.toml` `[project.optional-dependencies]`, `Justfile:install`)

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ repository = "https://github.com/modern-python/httpware"
4646
docs = "https://httpware.readthedocs.io"
4747

4848
[build-system]
49-
requires = ["uv_build>=0.11,<0.12"]
49+
requires = ["uv_build>=0.11,<1.0"]
5050
build-backend = "uv_build"
5151

5252
[tool.uv.build-backend]

src/httpware/response.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,13 @@ def text(self) -> str:
4848
return self.content.decode("utf-8")
4949

5050
def json(self) -> Any: # noqa: ANN401
51-
"""Parse `content` as JSON."""
52-
return json.loads(self.content)
51+
"""Parse `content` as JSON using the declared charset (default UTF-8).
52+
53+
Raises:
54+
json.JSONDecodeError: if the body is not valid JSON.
55+
56+
"""
57+
return json.loads(self.text)
5358

5459
def with_headers(self, headers: Mapping[str, str]) -> Self:
5560
"""Return a copy with the given headers merged in (incoming keys override existing)."""

src/httpware/transports/httpx2.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import json
1111
import time
1212
from contextlib import AbstractAsyncContextManager
13+
from http import HTTPStatus
1314
from typing import Any
1415

1516
import httpx2
@@ -141,10 +142,10 @@ async def __call__(self, request: Request) -> Response:
141142
# to the last value — see class docstring; widens with the
142143
# multi-valued header contract in a later story.
143144
headers = dict(resp.headers)
144-
if 400 <= status < 600: # noqa: PLR2004
145+
if HTTPStatus.BAD_REQUEST <= status < 600: # noqa: PLR2004 — 600 is the synthetic 5xx upper bound
145146
exc_class = STATUS_TO_EXCEPTION.get(
146147
status,
147-
ClientStatusError if status < 500 else ServerStatusError, # noqa: PLR2004
148+
ClientStatusError if status < HTTPStatus.INTERNAL_SERVER_ERROR else ServerStatusError,
148149
)
149150
raise exc_class(
150151
status=status,

tests/test_middleware.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import asyncio
44
from collections.abc import Awaitable, Callable
55
from contextlib import AbstractAsyncContextManager
6+
from http import HTTPStatus
67
from typing import get_type_hints
78

89
import pytest
@@ -64,7 +65,7 @@ async def test_empty_list_composes_to_transport_call() -> None:
6465
request = _make_request()
6566
response = await dispatch(request)
6667

67-
assert response.status == 200 # noqa: PLR2004
68+
assert response.status == HTTPStatus.OK
6869
assert response.content == b"transport"
6970
assert response.headers["x-from"] == "transport"
7071

@@ -192,7 +193,7 @@ async def __call__(self, request: Request, next: Next) -> Response: # noqa: A00
192193

193194
response = await compose([ShortCircuit(), NeverReached()], CountingTransport())(_make_request())
194195

195-
assert response.status == 418 # noqa: PLR2004
196+
assert response.status == HTTPStatus.IM_A_TEAPOT
196197
assert response.content == b"teapot"
197198
assert transport_calls == 0
198199

@@ -265,7 +266,7 @@ async def __call__(self, request: Request, next: Next) -> Response: # noqa: A00
265266

266267
for _ in range(3):
267268
response = await dispatch(_make_request())
268-
assert response.status == 200 # noqa: PLR2004
269+
assert response.status == HTTPStatus.OK
269270

270271
assert count == 3 # noqa: PLR2004
271272

@@ -332,7 +333,7 @@ async def recover(request: Request, exc: Exception) -> Response | None: # noqa:
332333
transport = RecordedTransport(default=RuntimeError("boom"))
333334
response = await compose([recover], transport)(_make_request())
334335

335-
assert response.status == 503 # noqa: PLR2004
336+
assert response.status == HTTPStatus.SERVICE_UNAVAILABLE
336337
assert response.headers["x-recovered"] == "true"
337338
assert response.content == b"recovered"
338339

tests/test_response.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Unit tests for httpware.response.Response."""
22

33
from dataclasses import FrozenInstanceError
4+
from http import HTTPStatus
45

56
import pytest
67

@@ -83,6 +84,18 @@ def test_response_json_parses_body() -> None:
8384
assert resp.json() == {"a": 1, "b": [2, 3]}
8485

8586

87+
def test_response_json_uses_declared_charset() -> None:
88+
body = '{"name": "café"}'.encode("iso-8859-1")
89+
resp = Response(
90+
status=HTTPStatus.OK,
91+
headers={"content-type": "application/json; charset=iso-8859-1"},
92+
content=body,
93+
url="/",
94+
elapsed=0.0,
95+
)
96+
assert resp.json() == {"name": "café"}
97+
98+
8699
def test_response_equality_on_identical_fields() -> None:
87100
r1 = Response(status=200, headers={"a": "1"}, content=b"x", url="/", elapsed=0.5)
88101
r2 = Response(status=200, headers={"a": "1"}, content=b"x", url="/", elapsed=0.5)
@@ -108,12 +121,12 @@ def test_response_with_headers_overrides_existing_key() -> None:
108121
def test_response_with_status_replaces_status() -> None:
109122
resp = Response(status=200, headers={"a": "1"}, content=b"body", url="/x", elapsed=0.5)
110123
new = resp.with_status(503)
111-
assert new.status == 503 # noqa: PLR2004
124+
assert new.status == HTTPStatus.SERVICE_UNAVAILABLE
112125
assert new.headers == {"a": "1"}
113126
assert new.content == b"body"
114127
assert new.url == "/x"
115128
assert new.elapsed == 0.5 # noqa: PLR2004
116-
assert resp.status == 200 # noqa: PLR2004
129+
assert resp.status == HTTPStatus.OK
117130

118131

119132
def test_response_with_status_accepts_arbitrary_int() -> None:

tests/test_transports_httpx2.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import asyncio
44
from collections.abc import Callable
5+
from http import HTTPStatus
56

67
import httpx2
78
import pytest
@@ -69,7 +70,7 @@ async def test_success_path_returns_response() -> None:
6970
await transport.aclose()
7071

7172
assert isinstance(resp, Response)
72-
assert resp.status == 200 # noqa: PLR2004
73+
assert resp.status == HTTPStatus.OK
7374
assert resp.content == b"hello"
7475
assert resp.url == "http://example.com/x"
7576
# lowercase ASCII keys per AC11
@@ -100,7 +101,7 @@ async def test_success_status_200_returns_response_not_raises() -> None:
100101
resp = await transport(Request(method="GET", url="http://example.com/"))
101102
finally:
102103
await transport.aclose()
103-
assert resp.status == 200 # noqa: PLR2004
104+
assert resp.status == HTTPStatus.OK
104105

105106

106107
@pytest.mark.parametrize(("code", "exc_cls"), _STATUS_LEAVES)
@@ -132,7 +133,7 @@ async def test_unknown_4xx_falls_back_to_client_status_error() -> None:
132133
finally:
133134
await transport.aclose()
134135
assert type(info.value) is ClientStatusError
135-
assert info.value.status == 418 # noqa: PLR2004
136+
assert info.value.status == HTTPStatus.IM_A_TEAPOT
136137

137138

138139
async def test_unknown_5xx_falls_back_to_server_status_error() -> None:
@@ -143,7 +144,7 @@ async def test_unknown_5xx_falls_back_to_server_status_error() -> None:
143144
finally:
144145
await transport.aclose()
145146
assert type(info.value) is ServerStatusError
146-
assert info.value.status == 504 # noqa: PLR2004
147+
assert info.value.status == HTTPStatus.GATEWAY_TIMEOUT
147148

148149

149150
# ----- (e) _try_decode_json branches ----------------------------------------

0 commit comments

Comments
 (0)