ivankorobkov / python-inject

Python dependency injection
Apache License 2.0
694 stars 79 forks source link

Providing container via contextvar or similar mechanism #100

Open etsvetanov opened 5 months ago

etsvetanov commented 5 months ago

@ivankorobkov The README states:

## Why no scopes?
I've used Guice and Spring in Java for a lot of years, and I don't like their scopes.
`python-inject` by default creates objects as singletons. It does not need a prototype scope
as in Spring or NO_SCOPE as in Guice because `python-inject` does not steal your class 
constructors. Create instances the way you like and then inject dependencies into them.

Other scopes such as a request scope or a session scope are fragile, introduce high coupling,
and are difficult to test. In `python-inject` write custom providers which can be thread-local, 
request-local, etc.

Would using a contextvar (or a similar mechanism) to "provide" the Injector be in a conflict with the above or other philosophy in this package? I think it would leave the default usage of the package the same but provide a way to "swap" the container. Our specific use case is in API testing (Fastapi), where we would like to have one container for the application code (run via an API client) and another container for tests that test or use some of the application code infrastructure directly. Currently we'd do by either configuring the container with an elaborate bind_to_provider functions that detect what needs to be injected in runtime and sometimes reset the whole container after each test. I can work on a PR if you consider this reasonable.

ivankorobkov commented 5 months ago

Hi!

Could you give some code examples how you want to implement this feature and how it is going to work?

Thanks

etsvetanov commented 5 months ago

Hi!

I would replace the _INJECTOR = None with INJECTOR: ContextVar[Injector | None] = None and this context variable would be exposed as a public member of the package.

Then I would replace

def get_injector() -> Optional[Injector]:
    """Return the current injector or None."""
    return _INJECTOR

with something along these lines:

def get_injector() -> Optional[Injector]:
    return INJECTOR.get()

maybe provide utility that sets/resets the injector (maybe expose only this utility and keep the context var private)

@contextmanager
def configure_injector(config: Optional[BinderCallable]):  # this can be expanded to allow inheritance of the current config
    injector = Injector(config, ...)
    token = INJECTOR.set(injector)

    try:
        yield
    finally:
        INJECTOR.reset(token)

and it could be used to create a new container for some portion of the call stack:

with configure_injector(some_config):
    ...

Let me know if this makes sense, I haven't fully thought it through.

In theory, some portion of the call stack will share a container but it won't allow for different containers on the same portion of the call stack.

ivankorobkov commented 5 months ago

Looks good to me.

etsvetanov commented 5 months ago

Great, I can work on a PR and we can discuss the details and if thе actual implementation makes sense.

etsvetanov commented 4 months ago

I've been thinking on how to keep the current behavior and make these changes backwards compatible and just swapping the _INJECTOR to use a ContextVar can introduce subtle breaking changes, e.g. if inject.configure() is called in async context and then in a different context or non-async context it is expected to keep the same configuration - the context var storing the Injector will not act is pure global variable - probably not many people are relying on this behavior but it is a breaking change nonetheless.

I'm going to think more about different solutions but one would be to keep the current global variable, introduce a separate one for ContextVar and extend the API for inject.configure (or maybe introduce a new API?) to allow users to opt-in into contextvar behavior?

@ivankorobkov what do you think?

ivankorobkov commented 4 months ago

Well, good thoughts. I totally agree about backwards compatibility.

Personally, I think I would make an execution mode. Something link this (pseudocode):

inject.configure(mode=ContextMode)
inject.configure(mode=SingletonMode)

And maybe two different classes with the same interface, ie.

_INJECTOR = _SingletonHolder()
_INJECTOR = _ContextHolder()
etsvetanov commented 3 months ago

Sorry about the long pause, had no free time to look into this. How do you feel about an environment variable which controls the "execution mode"?

ivankorobkov commented 3 months ago

@etsvetanov

Hi! Env var seems like a bag choice for me. It is more of a concrete application configuration, not a programmatic API.

Well, do you actually need this functionality at all?