Skip to content

refactor: own the lifespan composition, drop fastapi private import#20

Merged
lesnik512 merged 1 commit into
mainfrom
refactor/own-lifespan-composition
Jun 26, 2026
Merged

refactor: own the lifespan composition, drop fastapi private import#20
lesnik512 merged 1 commit into
mainfrom
refactor/own-lifespan-composition

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Candidate 2 from the architecture review — contain (here, remove) the private-API lifespan coupling. Behavior unchanged.

What

setup_di chained its lifespan via fastapi.routing._merge_lifespan_context — a private FastAPI symbol that could break silently on upgrade, with no test isolating the risk.

Research showed the helper's only real job is merging two ASGI state dicts, and our _lifespan_manager always yields None. So a correct composition collapses to ~4 lines using only the public app.router.lifespan_context:

def _compose_lifespan(original):
    @contextlib.asynccontextmanager
    async def composed(app_):
        async with original(app_) as state, fetch_di_container(app_):
            yield state          # original lifespan's state passes through
    return composed

Original stays the outer context (same startup/shutdown order, same reopen-across-cycles behavior); the container opens inside it.

Wins (review's terms)

  • locality: the FastAPI-internals risk is gone, not just contained
  • the install path no longer depends on a private symbol's stability
  • one characterization test guards the upgrade hazard

Tests

  • New test_setup_di_composes_with_existing_lifespan: an app with a state-yielding lifespan — asserts startup + shutdown both fire, the original's state reaches request.state, and the container opens/closes. Added first as a characterization test (green on the old _merge code), stays green after the refactor — pinning the exact contract the private helper provided. The state-passthrough path was previously untested.

Notes

  • The cast on the composed lifespan mirrors FastAPI's own _merge_lifespan_context (which carries a ty: ignore for the same reason): Lifespan is CM[None] | CM[Mapping] and can't express our CM[Mapping | None].
  • architecture/container-lifecycle.md promoted per the planning convention.

Verification

  • just test-ci100% coverage, 8 passed.
  • just lint-ci → ruff / ty / eof / check-planning clean.

🤖 Generated with Claude Code

setup_di chained its lifespan onto the app via the private
fastapi.routing._merge_lifespan_context — a FastAPI-internals dependency that
could break silently on upgrade, with no test isolating it. Replace it with a
small _compose_lifespan that keeps the original lifespan as the outer context,
opens the container inside it, and passes the original's yielded state through.
This uses only the public app.router.lifespan_context; the simplification is
valid because our nested manager always yields None.

Pin the contract the private helper provided with a characterization test:
setup_di over an app that already has a state-yielding lifespan — its
startup/shutdown both fire, its state reaches requests, and the container
opens/closes. Behavior unchanged; 100% coverage.

Promotes the change into architecture/container-lifecycle.md per the planning
convention.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lesnik512 lesnik512 merged commit 023b059 into main Jun 26, 2026
6 checks passed
@lesnik512 lesnik512 deleted the refactor/own-lifespan-composition branch June 26, 2026 19:32
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.

1 participant