From 07bc542478e10d20305631a94b6f1e447897ee6c Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sun, 19 Jul 2026 23:00:28 -0500 Subject: [PATCH] Remove the dead vcrpy/aiohttp compat shim (closes #2140) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit aiohttp 3.14 removed `aiohttp.streams.AsyncStreamReaderMixin`, which vcrpy 8.1.1's `vcr/stubs/aiohttp_stubs.py` subclassed at import time. vcrpy loads that stub lazily when a cassette is entered, so every VCR cassette entry raised AttributeError (issue #1920). The workaround was `ensure_aiohttp_vcr_compat()`, which re-injected the removed name as an empty class. vcrpy 8.2.0 fixed the stub upstream (kevin1024/vcrpy#996): `MockStream` now inherits only from `asyncio.StreamReader` and handles the 3.14 `stream_writer` argument itself. The pin has been at 8.2.1 or later since then, so the shim has been redundant — and not merely inert. Under aiohttp 3.14 the `hasattr` guard fell through, so conftest import was still writing a fake `AsyncStreamReaderMixin` into the real `aiohttp.streams` namespace on every test run. Verified in two environments, in both cases entering a cassette with no shim applied and confirming `MockStream.__bases__ == (asyncio.StreamReader,)`: - the local django image: vcrpy 8.2.1 + aiohttp 3.14.1 (the exact pre-bump pairing the issue asks about) - a clean venv: vcrpy 8.3.0 + aiohttp 3.14.1 (the current pin) Removed: - `ensure_aiohttp_vcr_compat()` in `opencontractserver/utils/vcr_replay.py` and its call in `maybe_vcr_cassette()` - the import and module-level call in `conftest.py` - `EnsureAiohttpVcrCompatTests` in `opencontractserver/tests/test_vcr_replay.py` - the `requirements/local.txt` comment, which still described vcrpy 8.1.1 as the pinned version and #996 as unreleased; replaced with a short note on why the floor is 8.2.0 The `aiohttp>=3.13,<3.14` cap this was paired with (issue #1914) was already lifted on main. `EnsureAiohttpVcrCompatTests` is replaced by `VcrCassetteEntryTests`, which keeps the part of the guard that survives the shim: entering a cassette forces the lazy import of `vcr/stubs/aiohttp_stubs.py`, so the test fails if a future vcrpy/aiohttp pairing reintroduces an import-time incompatibility. 72 tests pass across `test_vcr_replay.py`, `test_structured_response_simple.py`, `test_structured_response_api.py`, and `test_individual_extract_tasks.py`. --- changelog.d/2140-vcr-aiohttp-shim.removed.md | 14 ++++ conftest.py | 11 --- opencontractserver/tests/test_vcr_replay.py | 77 ++++++-------------- opencontractserver/utils/vcr_replay.py | 51 ------------- requirements/local.txt | 13 ++-- 5 files changed, 41 insertions(+), 125 deletions(-) create mode 100644 changelog.d/2140-vcr-aiohttp-shim.removed.md diff --git a/changelog.d/2140-vcr-aiohttp-shim.removed.md b/changelog.d/2140-vcr-aiohttp-shim.removed.md new file mode 100644 index 0000000000..ab926df9ac --- /dev/null +++ b/changelog.d/2140-vcr-aiohttp-shim.removed.md @@ -0,0 +1,14 @@ +- Removed the now-dead `ensure_aiohttp_vcr_compat()` aiohttp/vcrpy compat shim + (`opencontractserver/utils/vcr_replay.py`) and its call sites in `conftest.py` + and `maybe_vcr_cassette()`. The shim restored + `aiohttp.streams.AsyncStreamReaderMixin`, a symbol aiohttp 3.14 removed and + vcrpy 8.1.1's aiohttp stub subclassed at import time (issue #1920). vcrpy + 8.2.0 fixed the stub upstream (kevin1024/vcrpy#996) — `MockStream` no longer + inherits the mixin — so the shim has been a no-op since the pin moved to + 8.2.1. Verified empirically against `vcrpy==8.3.0` + `aiohttp==3.14.1`: + entering a cassette with no shim applied succeeds. The `aiohttp>=3.13,<3.14` + cap this was paired with had already been lifted. The + `EnsureAiohttpVcrCompatTests` shim unit tests are replaced by + `VcrCassetteEntryTests` in `opencontractserver/tests/test_vcr_replay.py`, + which keeps the meaningful regression guard: entering a cassette (and thus + lazily importing `vcr/stubs/aiohttp_stubs.py`) must not raise. Closes #2140. diff --git a/conftest.py b/conftest.py index 7cd4698180..2ea97ec3a5 100644 --- a/conftest.py +++ b/conftest.py @@ -11,17 +11,6 @@ import pytest from django import db -from opencontractserver.utils.vcr_replay import ensure_aiohttp_vcr_compat - -# Many integration tests record/replay VCR cassettes. vcrpy 8.1.1 imports its -# aiohttp stub lazily when a cassette is entered, and that stub subclasses -# ``aiohttp.streams.AsyncStreamReaderMixin`` — a symbol aiohttp 3.14 removed — at -# module-evaluation time. A fresh CI resolution that picks up aiohttp >= 3.14 -# would therefore make every VCR-using test raise AttributeError. Apply the -# compat shim here, at conftest import, so the symbol exists before any test -# runs. See opencontractserver/utils/vcr_replay.py and issue #1920. -ensure_aiohttp_vcr_compat() - @pytest.fixture(scope="session", autouse=True) def make_create_permissions_xdist_safe(): diff --git a/opencontractserver/tests/test_vcr_replay.py b/opencontractserver/tests/test_vcr_replay.py index 50ccef9ba6..3b1c30d607 100644 --- a/opencontractserver/tests/test_vcr_replay.py +++ b/opencontractserver/tests/test_vcr_replay.py @@ -34,7 +34,6 @@ _VOLATILE_PATTERNS, _match_llm_body, _normalize_body, - ensure_aiohttp_vcr_compat, maybe_vcr_cassette, ) @@ -323,70 +322,38 @@ def test_replay_mode_yields_active_cassette_when_file_missing(self): self.assertIsNotNone(ctx) -class EnsureAiohttpVcrCompatTests(TestCase): - """Regression for issue #1920 / kevin1024/vcrpy#995. +class VcrCassetteEntryTests(TestCase): + """Guard the vcrpy <-> aiohttp import contract (issue #1920, #2140). - aiohttp 3.14 removed ``aiohttp.streams.AsyncStreamReaderMixin``, which the - pinned vcrpy 8.1.1 aiohttp stub subclasses at module-evaluation time. vcrpy - imports that stub lazily when a cassette is entered, so under aiohttp >= 3.14 - every VCR cassette entry raises ``AttributeError``. The compat shim restores - the symbol so cassette entry works again. - """ + Entering a cassette makes ``vcr/patch.py`` build its patchers, which imports + ``vcr/stubs/aiohttp_stubs.py`` — a module vcrpy evaluates lazily and that + this codebase never otherwise touches (we only record/replay *httpx* LLM + traffic; aiohttp is merely importable, transitively via llama-index-core). + That lazy import is exactly where vcrpy 8.1.1 blew up under aiohttp 3.14, + which had removed ``aiohttp.streams.AsyncStreamReaderMixin``: every cassette + entry raised ``AttributeError`` and aborted whole test runs. - def test_mixin_symbol_present_after_shim(self): - try: - from aiohttp import streams - except ModuleNotFoundError: # pragma: no cover - aiohttp always present - self.skipTest("aiohttp not installed") - - had_real_symbol = hasattr(streams, "AsyncStreamReaderMixin") - original = getattr(streams, "AsyncStreamReaderMixin", None) - - # Keep the test self-contained: restore the module to whatever state it - # was in before this test ran (the shim may add the symbol below). - def _restore() -> None: - if had_real_symbol: - setattr(streams, "AsyncStreamReaderMixin", original) - elif hasattr(streams, "AsyncStreamReaderMixin"): - delattr(streams, "AsyncStreamReaderMixin") - - self.addCleanup(_restore) - - ensure_aiohttp_vcr_compat() - - # Whichever aiohttp is installed, the symbol must exist afterward so - # vcrpy's MockStream class statement can evaluate. - self.assertTrue(hasattr(streams, "AsyncStreamReaderMixin")) - if had_real_symbol: - # Under aiohttp < 3.14 the real mixin must be left untouched. - self.assertIs(getattr(streams, "AsyncStreamReaderMixin"), original) - - def test_shim_is_idempotent(self): - # Called twice (conftest + maybe_vcr_cassette both invoke it) must not - # raise or swap the symbol out from under a prior call. - ensure_aiohttp_vcr_compat() - try: - from aiohttp import streams - except ModuleNotFoundError: # pragma: no cover - aiohttp always present - self.skipTest("aiohttp not installed") - first = getattr(streams, "AsyncStreamReaderMixin") - ensure_aiohttp_vcr_compat() - self.assertIs(getattr(streams, "AsyncStreamReaderMixin"), first) + vcrpy 8.2.0 fixed the stub upstream (kevin1024/vcrpy#996), so the local + compat shim was deleted in #2140. This test is what remains of that + regression guard: it fails if a future vcrpy/aiohttp pair reintroduces an + import-time incompatibility. ``import vcr`` alone does NOT trigger the stub, + which is why this enters a cassette rather than just importing. + """ def test_vcr_cassette_entry_works(self): - # The real contract: vcrpy imports its aiohttp stub lazily when a - # cassette is entered (vcr/patch.py builds its patchers). Under aiohttp - # >= 3.14 that stub subclasses the removed AsyncStreamReaderMixin and - # raises AttributeError. With the shim applied, entering an (empty) - # cassette must succeed. (`import vcr` alone does NOT trigger the stub, - # which is why this enters a cassette rather than just importing.) - ensure_aiohttp_vcr_compat() import vcr with tempfile.TemporaryDirectory() as td: with vcr.VCR().use_cassette(os.path.join(td, "regression.yaml")): pass + def test_aiohttp_stub_is_importable(self): + # Pin the specific failure mode directly: the stub module must evaluate + # against the installed aiohttp without any shim in place. + import vcr.stubs.aiohttp_stubs as aiohttp_stubs + + self.assertTrue(hasattr(aiohttp_stubs, "MockStream")) + class LlmHostsTests(TestCase): """Sanity check for the host allowlist.""" diff --git a/opencontractserver/utils/vcr_replay.py b/opencontractserver/utils/vcr_replay.py index 9ac49c5460..3068dd70c6 100644 --- a/opencontractserver/utils/vcr_replay.py +++ b/opencontractserver/utils/vcr_replay.py @@ -42,52 +42,6 @@ logger = logging.getLogger(__name__) -def ensure_aiohttp_vcr_compat() -> None: - """Make vcrpy 8.1.1's aiohttp stub importable under aiohttp >= 3.14. - - vcrpy 8.1.1 (the latest release) defines - ``class MockStream(asyncio.StreamReader, streams.AsyncStreamReaderMixin)`` - in ``vcr/stubs/aiohttp_stubs.py``. aiohttp 3.14 removed - ``aiohttp.streams.AsyncStreamReaderMixin`` (its helpers were folded onto the - stream classes). vcrpy imports that stub lazily — when a cassette is entered - and ``vcr/patch.py`` builds its patchers — so under aiohttp >= 3.14 every VCR - cassette entry raises ``AttributeError`` (issue #1920; upstream - kevin1024/vcrpy#995, fix proposed in the unreleased PR #996). - - This codebase only records/replays *httpx* (LLM provider) cassettes — the - aiohttp stub is pulled in incidentally because aiohttp is importable - (transitive via llama-index-core). ``MockStream`` is therefore never - instantiated here, so restoring the removed name as an empty mixin is enough - to let the stub's class body evaluate; none of the mixin's (now-removed) - helper methods are exercised. - - Idempotent, and a no-op under aiohttp < 3.14 (where the real mixin is still - present — it is left untouched). Delete this shim, and bump ``vcrpy`` in - requirements/local.txt, once vcrpy ships a release that supports the aiohttp - 3.14 stream API. - """ - try: - from aiohttp import streams - except ModuleNotFoundError: - # aiohttp isn't installed → vcrpy won't load its aiohttp stub anyway. - return - - if hasattr(streams, "AsyncStreamReaderMixin"): - return - - class AsyncStreamReaderMixin: - """Empty stand-in for the base class aiohttp 3.14 removed. - - Only needs to exist so vcrpy's ``MockStream`` class statement can - evaluate; its methods are never called (we never replay aiohttp - cassettes). - """ - - # setattr (string name) rather than attribute assignment so mypy does not - # flag a member aiohttp 3.14 removed from the ``streams`` module. - setattr(streams, "AsyncStreamReaderMixin", AsyncStreamReaderMixin) - - # These hostnames are the LLM provider endpoints VCR should intercept. # Other hosts (LlamaParse, embedder microservice, S3) bypass VCR. _LLM_HOSTS = {"api.openai.com", "api.anthropic.com"} @@ -248,11 +202,6 @@ def maybe_vcr_cassette() -> Iterator[object | None]: yield None return - # vcrpy 8.1.1's aiohttp stub references a symbol aiohttp 3.14 removed; - # restore it before importing vcr so the live E2E record/replay harness - # (which runs outside pytest, where conftest isn't loaded) also works under - # aiohttp >= 3.14. See ensure_aiohttp_vcr_compat above and issue #1920. - ensure_aiohttp_vcr_compat() import vcr # local import keeps prod paths free of vcr cost cassette_dir = os.path.dirname(os.path.abspath(cassette_path)) diff --git a/requirements/local.txt b/requirements/local.txt index 58f6a8217d..d363692dd9 100644 --- a/requirements/local.txt +++ b/requirements/local.txt @@ -14,14 +14,11 @@ pytest-xdist==3.8.0 # https://github.com/pytest-dev/pytest-xdist (parallel test pytest-timeout==2.4.0 # https://github.com/pytest-dev/pytest-timeout (per-test hang guard in CI) djangorestframework-stubs==3.17.0 # https://github.com/typeddjango/djangorestframework-stubs responses==0.26.2 # https://github.com/getsentry/responses -# vcrpy 8.1.1 is the latest release and is NOT yet compatible with aiohttp -# >=3.14: its aiohttp stub subclasses aiohttp.streams.AsyncStreamReaderMixin at -# import time, a symbol aiohttp 3.14 removed (kevin1024/vcrpy#995; fix proposed -# in the unreleased PR #996). aiohttp is an unpinned transitive dep, so a fresh -# CI resolution picks up 3.14+. ensure_aiohttp_vcr_compat() in -# opencontractserver/utils/vcr_replay.py restores that symbol so vcrpy works -# under aiohttp >=3.14. Bump vcrpy here and delete the shim once a release ships -# the vcrpy#996 fix. See issue #1920. +# Keep at >=8.2.0. Earlier releases' aiohttp stub subclasses +# aiohttp.streams.AsyncStreamReaderMixin at import time, a symbol aiohttp 3.14 +# removed, which breaks every VCR cassette entry (issues #1920 / #2140, +# kevin1024/vcrpy#995, fixed upstream in #996). aiohttp is an unpinned +# transitive dep, so a fresh CI resolution picks up 3.14+. vcrpy==8.3.0 # Profiling