python-dependency-injector icon indicating copy to clipboard operation
python-dependency-injector copied to clipboard

Pass arguments to __init__ of (Async)Resource[T] instead of init

Open EdwardBlair opened this issue 3 years ago • 1 comments

It would be nice if we could do the following:

class MyDep:
    def __init__(self, param: str) -> None:
        ...

class MyDepResource(resources.AsyncResource[MyDep]):
    def __init__(self, argument1: str) -> None:
        self._argument1 = argument1
    
    async def init(self) -> MyDep:
        param = self._do_something_complicated()
        return MyDep(param)

    async def shutdown(self, resource: MyDep) -> None:
        ...

    def _do_something_complicated(self) -> str:
        return self._argument1[::-1]

Conceptually instantiation is currently python MyDepResource().init(...args...), I propose MyDepResource(...args...).init()

The protocol for (Async)Resource then becomes more simple, and as I understand, you would get more strict typing of the arguments since they're now defined on the subclass's __init__ (instead of just being *args: Any, **kwds:Any)

class AsyncResource(Generic[T], metaclass=ResourceMeta):

    @abc.abstractmethod
    async def init(self) -> T:
        ...

    @abc.abstractmethod
    async def shutdown(self, resource: T) -> None:
        ...

There's no real difference in practice, but it feels more natural to do plumbing in __init__.

I believe there would be a possiblity for backwards compatability with this change to aid API migration should you wish to do it (pass arguments to init also when it is invoked).

Thoughts?


Additionally:

Method init() receives arguments specified in resource provider. It performs initialization and returns resource object. Returning of the object is not mandatory.

Should the type param not be Optional[T]

Method shutdown() receives resource object returned from init(). If init() didn’t return an object shutdown() method will be called anyway with None as a first argument.

Should the signature be shutdown(self, resource: Optional[T]) -> None:, and since its default behaviour is no-op, is there a requirement for it to be marked as abstract? (I'm guessing so, but would be nice to know why)

EdwardBlair avatar Aug 15 '21 16:08 EdwardBlair

Should the type param not be Optional[T]

It should

Should the signature be shutdown(self, resource: Optional[T]) -> None:

It should.

and since its default behaviour is no-op, is there a requirement for it to be marked as abstract? (I'm guessing so, but would be nice to know why)

No, definition in subclass can be avoided. Framework will call shutdown method anyway, but it can go to default implementation.

Appreciate if you could take a look on #492.


Still thinking on the first part of your question.

rmk135 avatar Aug 19 '21 22:08 rmk135