Support context managers#71
Open
martinmkhitaryan wants to merge 1 commit intoNeoteroi:mainfrom
Open
Conversation
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.
Add opt-in context manager management via
manage_contextflagSummary
Adds opt-in support for rodi to enter and exit services that implement the
context manager protocol (
__enter__/__exit__and/or__aenter__/__aexit__).Behavior is controlled by a new
manage_context: bool = Falseparameter on theregistration methods. The default is
False, so all existing behavior ispreserved exactly.
Motivation
The rodi documentation page on
context managers
states the core problem directly:
Today rodi resolves the ambiguity by doing nothing: classes that implement the
context manager protocol are instantiated but never entered or exited. Users
have to wrap every call site themselves with
with/async with, or wireper-request middleware that mirrors what a DI container should already be
doing.
This PR resolves the ambiguity by giving the developer an explicit declaration
of intent.
manage_context=Trueon registration tells rodi "yes, please enterthis on resolve and exit it when the owning scope ends." The default
(
manage_context=False) preserves today's behavior verbatim, so no existingapp is affected.
A concrete case where this matters is SQLAlchemy's async session:
The same pattern applies to httpx clients, file handles, locks, tracing
spans, and anything else that wants entry on borrow / exit on return.
API
Behavior matrix
provider.create_scope()TypeErrorprovider.create_async_scope()scope end.
scope end (LIFO).
InvalidContextManagerRegistration(rationale below).Why singleton is not supported (yet)
Singletons live for the lifetime of the
Servicesprovider, but rodi has noexplicit teardown hook for the provider/container today. Supporting
manage_context=Truefor singletons would require introducing one (e.g.Services.dispose()or makingContaineritself a context manager), which isits own design discussion (atexit semantics, shutdown ordering, error
handling).
To keep this PR focused and reviewable, singleton support is deferred to a
follow-up issue. The current behavior is to raise
InvalidContextManagerRegistrationat registration with a clear messagepointing the user to
SCOPED/TRANSIENTor to manual management.Implementation notes
ManagedScopedProviderandManagedTransientProviderwrap any existinginner provider (
TypeProvider,ArgsTypeProvider,FactoryTypeProvider).This keeps the existing 6 provider classes completely untouched, so users
who don't opt in pay zero overhead on the hot resolve path.
ActivationScopegained a lazy_exit_stack: ExitStack | None. Drainedbefore
dispose()in__exit__, in LIFO order. Standardswap-and-call idiom for single-shot consumption.
AsyncActivationScopeis a new subclass that adds a lazy_async_exit_stack: AsyncExitStack | None. Resolution stays synchronous —agetflips an_async_modeflag for the duration ofServices.get,causing
_enter_contextto defer instances into a_pending_aenterlist. Once sync resolution returns,
agetdrains the list usingenter_async_context(for async CMs) orenter_context(for sync CMs)on the same
AsyncExitStack. Syncgetcalls on the same async scopealso route into that unified stack via an overridden
_register_sync_context, so cross-protocol exit order across mixedget+agetcalls is strictly LIFO regardless of which entry pointadded the instance.
AsyncContainerProtocolis a separateProtocoldeclaringaresolve.Containerimplements bothContainerProtocolandAsyncContainerProtocol. The existingContainerProtocolis unchanged,so sync-only third-party DI containers remain compatible. Frameworks that
want async resolution opt into it structurally by typing against
AsyncContainerProtocol(or againstContainerdirectly).Container.aresolveis a thin wrapper aroundscope.agetthat validatesthe scope type up front, so misuse fails with a clear
TypeErrorinsteadof silently leaking managed CMs into an un-awaited throwaway scope.
TrackingActivationScope.__exit__now callssuper().__exit__(...)insteadof
self.dispose()directly, so the new exit-stack draining runs for userswho opt into nested-scope tracking. Pre-existing tests confirm dispose
semantics are unchanged.
_enter_contextrather than at registration. Class-level checks atregistration are an easy follow-up; deferred because the factory path can't
reliably introspect the type the factory will return.
Backward compatibility
manage_contextdefaults toFalseeverywhere. All existing call sites,fixtures, and tests work without changes. No public symbol was renamed or
removed. New public symbols added:
AsyncActivationScopeAsyncContainerProtocolContainer.aresolveInvalidContextManagerRegistrationManagedScopedProvider,ManagedTransientProvider(intended internal butexposed for parity with the other provider classes already in the public
module surface)
Services.create_async_scope,Services.agetFollow-up (not in this PR)
A separate issue/PR will discuss singleton +
manage_context=True. The shapeof that work — provider-level dispose, atexit semantics, container-as-context-
manager — is large enough that bundling it here would make this PR much harder
to review.