fix: importing the FastAPI adapter without its extra names the extra, not starlette - #54
Merged
Merged
Conversation
… not starlette In a bare install, `import servicewright.adapters.fastapi` raised `ModuleNotFoundError: No module named 'starlette'` — a package the user never asked for and that appears in no extras table — while litestar, grpc, dishka and settings all named theirs. `adapters/fastapi/_imports.py` guards the stack and says in its own docstring that every HTTP submodule imports its third-party symbols from there, but eight module-level imports had drifted out of it: starlette in three middlewares, fastapi in `headers` and `unit_scope`, pydantic in `schemas`, and fastapi plus deadline_budget in `exceptions`. Whichever ran first was the message, and the first one reached is `middlewares/context.py`. Route all of them back through the guard, so it is the single door structurally rather than by import order, and add a test per extra-gated subpackage that imports it in a subprocess with everything the extra installs blocked by a `sys.meta_path` finder, asserting an `ImportError` that names the extra.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #51
The problem
In a bare
pip install servicewright, three of the four adapter subpackages told you what to install and the fourth named a package you never asked for:starletteis in no extras table, and nothing maps it back toservicewright[fastapi].docs/agents.mdpromises the opposite in two places: the adapters section ("importing one without it raisesImportErrornaming what to install") and the errors table ("the message names the extra").The cause
adapters/fastapi/_imports.pyguards the stack correctly and states in its own docstring that "every HTTP submodule imports its third-party symbols from here". Eight module-level imports had drifted out of it:middlewares/context.pyfrom starlette.datastructures import Headersmiddlewares/logging.pyfrom starlette.datastructures import Headersmiddlewares/processing_time.pyfrom starlette.datastructures import MutableHeadersschemas.pyfrom pydantic import BaseModel, ConfigDict, Fieldheaders.pyfrom fastapi import Headerunit_scope.pyfrom fastapi import Depends, Requestexceptions.pyfrom deadline_budget import DeadlineExceededErrorexceptions.pyfrom fastapi.exceptions import RequestValidationErrorWhichever runs first is the message the user sees, and the first one the package reaches is
middlewares/context.py. I parsed every extra-gated adapter the same way:litestar,grpc,dishka,settings,apscheduler3andapscheduler4have no runtime third-party import outside their guard, which is exactly why their messages were right.The fix
Route all eight back through
_imports.py, and add the seven names they need to its__all__. The guard is now the single door structurally, not by import order.TYPE_CHECKING-only imports keep importing fromstarlettedirectly — they never run — and the deferredstarlette.requests.RequestinsideContextMiddleware.__call__goes through the guard too (fastapi.Requestis the same class object).No public name changes:
_importsis private and every subpackage__all__is untouched, sodocs/agents.mdneeds no edit. The docs already describe the behaviour this PR makes true —docs/operations/runbooks.mdeven says "the message always names the extra".What I rejected
starletteto the guard'stry:block. That is the symptom.pydanticanddeadline_budgetwere unguarded too, and the next import that drifts out re-opens the same hole.._importsfirst inadapters/fastapi/__init__.py. One line, and it would have worked — Python runs the package__init__before any submodule, so every import of the subpackage passes through it. But it is a fix that lives in the order of an import block: one reordering, one__init__refactor, and it is silently gone, with the docstring's claim still false. (I said in my plan comment that a direct submodule import would defeat it — that was wrong, and it is why I checked before writing the test rather than after.)require_extra(...)helper across all adapters. A new concept for every reader, replacing a per-adapter_imports.pypattern that already works in six of seven adapters. The one that drifted needs discipline, not a framework.The test
tests/unit/test_adapter_extras.py: one case per extra-gated subpackage (fastapi,litestar,grpc,apscheduler4,apscheduler3,dishka,settings). The dev environment installs every extra, so absence is simulated honestly — a subprocess with asys.meta_pathfinder that refuses everything the extra puts on the path, then the import, then an assertion that the exception is anImportErrorand not the bareModuleNotFoundError, and that the message namesservicewright[<extra>].On master it fails for
fastapionly:Verification
make checkclean,make test→737 passed, 14 skipped, coverage 93.00% (730 passed before).The reporter's scenario, re-run against this branch installed with no extras into the venv that produced the report:
Nothing changes for anyone who has the extra installed: the symbols are the same objects, reached one module earlier.
On
servicewright.settingsNot a bug, and not fixed here: that module has never existed. The models live at
servicewright.adapters.settings— which is whatdocs/agents.md, the settings guide and the API reference all point at — and it already raised the right thing before this PR:It is now covered by the same test under its real name.