ets-labs / python-dependency-injector

Dependency injection framework for Python
https://python-dependency-injector.ets-labs.org/
BSD 3-Clause "New" or "Revised" License
3.88k stars 304 forks source link

`Closing` does not resolve nested dependecies #702

Open jazzthief opened 1 year ago

jazzthief commented 1 year ago

Closing doesn't seem to resolve nested dependecies: e.g. when a Factory depends on another Factory dependent on a Resource. I have followed issue #633, and linked PR #636 that dealt with the detection of dependent resources, however in my example it doesn't look like it is working when the dependencies are nested.

The following example replicates the code from dependecy-injector test suite used to test the dependent resources resolution in PR #636. Apart from different naming, the only things I added are NestedService and test_nested_dependency():

from dependency_injector import containers, providers
from dependency_injector.wiring import inject, Provide, Closing

class MyResource:
    init_counter: int = 0
    shutdown_counter: int = 0

    @classmethod
    def reset_counter(cls):
        cls.init_counter = 0
        cls.shutdown_counter = 0

    @classmethod
    def init(cls):
        cls.init_counter += 1

    @classmethod
    def shutdown(cls):
        cls.shutdown_counter += 1

class FactoryService:
    def __init__(self, resource: MyResource):
        self.resource = resource

class NestedService:
    def __init__(self, factory: FactoryService):
        self.factory = factory

def init_resource():
    resource = MyResource()
    resource.init()
    yield resource
    resource.shutdown()

class Container(containers.DeclarativeContainer):

    resource = providers.Resource(init_resource)
    factory_service = providers.Factory(FactoryService, resource)
    nested_service = providers.Factory(NestedService, factory_service)

@inject
def test_function(resource: MyResource = Closing[Provide["resource"]]):
    return resource

@inject
def test_function_dependency(
    factory: FactoryService = Closing[Provide["factory_service"]]
):
    return factory

@inject
def test_nested_dependency(
    nested: NestedService = Closing[Provide["nested_service"]]
):
    return nested.factory

if __name__ == "__main__":
    container = Container()
    container.wire(modules=[__name__])

    MyResource.reset_counter()
    for _ in range(3):
        result = test_nested_dependency()
        print(f"Initialized {result.resource.init_counter} times")
        print(f"Shut down {result.resource.shutdown_counter} times")

Running this with python 3.11.1 and dependency-injector 4.41.0 fails for me with the following traceback:

  File "/home/jazzthief/prg/api-test/api_test/test_closing.py", line 68, in <module>
    container.wire(modules=[__name__])
  File "src/dependency_injector/containers.pyx", line 317, in dependency_injector.containers.DynamicContainer.wire
  File "/home/jazzthief/prg/api-test/.venv/lib/python3.11/site-packages/dependency_injector/wiring.py", line 426, in wire
    _bind_injections(patched, providers_map)
  File "/home/jazzthief/prg/api-test/.venv/lib/python3.11/site-packages/dependency_injector/wiring.py", line 633, in _bind_injections
    deps = _locate_dependent_closing_args(provider)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jazzthief/prg/api-test/.venv/lib/python3.11/site-packages/dependency_injector/wiring.py", line 608, in _locate_dependent_closing_args
    closing_deps += _locate_dependent_closing_args(arg)
TypeError: unsupported operand type(s) for +=: 'dict' and 'dict'

Could anyone please provide any guidance as to whether this is expected behaviour or any existing workarounds? I am trying to use dependency-injector in a large FastAPI project which would greatly benefit from nested Resource closing - and would be glad to try and fix this if possible.

kkjot88 commented 1 year ago

Here is your example fixed to at least run, results are not what u want thou:

from dependency_injector import containers, providers
from dependency_injector.wiring import inject, Provide, Closing

class MyResource:
    init_counter: int = 0
    shutdown_counter: int = 0

    @classmethod
    def reset_counter(cls):
        cls.init_counter = 0
        cls.shutdown_counter = 0

    @classmethod
    def init(cls):
        cls.init_counter += 1

    @classmethod
    def shutdown(cls):
        cls.shutdown_counter += 1

class FactoryService:
    def __init__(self, resource: MyResource):
        self.resource = resource

class NestedService:
    def __init__(self, factory: FactoryService):
        self.factory = factory

def init_resource():
    resource = MyResource()
    resource.init()
    yield resource
    resource.shutdown()

class Container(containers.DeclarativeContainer):
    resource = providers.Resource(init_resource)
    factory_service = providers.Factory(FactoryService, resource)
    nested_service = providers.Factory(NestedService, factory_service)

@inject
def test_function(resource: MyResource = Closing[Provide["resource"]]):
    return resource

@inject
def test_function_dependency(
    # no "Closing" since Container.factory_service itself is not a providers.Resource
    factory: FactoryService = Provide["factory_service"],
):
    return factory

@inject
def test_nested_dependency(
    # no "Closing" since Container.nested_service itself is not a providers.Resource
    nested: NestedService = Provide["nested_service"],
):
    return nested.factory

if __name__ == "__main__":
    container = Container()
    container.wire(modules=[__name__])

    MyResource.reset_counter()
    for _ in range(3):
        result = test_nested_dependency()
        print(f"Initialized {result.resource.init_counter} times")
        print(f"Shut down {result.resource.shutdown_counter} times")
<__main__.NestedService object at 0x7fa7838c15d0> <generator object at 0x7fa783a75d00>
Initialized 1 times
Shut down 0 times
<__main__.NestedService object at 0x7fa7838c14b0> <generator object at 0x7fa783a75d00>
Initialized 1 times
Shut down 0 times
<__main__.NestedService object at 0x7fa7838c14b0> <generator object at 0x7fa783a75d00>
Initialized 1 times
Shut down 0 times

Process finished with exit code 0

This is because Closing is a wiring thing and it only works when stuff is injected. "Initialized 1 times" is printed 3 times because all the resource declared in the container are initialized once upon container instantiation (or when "container.init_resources()" is called directly) and will only be closed if u call "container.shutdown_resources()", which is not done at all in this example.

To achieve what u want this is what u could do, i would be cautious thou since "Closing" is kind of bugged - https://github.com/ets-labs/python-dependency-injector/issues/699

from dependency_injector import containers, providers
from dependency_injector.wiring import inject, Provide, Closing

class MyResource:
    init_counter: int = 0
    shutdown_counter: int = 0

    @classmethod
    def reset_counter(cls):
        cls.init_counter = 0
        cls.shutdown_counter = 0

    @classmethod
    def init(cls):
        cls.init_counter += 1

    @classmethod
    def shutdown(cls):
        cls.shutdown_counter += 1

class FactoryService:
    @inject
    def __init__(self, resource: MyResource = Closing[Provide["resource"]]):
        self.resource = resource

class NestedService:
    def __init__(self, factory: FactoryService):
        self.factory = factory

def init_resource():
    resource = MyResource()
    resource.init()
    yield resource
    resource.shutdown()

class Container(containers.DeclarativeContainer):
    resource = providers.Resource(init_resource)
    # We don't pass resource here because it has to injected to take advantage
    # of the "Closing" marker.
    factory_service = providers.Factory(FactoryService)
    nested_service = providers.Factory(NestedService, factory_service)

@inject
def test_function(resource: MyResource = Closing[Provide["resource"]]):
    return resource

@inject
def test_function_dependency(
    # no "Closing" since Container.factory_service itself is not a providers.Resource
    factory: FactoryService = Provide["factory_service"],
):
    return factory

@inject
def test_nested_dependency(
    # no "Closing" since Container.nested_service itself is not a providers.Resource
    nested: NestedService = Provide["nested_service"],
):
    return nested.factory

if __name__ == "__main__":
    container = Container()
    container.wire(modules=[__name__])
    container.init_resources()
    container.shutdown_resources()

    MyResource.reset_counter()
    for _ in range(3):
        result = test_nested_dependency()
        print(f"Initialized {result.resource.init_counter} times")
        print(f"Shut down {result.resource.shutdown_counter} times")
<__main__.MyResource object at 0x7fa26cf48d00> <generator object at 0x7fa26de6b420>
<__main__.NestedService object at 0x7fa26cf48d30> <generator object at 0x7fa26de6b420>
Initialized 1 times
Shut down 1 times
<__main__.MyResource object at 0x7fa26cf48d60> <generator object at 0x7fa26de6b420>
<__main__.NestedService object at 0x7fa26cf48dc0> <generator object at 0x7fa26de6b420>
Initialized 2 times
Shut down 2 times
<__main__.MyResource object at 0x7fa26cf48cd0> <generator object at 0x7fa26de6b420>
<__main__.NestedService object at 0x7fa26cf48dc0> <generator object at 0x7fa26de6b420>
Initialized 3 times
Shut down 3 times

Process finished with exit code 0
jazzthief commented 1 year ago

@kkjot88 I see your point, but my example is just for display purposes. What I wanted to do is use Closing to achieve per-function execution scope - as stated in the docs. And it seems to me that the only logical thing for Closing to do is to traverse the dependency graph until it hits a Resource, warning the user (or even throwing an exception) if no resource was found. #636 added just that, but the tests don't cover the "nested" case I describe, so it has an error that was never found. I am currently resolving these issues and will make sure to link a PR to the present issue.

kkjot88 commented 1 year ago

@kkjot88 I see your point, but my example is just for display purposes. What I wanted to do is use Closing to achieve per-function execution scope - as stated in the docs. And it seems to me that the only logical thing for Closing to do is to traverse the dependency graph until it hits a Resource, warning the user (or even throwing an exception) if no resource was found. #636 added just that, but the tests don't cover the "nested" case I describe, so it has an error that was never found. I am currently resolving these issues and will make sure to link a PR to the present issue.

Yeah, u are right. It should work. This minor change fixes it for me and your original code works as expected.

# .../venv/lib/python3.10/site-packages/dependency_injector/wiring.py
def _locate_dependent_closing_args(provider: providers.Provider) -> Dict[str, providers.Provider]:
    if not hasattr(provider, "args"):
        return {}

    closing_deps = {}
    for arg in provider.args:
        if not isinstance(arg, providers.Provider) or not hasattr(arg, "args"):
            continue

        if not arg.args and isinstance(arg, providers.Resource):
            return {str(id(arg)): arg}
        else:
            closing_deps.update(_locate_dependent_closing_args(arg))
            # closing_deps += _locate_dependent_closing_args(arg)
    return closing_deps
<__main__.NestedService object at 0x7f9dd05c16c0> <generator object at 0x7f9dd0775bc0>
Initialized 1 times
Shut down 1 times
<__main__.NestedService object at 0x7f9dd05c3490> <generator object at 0x7f9dd0775bc0>
Initialized 2 times
Shut down 2 times
<__main__.NestedService object at 0x7f9dd05c3490> <generator object at 0x7f9dd0775bc0>
Initialized 3 times
Shut down 3 times

Process finished with exit code 0

Still I would advise caution when dealing with "Closing" because of https://github.com/ets-labs/python-dependency-injector/issues/699 unless I am wrong on that too :D.

jazzthief commented 1 year ago

@kkjot88 Your advice is absolutely right, as this += is not the only problem with _locate_dependent_closing_args(). I am working to change it and add more tests with various dependency graph configurations.

ssbbkn commented 9 months ago

@jazzthief @kkjot88 Hi, guys! Thanks for your conversation - it helped me a lot in solving my problem. Found another issue in _locate_dependent_closing_args. Function does not work correctly in case when there are more than 1 nested resource dependency in provider. In current state function just overwrites closing_deps with its return {str(id(arg)): arg}.

Also for some reason this function is looking for Resource only in provider args, but not in kwargs. Fixed this too.

There is a version that works for me:

def _locate_dependent_closing_args(provider: providers.Provider) -> Dict[str, providers.Provider]:
    if not hasattr(provider, "args"):
        return {}

    closing_deps = {}
    # for arg in provider.args:
    for arg in [*provider.args, *provider.kwargs.values()]:
        if not isinstance(arg, providers.Provider) or not hasattr(arg, "args"):
            continue

        if not arg.args and isinstance(arg, providers.Resource):
            # return {str(id(arg)): arg}
            closing_deps.update({str(id(arg)): arg})
        else:
            closing_deps.update(_locate_dependent_closing_args(arg))
    return closing_deps