modern-python / that-depends

Simple DI-framework with IOC-container inspired by python-dependency-injector
https://that-depends.readthedocs.io/
MIT License
160 stars 12 forks source link

feature request: Resource but for callables that return context manager #79

Closed zerlok closed 1 week ago

zerlok commented 2 months ago

Resource provider expects callable object that returns either Iterable or AsyncIterable that yields only one element (the result value for provider attribute in DI container).

I think it would be great to support python context managers too.

import typing

T = typing.TypeVar("T")

async def create_async_cm_resource() -> typing.AsyncContextManager[T]:
    ...

class DIContainer(BaseContainer):
    async_resource = providers.Resource(create_async_resource)

async def main() -> None
    await DIContainer.init_resources()  # CM instance got from `create_async_cm_resource` and it's `__aenter__` invoked
    cm_result = await DIContainer.async_resource()  # result is T
    ...
    await DIContainer.tear_down()  # CM instance `__aexit__` invoked

P.S. I'm ready to make a PR

alexanderlazarev0 commented 2 months ago

I think this is a good idea, but here you are suggesting that providers.Resource accepts a Callable[P, AsyncContextManager[T]], why not just AsyncContextManager[T] directly?

lesnik512 commented 2 months ago

I agree with @alexanderlazarev0 , accepting context managers directly seems better.

@zerlok you are welcome to try, thank you for the proposal

zerlok commented 1 week ago

@lesnik512 , @alexanderlazarev0 , if resource can accept Callable[P, AsyncContextManager[T]] , then client can use async factory methods, without extra code.

For example:

class MySender:
    @asynccontextmanager
    @classmethod
    async def create(cls, conn: AMQPConnection) -> typing.AsyncIterator[MyPublisher]:
        exchange = await conn.declare_exchange(...)
        try:
            yield cls(exchange)
        finally:
            await conn.delete_exchange(exchange)

    def __init__(self, exchange: AMQPExchange) -> None:
        self.__exchange = exchange

    async def send(self, msg: bytes) -> None:
        await self.__exchange.publish_message(msg)
class Container(BaseContainer):
    conn = providers.Resource(_connect_amqp)
    my_publisher = providers.Resource(MySender.create, conn=conn.cast)

For now it has to be done with extra code:

async def _init_my_sender(conn: AMQPConnection) -> AsyncIterator[MySender]:
    async with MySender.create(conn) as my_sender:
        yield my_sender

class Container(BaseContainer):
    conn = providers.Resource(_connect_amqp)
    my_publisher = providers.Resource(_init_my_sender, conn=conn.cast)

I think the best solution is to support both types: simple async context manager & callable that returns async context manager.

lesnik512 commented 1 week ago

I think, that we don't need to accept iterators wrapped in contextmanager and asynccontextmanager decorators. Only inherited from ContextManager and AsyncContextManager.

Making some experiments now. Will make PR to show.

alexanderlazarev0 commented 1 week ago

@lesnik512 I don't believe this functionality was necessary in the first place, since the above example would work by simply not wrapping the create method with a asyncontextmanager. From what I can understand @zerlok just wanted to use methods that were wrapped as providers, not sure if making one implement ContextManager or AsyncContextManager help...

lesnik512 commented 1 week ago

@alexanderlazarev0 I agree about wrapped context managers. But about inherited context managers, I think it can be useful. So I'm not planning to release it yet before making some changes and discussing them through PR

alexanderlazarev0 commented 1 week ago

@lesnik512 Yes I agree it can be useful, was just mentioning that it might not resolve this issue :)

lesnik512 commented 1 week ago

@alexanderlazarev0 A little off topic, I'm writing some alternative version for alternative universe) https://github.com/modern-python/modern-di

lesnik512 commented 1 week ago

And there I wrote resource which accepts context manager classes https://github.com/modern-python/modern-di/commit/4582b5c9c8990a111e381ee3fd352188a95a8cb4