Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

using a dependencies container as a dependency #354

Closed
shaunc opened this issue Jan 12, 2021 · 10 comments
Closed

using a dependencies container as a dependency #354

shaunc opened this issue Jan 12, 2021 · 10 comments
Assignees
Labels

Comments

@shaunc
Copy link

shaunc commented Jan 12, 2021

Here is another example causing a problem -- using a dependenciesContainer as a dependency:

class D(containers.DeclarativeContainer):
    foo = providers.Object("foo")

class A(containers.DeclarativeContainer):
    d = providers.DependenciesContainer()
    bar = providers.Callable(lambda f: f + "++", d.foo.provided)

class B(containers.DeclarativeContainer):
    d = providers.DependenciesContainer()

    a = providers.Container(A, d=d)

b = B(d = D())
b.a().bar()  # gives Error: Dependency is not defined
@shaunc
Copy link
Author

shaunc commented Jan 12, 2021

FWIW -- here is a workaround. First, convert the DependenciesContainer into dependencies. Then there is still a problem, as the lazy eval doesn't make it through two layers of laziness on its own. So for instance converting d.foo.provided into d.provided.foo.provided doesn't work... which brings us to the workaround:

from typing import Any
def resolve_part(c: Any, part: str) -> Any:
    """
    Resolve a part of a container.

    Helper to work around: https://github.com/ets-labs/python-dependency-injector/issues/354

    As `DependenciesContainer` can't be passed, have converted
    into a `Dependency`. However, that makes things "too lazy",
    as we have to use <container>.provided.<part>.provided, and
    consumer must do <container-instance>()().

    Instead, we take first provided "lazy" and resolve last greedily
    """
    return providers.Callable(lambda c: getattr(c, part)(), c)


class D(containers.DeclarativeContainer):
    foo = providers.Object("foo")

class A(containers.DeclarativeContainer):
    d = providers.Dependency(instance_of=containers.DynamicContainer)
    foo = resolve_part(d.provided, "foo")
    bar = providers.Callable(lambda f: f + "++", foo.provided)

class B(containers.DeclarativeContainer):
    d = providers.Dependency(instance_of=containers.DynamicContainer)

    a = providers.Container(A, d=d)

b = B(d = D())
b.a().bar()  # prints "foo++"

@rmk135 rmk135 self-assigned this Jan 12, 2021
@rmk135 rmk135 added the bug label Jan 12, 2021
@rmk135
Copy link
Member

rmk135 commented Jan 12, 2021

Hi @shaunc ,

To avoid the problem with current codebase you need to explicitly specify the interface of B.d dependencies container:

Screenshot 2021-01-12 at 08 24 43

The problem is that B.d dependencies container is empty when it's wired with B.a. The wiring might happen later, that's the point to improve providers.Container.

@rmk135
Copy link
Member

rmk135 commented Jan 12, 2021

I have published version 4.8.0 that adds support of overriding Container provider:

Screenshot 2021-01-12 at 08 45 34

@shaunc
Copy link
Author

shaunc commented Jan 12, 2021

Thanks for the fix. It is still a WIP to have both A and B lazily take a dependency container, I guess? The work around you provide seems to allow for D to provide a fixed selection of things -- which fit the example I gave, but that was just because I wanted provide a minimal example. Can we extend that workaround to the case where there are multiple dependencies D provides? (Perhaps create DDummy with all dependencies ... hmm...)

In passing -- sort of odd that one can specify a type for a dependency in general, but not one for a DependenciesContainer. This is forced, I guess, by the fact that a container instance C() is not actually an instance of C -- but.. maybe in your meta class you could create a derivative of DynamicContainer that also inherits from C, so that C() could be an instanceof both?

@rmk135
Copy link
Member

rmk135 commented Jan 13, 2021

Yes, WIP. Provider Container could be more lazier.

I have a fix, but I'm not yet sure if it's safe to apply it. I have an idea what I could do to make it safer, but it will take some more time. Plan to finish it later today.

@rmk135
Copy link
Member

rmk135 commented Jan 13, 2021

@shaunc , fixed in 4.8.2. Initial code sample works now.

@rmk135
Copy link
Member

rmk135 commented Jan 13, 2021

Close the issue, re-open if needed. Many thanks for submitting it!

@rmk135 rmk135 closed this as completed Jan 13, 2021
@shaunc
Copy link
Author

shaunc commented Jan 13, 2021

Great -- thank you!

@approxit
Copy link

Instead of opening new issue I'll comment right here, because 4.8.2 broke different case for providers.Container usage where declaration is not at class root level.

Here is somewhat simplified example:

from dependency_injector import providers, containers


class NestedContainer(containers.DeclarativeContainer):
	settings = providers.Configuration()

	print_settings = providers.Callable(
		lambda s: print(s),
		settings,
	)


class TestContainer(containers.DeclarativeContainer):
	settings = providers.Configuration()

	root_container = providers.Container(
		NestedContainer,
		settings=settings,
	)

	not_root_container = providers.Selector(
		settings.container,
		using_factory=providers.Factory(
			NestedContainer,
			settings=settings,
		),
		using_container=providers.Container(
			NestedContainer,
			settings=settings,
		)
	)


container = TestContainer(settings=dict(
	container='using_factory',
	foo='bar'
))

container.root_container().print_settings()  # Prints {'container': 'using_factory', 'foo': 'bar'}
container.not_root_container().print_settings()  # Prints {'container': 'using_factory', 'foo': 'bar'}

container = TestContainer(settings=dict(
	container='using_container',
	foo='bar'
))

container.root_container().print_settings()  # Prints {'container': 'using_container', 'foo': 'bar'}
container.not_root_container().print_settings()  # Prints {} after 4.8.2

We expect here for all prints to be equal, and 4.8.2 breaks it. Workaround is using providers.Factory, but still case is some kind of a regression.

@rmk135
Copy link
Member

rmk135 commented Jan 27, 2021

ezgif-7-0d11c34a9a3a

@approxit thanks for the heads-up. I'll covert it into an issue and take a look asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants