Increase granularity of context.
Currently, container_context() resets/reinitializes the whole context. This makes it hard to manage resources with different lifetimes.
As a theoretical workaround, it is possible to build an initial_context that keeps resources in the scope, but that requires using context internals (starting with _).
Proposal:
- Allow passing providers to
container_context:
async with container_context(container.provider1, container.provider2, ...):
# resets the context for listed providers.
- Allow passing containers to
container_context:
async with container_context(container1, container2, ...):
# resets the context for context-resources in given containers
Implementation details:
- Make each
ContextResourcemanage its own context(var) on a class level. -
container_contextnow just interacts with these context(var)s to set and reset.
Goals:
- Maintain compatibility with the current solution:
async with container_context(): # will still reset all contexts across all containers
- Maintain current functionality with
initial_context
@lesnik512 If you approve of this proposal, I will start working on it. I also saw that you make active use of setting the initial_context, if you could provide a functional example I will make sure to test against it when converting functionality.
@alexanderlazarev0 seems promising!
Make each ContextResource manage its own context(var) on a class level.
Do you mean that in init method of ContextResource will be created context var and context stack will be stored in it?
I approve. And I will provide an example of initial context usage
Do you mean that in init method of ContextResource will be created context var and context stack will be stored in it?
@lesnik512 Yes (this was a mistake of mine in the post), since each ContextResource instance would need to manage its own context. What is should have said is at instance level.
@alexanderlazarev0 this could work, thank you for clarification
@alexanderlazarev0 I thought about separate context vars.
Do we really need this? I think we can achieve described behaviour with single contextvar
@alexanderlazarev0 about example of initial context usage: this example is quite good https://github.com/modern-python/that-depends/blob/main/tests/integrations/test_fastapi_di_pass_request.py
@lesnik512
Do we really need this? I think we can achieve described behaviour with single contextvar
Currently, all context resources are managed using one context-variable that stores a dictionary. In practice, this dictionary has a shared namespace (dict-keys) for both:
- explicitly set context-resources (as in the example you provided in the previous comment):
async with container_context({"my_custom_key" : some_object }): ...
... that can then be retrieved anywhere within that context using:
some_object = fetch_context_item("my_custom_key")
- context-resources that are created by
ContextResourceproviders, whose data is also stored in the same dict but contains auuidin the key to avoid collision. These keys also make these resources hard to fetch usingfetch_context_itemand hard to set a pre-determined value for usinginitial_context, since retrieving the key requires some kind of "workaround".
Also it is important to note that entering a new container context using:
async with container_context()
Resets our dict completely without preserving any user set context-resources and provided context-resources.
From this I think we have a clear separation of context-resources which I think we should handle differently.
- We continue handling user set context-resources in a "global" dict, however, we introduce an additional argument into
container_context:
async with container_context(preserve_globals=true): ...
Where preserve_globals (we can rework the name) is an optional kwarg that has true (? not sure whats best here) default. This will make it such that entering a new container_context does not reset the dictionary and preserves objects that were set with initial_context or otherwise.
- We start handling
ContextResourceprovided resources usingContextResourceinstance levelcontextvarswhich are specific to that resource. This allows us to create a clear separation between these resources and separate management of these resources to another location. Yes, in theory everything still could be managed in one dict, but I think that will become more difficult to manage in the future.
Finally, the API would look something like this:
@container_context() # resets all context-providers, but global context is preserved
async foo(resource = Provide[MyContainer.context_resource]): ...
@container_context(initial_context=my_custom_context) # set new global context to my_custom_context and reset all providers.
async foo(resource = Provide[MyContainer.context_resource]): ...
@container_context(preserve_globals=false) # completely clean context
async foo(resource = Provide[MyContainer.context_resource]): ...
@container_context(providers=[MyContainer.context_resource]) # reset only the given provider, keep global context.
async foo(resource = Provide[MyContainer.context_resource]): ...
@container_context(containers=[MyContainer]) # reset all context-resources in given container, keep global context.
async foo(resource = Provide[MyContainer.context_resource]): ...
@container_context(providers=[MyContainer.context_resource], preserve_globals=false) # reset given provider and global context.
async foo(resource = Provide[MyContainer.context_resource]): ...
And (for completeness):
- Resetting
globalcontext always works whether usingasync with container_context()orwith_container_context(). - Using
async with container_context()allows resetting all providers and containers (since we can call sync from async). - Using
with container_context(equivalent to wrapping a sync function with@container_context) allows only to reset sync providers (specifying async provider will throw an error), and if a container is specified, then only its syncContextResourceproviders will be reset, async providers will keep their state. This is not an issue since once a sync container_context is entered, resolution of async dependencies is no longer possible (you are either already in a sync function OR you wrote the following code:
async def foo():
with container_context(containers=[MyContainer]):
await MyContainer.async_resource.async_resolve() # Error: Cannot resolve async resource in sync container_context()
So we should specifically watch out for this case (and also mark it bad practice to enter with container_context() inside an async function.
Please let me know what you think)
@alexanderlazarev0 I agree that using separate contextvars can be easier to manage, but I have several concerns/questions:
- how without passing providers and containers to container_context to tear down all context resources? Do you want to to use subclasses() method of BaseContainer?
- where we will store initial context? Will it be separate contextvar?
- I'm concerned about performance degradation. What do you think, will it significantly affect performance?
@lesnik512
how without passing providers and containers to container_context to tear down all context resources? Do you want to to use subclasses() method of BaseContainer?
My initial approach would be to modify the BaseContainer or its meta to "register" all containers in a class variable, likely also a contextvar.
where we will store initial context? Will it be separate contextvar?
Yes initial_context and all the variables which have user set key can go in a separate contextvar
I'm concerned about performance degradation. What do you think, will it significantly affect performance? I see two reasonable approaches:
- Have two
contextvars: one for user-set resources, and one for providers
- fast reinit of context when all providers need to be reset, just
contextvar.set({}} - Each time I want to reinit the context of only 1 provider, I still need to copy the rest of the "context-dict" to the new context
O(numOfContextProviders*averageContextStackDepth) space complexity. - Each time the context of a whole container needs to be reset I still have to traverse all the providers, get their keys and then remove them from context.
- Have one
contextvarfor user-set resources, and have all providers manage their owncontextvars
- need to traverse all providers when fully resetting context and performance an
isinstancecheck to then reinit their contextO(numberOfProviders) time complexity. - Each time I want to reinit the context of only 1 provider, I don't need to copy the context of the other providers along with it.
- Resetting on container level is exactly the same.
So there is a trade of time-complexity and space-complexity.
@alexanderlazarev0 ok, let's make separate contextvars then