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.89k stars 304 forks source link

Async resources init order #449

Open laukhin opened 3 years ago

laukhin commented 3 years ago

Hi!

We've started to use dependency-injector in production recently and we're happy so far, but we've faced issue with async resources initialization order. The problem is that according to the source code async resources init methods runs concurrently using asyncio.gather https://github.com/ets-labs/python-dependency-injector/blob/155f59869922de2c0ae57ab2e77d85655e15f21b/src/dependency_injector/containers.pyx#L278

But in some cases we need to initialize some resources before the others and that's where we've faced race conditions.

Is there any way to avoid such situations in current implementation or future releases? If not i'd be happy to contribute :)

rmk135 commented 3 years ago

Hi @laukhin ,

There is no way to initialize resources in a certain order with container.init_resources() right now. As a workaround you initialize the resources in explicit order in a blocking fashion:

await container.resource1.init()
await container.resource2.init()
await container.resource3.init()

Looks like the proper solution here is to implement some sort of priority mechanism for the Resource provider. Need to think of interface for this. Also interface suggestions (or any other help) is more then welcome!

laukhin commented 3 years ago

I see a couple of options for interfaces here

  1. we could explicitly declare the dependencies list

    from dependency_injector import containers, providers
    class Container(containers.DeclarativeContainer):
    first_resource = providers.Resource(some_resource)
    second_resource = providers.Resource(other_resource, dependencies=[first_resource])
  2. or we could go django-way and make something like this

    from dependency_injector import containers, providers
    class Container(containers.DeclarativeContainer):
    first_resource = providers.Resource(some_resource)
    second_resource = providers.Resource(other_resource)
    
    class Config:
        resources_init_order = ['first_resource', 'second_resource', '*']

Personally, i like first approach, it's user-friendly, but it could be tricky to build dependencies graph from it with more complicated cases. Second approach is more cryptic, but potentially easier to implement. Seems like in both ways we're protected from circular dependencies, so there shouldn't be any problems with it.

rmk135 commented 3 years ago

I also like first approach more. I would leave dependencies for a potential kwarg and transform it to something like this:

from dependency_injector import containers, providers

class Container(containers.DeclarativeContainer):

    first_resource = providers.Resource(some_resource)

    second_resource = providers.Resource(other_resource, dependencies=[first_resource])
    second_resource.add_init_dependencies(first_resource)

How does it look to you?

laukhin commented 3 years ago

To summarize: you suggest to drop kwargs approach and simply make method add_init_dependencies for the resource as an interface? If i understand correctly, LGTM! I could work on it if you don't mind 🙂

rmk135 commented 3 years ago

If i understand correctly, LGTM!

Yes, correct. The only difference is that I think we could remove "init" from the method name add_dependencies(). Here is a complete example:

from dependency_injector import containers, providers

class Container(containers.DeclarativeContainer):

    first_resource = providers.Resource(some_resource)

    second_resource = providers.Resource(other_resource, dependencies=[first_resource])
    second_resource.add_dependencies(first_resource)

I could work on it if you don't mind 🙂

That would be nice if you could help with it. We could use slack to communicate faster. Here is a dependency injector slack link: https://join.slack.com/t/ets-labs/shared_invite/zt-pqbw3869-IZkpiLLFkev9LmlAuChyww

rmk135 commented 3 years ago

@laukhin Btw, there is a similar problem with a shutdown, look at #432

laukhin commented 3 years ago

thx, i've just joined your slack workspace (by email laukhin97@gmail.com) :) i'll contact you if any questions arise

amazingguni commented 3 years ago

I love this feature too :D I used to modify __init__ to set the order of initialization. (you know, it looks not a good inplementation)

skewty commented 2 years ago

It is coming up 1 year since this issue was first opened. What ever came of this?