Skip to content

BK-326: Lift dev-env aiohttp<3.14 cap, floor vcrpy at 8.2#936

Open
haalfi wants to merge 3 commits into
masterfrom
claude/vcrpy-aiohttp-compat-9nm9f0
Open

BK-326: Lift dev-env aiohttp<3.14 cap, floor vcrpy at 8.2#936
haalfi wants to merge 3 commits into
masterfrom
claude/vcrpy-aiohttp-compat-9nm9f0

Conversation

@haalfi

@haalfi haalfi commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Lifts the dev-env aiohttp<3.14 cap and floors vcrpy>=8.2. vcrpy 8.1.1
    subclassed aiohttp.streams.AsyncStreamReaderMixin at import time; aiohttp 3.14
    removed that symbol, so every test crashed at collection on the vcr aiohttp-stub
    import. vcrpy 8.2.0 fixed it (kevin1024/vcrpy#995);
    8.3.0 is current.

  • Diverges from dependabot Chore(deps-dev): Update aiohttp requirement from <3.14,>=3.13 to >=3.13,<3.15 #934, which widened aiohttp alone. The causal
    constraint is vcrpy's version, and vcrpy is a transitive of pytest-recording,
    which floors it at only >=2.0.1 — nothing else in the tree guarantees the
    fixed release. So the floor goes in the dev deps and the aiohttp bound is
    demoted from a compat cap to a minor-version tripwire (<3.15; a major
    guard would be redundant, aiobotocore already caps <4). Ceiling kept rather
    than dropped because the dep is test-only: the cost is one dependabot PR per
    minor, the benefit is that a vcrpy-breaking minor gets a full CI run before it
    can land.

  • Re-measures the two workarounds that rest on 8.1.1 claims, rather than
    deferring them.
    Moving the floor is precisely what restates every "vcrpy
    8.1.1's aiohttp stub …" claim as a claim about 8.2/8.3, so proving them belongs
    to this change. Both were run:

    1. Async-Azure transport shim — still required. Dropping
      AsyncioRequestsTransport from azure_replay_async runs the fixture onto
      AioHttpTransport against the committed cassettes (replay is offline, no
      credentials needed): 114 passed in 8.00s shimmed, hangs after 11 tests
      unshimmed (killed at 600s), faulthandler pinning the block at
      azure/core/pipeline/transport/_aiohttp.py:452 __anext__ under
      download_blobprocess_content_azure.py:440 read. The REC-008
      fidelity caveat stands.
    2. S3 cassettes — still infeasible, but the failure moved. Recording
      S3Backends3fsaiobotocore under vcrpy 8.3.0:
      SMALL WRITE : OK
      SMALL READ  : FAILED  0 read, but total bytes expected is 27
      LARGE WRITE : OK
      LARGE READ  : FAILED  0 read, but total bytes expected is 1048576
      coroutine-never-awaited warnings: 0
      
      Writes now succeed where 8.1.1 died on AioAwsChunkedWrapper with a
      never-awaited coroutine; what remains is a response-side body drop. So the
      request-side defect the BK-181 spike diagnosed is gone, and spec 048's
      "cannot drive the aiobotocore request/response wrappers" is corrected to
      the response bodies specifically. An earlier revision of this PR predicted
      the request-side defect would persist — that was inference, and the run
      disproved it.

    Both results agree that vcrpy 8.2.0 restored import compatibility only and left
    body handling alone.

  • De-stamps seven live version claims. Six justify the Azure shim (four
    fixtures, graph_replay, spec 048) and now cite the 8.3.0 measurement instead
    of hedging. The seventh,
    test_cassettes.py::test_user_agent_absent_stays_absent (REC-005), cites
    vcrpy's filters.replace_headers guard; it escaped the first sweep, which keyed
    on the aiohttp-stub phrasing rather than the version stamp. Guard verified still
    present in 8.3.0. Historical artifacts (sdd/research/*, sdd/traces/*,
    rfc-0010) keep their 8.1.1 stamps by design.

  • Second unanticipated coupling, checked and clear. The BUG-224
    filterwarnings entry anchors on aiohttp's _wait_for_close connector
    qualname, and its guard test is synthetic (never imports aiohttp), so neither a
    green 3.14 matrix nor the guard proves the anchor survives a widened bound. The
    symbol still exists in 3.14.3; the pin comment now carries that re-check duty.

Verification. Both directions of the original break reproduced in a scratch
venv before anything changed: vcrpy 8.1.1 + aiohttp 3.14.3 raises the
AttributeError; 8.3.0 + 3.14.3 imports clean. The hatch env resolves to vcrpy
8.3.0 + aiohttp 3.14.3 and hatch run all passes end to end (exit 0) — 7330
passed, 614 skipped, 2 xfailed, 96% coverage. infra/drift-locks/s3.txt has
pinned aiohttp==3.14.2 green since 2026-07-21: the cap only ever bit the dev
env, never the shipped package, which carries no aiohttp constraint at all.

Known gap. The S3 re-measure ran against a moto endpoint, not real AWS. The
diagnosed failure is client-side in aiobotocore, so the endpoint should not affect
the mechanism, but the BK-181 spike deliberately refuses an emulator endpoint so
its cassettes keep real-AWS fidelity. Recorded as a caveat in the trace rather
than claimed as spike-equivalence; a live-S3 run would close it.

Related issues

Supersedes #934. Upstream: kevin1024/vcrpy#995.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Refactoring
  • CI / Tooling

Checklist

  • Tests pass locally (hatch run test) — full hatch run all, exit 0
  • Linter and formatter pass (hatch run lint)
  • Type checker passes (hatch run typecheck)
  • New/changed behavior is covered by tests — n/a, no behaviour change; the
    dependency change is exercised by the whole suite running against the new
    resolution
  • CHANGELOG.md updated (if user-facing) — not user-facing; infra.test
    audience, which sdd/traces/_schema.yml's derived rule exempts
  • Backlog updated: BK-326 → BACKLOG-DONE.md, trace at
    sdd/traces/bk-326-vcrpy-aiohttp-cap.yml

🤖 Generated with Claude Code

https://claude.ai/code/session_01ApevayvPGhMs7UDYsRJeR6

vcrpy 8.1.1 subclassed aiohttp.streams.AsyncStreamReaderMixin at import time; aiohttp 3.14 removed that symbol, so every test crashed at collection on the vcr aiohttp-stub import. vcrpy 8.2.0 fixed it (kevin1024/vcrpy#995) and 8.3.0 is current. Verified both directions in a scratch venv: 8.1.1 + aiohttp 3.14.3 raises the AttributeError, 8.3.0 + 3.14.3 imports clean; a dry-run resolve of .[dev] with the widened bound picks 3.14.3 + 8.3.0 across 256 packages with no conflicts.

Diverges from dependabot #934, which widened aiohttp alone. The causal constraint is vcrpy's version and vcrpy is a transitive of pytest-recording, which floors it at only >=2.0.1, so vcrpy>=8.2 goes in the dev deps and the aiohttp bound is demoted to a major-version guard (<3.15, dependabot-driven so each minor still gets a full CI run before it lands).

Ripple the ripple-check Dependency row did not anticipate: moving the floor off 8.1.1 staled six live version stamps justifying the async-Azure AsyncioRequestsTransport shim (four Azure fixtures, graph_replay, spec 048). De-stamped here; the substance re-test of both vcrpy aiohttp limitations (Azure shim, BK-181 S3 infeasibility) is split out as BK-327. Historical artifacts (sdd/research, sdd/traces, rfc-0010) keep their 8.1.1 stamps by design.

No CHANGELOG entry: test-only, infra.test audience.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01ApevayvPGhMs7UDYsRJeR6

@haalfi haalfi left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — BK-326 (dev-env aiohttp cap lift, vcrpy floor)

The diagnosis is right and better-evidenced than #934: the causal constraint really is vcrpy's version, pytest-recording's >=2.0.1 floor really does leave it unpinned, and reproducing both directions in a scratch venv before touching anything is the standard this repo asks for. Splitting the substance re-test into BK-327 rather than asserting 8.2+ behaviour from a source read is the correct call, and saying so plainly in the item is what makes it correct. sdd/backlogid.json at 326 matches gen_backlogid.py's derivation (max in BACKLOG-DONE, active IDs excluded), trace validates against _schema.yml, no CHANGELOG needed under the infra.test derived rule. All checks green including test (3.10) and test (3.14).

Four findings, none blocking, ordered by how much they can bite:

  1. pyproject.toml — the new comment calls <3.15 a "major-version guard". It is not; aiohttp's next major is 4 and aiobotocore already caps <4. As written it describes a redundant bound, which is an invitation to delete the pin. The actual rationale (one dependabot PR per minor, so a vcrpy-breaking minor cannot land silently) is in the PR body and BACKLOG-DONE but not in the file that has to survive without them. Same line: the >=3.13 floor's stated cause was AsyncStreamReaderMixin, which the vcrpy floor just retired — it is now unexplained.

  2. pyproject.toml (ripple) — widening past 3.13 also moves the env onto an aiohttp whose connector internals the BUG-224 filterwarnings entry anchors on by qualname (_wait_for_close). The guard test is synthetic (never imports aiohttp), and its own docstring says a green test (3.14) is not proof for a ~50% flake — so nothing here confirms the anchor still resolves on 3.14.3. A one-line check settles it. Neither the Dependency ripple row nor the trace covers this coupling; if it is real it belongs in surprising_ripples.

  3. sdd/BACKLOG-DONE.md — the sweep was six, but tests/backends/fixtures/test_cassettes.py:147 carries a seventh live 8.1.1 stamp (vcrpy's filters.replace_headers internals, justifying a REC-005 byte-identity invariant). It escaped because the sweep keyed on the aiohttp-stub phrasing rather than the version stamp. Either de-stamp it too or narrow the entry's claim — as written it reads exhaustive.

  4. tests/backends/fixtures/azure_live_async.py (+ azure_live_hns_async.py, azure_replay_hns_async.py) — three of the six dropped the version outright, turning a dated checkable claim into an undated one that now implicitly covers 8.3.0, the version the PR is careful to say it has not tested. azure_replay_async.py's "PoC finding on 8.1.1 … BK-327 re-tests it" phrasing already gets this right; applying it to the other three would make the six consistent.

Nothing found against BK-327's framing, the spec 048 edit (backlog-ID citation in a spec has precedent — REC-008 cites BK-303), the trace, the backlog placement, or the decision to keep a ceiling on a test-only dep.


Generated by Claude Code

Comment thread pyproject.toml Outdated
Comment thread pyproject.toml
Comment thread sdd/BACKLOG-DONE.md Outdated
Comment thread tests/backends/fixtures/azure_live_async.py Outdated
claude added 2 commits July 27, 2026 14:18
Four findings, all valid, all fixed.

1. pyproject: the pin comment called <3.15 a major-version guard. It is not; aiohttp's next major is 4 and aiobotocore already caps <4, so as written it described a bound redundant with a transitive constraint. Rewritten as what it actually is, a minor-version tripwire (one dependabot PR per minor, each getting full CI before it can land), and the >=3.13 floor re-justified: AsyncStreamReaderMixin was its cause and the vcrpy floor retired it, so it now only stops a resolve dropping below the oldest minor CI has exercised.

2. pyproject ripple: widening the bound also moves the env onto an aiohttp whose _wait_for_close connector qualname the BUG-224 filterwarnings entry anchors on, and whose guard test is synthetic (never imports aiohttp, so it passes either way). Checked rather than assumed: the symbol still exists in aiohttp 3.14.3, so the suppression is live. Added the re-check duty to the pin comment and a trace step.

3. The de-stamping sweep was six but should have been seven: test_cassettes.py::test_user_agent_absent_stays_absent (REC-005) cites vcrpy's filters.replace_headers guard and escaped because the sweep keyed on the aiohttp-stub phrasing rather than the version stamp. De-stamped, BACKLOG-DONE corrected.

4. Three of the six de-stamps dropped the version outright, turning dated checkable claims into undated ones that silently annex the 8.2/8.3 behaviour this PR never ran. All six now carry azure_replay_async.py's dated form. The seventh stays undated deliberately: its claim is re-asserted by the test body against the installed vcrpy (guard verified still present in 8.3.0), so it goes red rather than stale. Both dispositions recorded in the backlog entry.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01ApevayvPGhMs7UDYsRJeR6
… them

Filing BK-327 was wrong: moving the dev floor off vcrpy 8.1.1 is exactly what restates every '8.1.1's aiohttp stub ...' claim as a claim about 8.2/8.3, so proving those claims belongs to this change, not to a follow-up. BK-327 is deleted and its content folded into BK-326.

Azure shim, still required. Ran azure_replay_async without AsyncioRequestsTransport against the committed cassettes (replay is offline, no credentials): 114 passed in 8s shimmed, hangs after 11 tests unshimmed and killed at 600s, faulthandler pinning the block at azure/core/pipeline/transport/_aiohttp.py __anext__ under download_blob -> process_content. The REC-008 fidelity caveat stands.

S3 cassettes, still infeasible but the failure moved. Recording S3Backend -> s3fs -> aiobotocore under vcrpy 8.3.0 against a moto endpoint (the diagnosed failure is client-side, so the endpoint is irrelevant to the mechanism): writes now succeed where 8.1.1 died on AioAwsChunkedWrapper with a never-awaited coroutine, but every read returns an empty body (0/27 bytes small, 0/1048576 streaming, zero never-awaited warnings). So the request-side defect the BK-181 spike diagnosed is gone and a response-side body drop remains. spec 048's 'cannot drive the request/response wrappers' is corrected to the response bodies specifically; the earlier claim that the request-side defect most likely persists was inference and was wrong.

Both results agree that vcrpy 8.2.0 (kevin1024/vcrpy#995) restored import compatibility only and left body handling alone. The four Azure docstrings now cite the 8.3.0 measurement rather than hedging against an untested version. Caveat recorded in the trace: the S3 re-measure used moto, not the real-AWS endpoint the spike requires for cassette fidelity.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01ApevayvPGhMs7UDYsRJeR6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants