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

feature request (/doc?): make use of providers from base class less confusing #367

Closed
shaunc opened this issue Jan 20, 2021 · 4 comments
Closed
Assignees
Labels

Comments

@shaunc
Copy link

shaunc commented Jan 20, 2021

Version 1

Here is what I aspire to do:

class A(containers.DeclarativeContainer):
    a = providers.Dependency(str)
    c = providers.Dependency(str)

@containers.copy(A)
class B(A):
    b = providers.Callable(lambda a: f"b_{a}", A.a.provided)
    c = b.provided[0]

print(B(a="a").c())  # f? no: Dependency not defined

So base class A sets up some dependencies. Subclass B provides a provider that depends on one of them, and provides the other. Unfortunately that version doesn't work.

Version 2

Without the copy still doesn't work, though traceback looks different:

class A(containers.DeclarativeContainer):
    a = providers.Dependency(str)
    c = providers.Dependency(str)

# @containers.copy(A)
class B(A):
    b = providers.Callable(lambda a: f"b_{a}", A.a.provided)
    c = b.provided[0]

print(B(a="a").c())  # Dependency not defined

Version 3

If I copy a explicitly, but still leave off the copy, still doesn't work:

class A(containers.DeclarativeContainer):
    a = providers.Dependency(str)
    c = providers.Dependency(str)

# @containers.copy(A)
class B(A):
    a = providers.Dependency(str)
    b = providers.Callable(lambda a: f"b_{a}", a.provided)
    c = b.provided[0]

print(B(a="a").c())  # f? -- Dependency is not defined

Version 4

Finally, with copy of a and copy, will work:

class A(containers.DeclarativeContainer):
    a = providers.Dependency(str)
    c = providers.Dependency(str)

@containers.copy(A)
class B(A):
    a = providers.Dependency(str)
    b = providers.Callable(lambda a: f"b_{a}", a.provided)
    c = b.provided[0]

print(B(a="a").c())  # f

However

However, it isn't ideal to have to explicitly copy parts of the base class in the derived class. For more complex examples I find I have to copy up whole subgraphs, which isn't DRY -- after all the point of factoring out some definitions into a mixin is so that you can use them as a single source of authority.

So the feature request is for the first version to work: if I write A.a in class B which derives from A, then a reference to A.a should be automatically put into B (unless it is explicitly declared). That way, if, e.g. I decide to change the name of a in the base class, I'll "fail fast" -- I won't be able to load the code.

@rmk135 rmk135 self-assigned this Jan 20, 2021
@rmk135
Copy link
Member

rmk135 commented Jan 29, 2021

Hey @shaunc ,

I found a bug in the container metaclass. It was breaking child container. Second example now works like you described:

class A(containers.DeclarativeContainer):
    a = providers.Dependency(str)
    c = providers.Dependency(str)


class B(A):
    b = providers.Callable(lambda a: f"b_{a}", A.a.provided)
    c = b.provided[0]


print(B(a="a").c())  # prints "b"

I'm publishing version 4.13.1 with a fix: https://github.com/ets-labs/python-dependency-injector/actions/runs/522074952

@rmk135 rmk135 added the bug label Jan 29, 2021
@rmk135
Copy link
Member

rmk135 commented Jan 30, 2021

@shaunc fix is published.

@rmk135
Copy link
Member

rmk135 commented Feb 2, 2021

@shaunc I'm closing this for now. Please, comment if any issues.

@rmk135 rmk135 closed this as completed Feb 2, 2021
@shaunc
Copy link
Author

shaunc commented Feb 2, 2021

I think we're good -- thanks!

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

2 participants