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

Enhancement Proposal: Generalized DI system #3641

Closed
wants to merge 10 commits into from

Conversation

adriangb
Copy link
Contributor

@adriangb adriangb commented Aug 1, 2021

A high-level proposal for enhancements to the DI system.

@adriangb
Copy link
Contributor Author

adriangb commented Aug 31, 2021

I believe tbe original request in #3620 also be made to work by having a dependency scope that goes out of scope before the request is returned (in addition to the default/current which goes out of scope after the request is returned)

@ghandic
Copy link
Contributor

ghandic commented Sep 28, 2021

@adriangb does this PR help this issue too? #3953

@adriangb
Copy link
Contributor Author

@adriangb does this PR help this issue too? #3953

I can't tell just from the description of the issue, but if it is about raising exceptions after yield in dependencies, then yes, this could potentially help certain use cases

@ghandic
Copy link
Contributor

ghandic commented Sep 28, 2021

It derives from this issue raised in the discussions where exceptions are raised in the Depends - for instance, pydantic validation errors coming from BaseModel Query parmeters #3426 (reply in thread)

@adriangb
Copy link
Contributor Author

I think I understand. Truth be told, that seems like a bug in FastAPI exception handling or an unsupported feature, depending on how you look at it. This proposal may help in cleaning up the implementations, which might indirectly help with that issue, but since this proposal doesn't say anything about exception handling, I don't think it directly addresses that issue

@adriangb
Copy link
Contributor Author

adriangb commented Oct 4, 2021

Seems like #3902 and #639 are also related. di is currently able to parallelize both sync and async dependencies: https://github.com/adriangb/di/tree/main/benchmarks

@adriangb
Copy link
Contributor Author

Relevant discussion: #4035

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

Successfully merging this pull request may close these issues.

None yet

2 participants