-
-
Notifications
You must be signed in to change notification settings - Fork 340
Fix Closing dependency resolution
#711
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
Fix Closing dependency resolution
#711
Conversation
Allow Closing to detect dependent resources passed as kwargs too ets-labs#636
| nested_service = providers.Factory(NestedService, factory_service) | ||
|
|
||
|
|
||
| class ContainerSingleton(containers.DeclarativeContainer): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added another container to cover a case where a resource is not at the end of the dependency graph. A Singleton provider type is arbitrary - its type doesn't play any role in injections, just needed something for a Resource to depend on.
|
I have in mind a few potential test cases that could be added: when there are no resources in the dependency graph; and when there are several different resources. |
|
|
||
| if not arg.args and isinstance(arg, providers.Resource): | ||
| if isinstance(arg, providers.Resource): | ||
| return {str(id(arg)): arg} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @jazzthief. Fix not working for several Resources. Maybe something like this?
| return {str(id(arg)): arg} | |
| closing_deps |= {str(id(arg)): arg} |
|
Changes from this PR were merged as part of #852 |
Fix issues found when using
Closingto implement per-function scope forResources.Closingdoes not resolve nested dependecies #702: e.g. when aFactorydepends on anotherFactorydependent on aResourceResourcewhen it is the last element in the dependency graph