♻️ Create dependency-cache dict in solve_dependencies only if None (don't re-create if empty)#13689
Merged
Merged
Conversation
f64d254 to
a30f83c
Compare
a30f83c to
7c0a1e6
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
Author
|
@YuriiMotov sorry for mention you, can you please give any advice that can I do to move this PR further? I think these changes a pretty safe and improve a bit performance and mem-print, although in my company we need them to remove hacks relates to manual solve_dependencies usage out of fastapi-http-scope |
dependency-cache dict in solve_dependencies only if None (don't re-create if empty)
YuriiMotov
approved these changes
Jun 28, 2025
YuriiMotov
left a comment
Member
There was a problem hiding this comment.
LGTM
These changes make code clearer and less error-prone.
@bokshitsky, I would recommend you update the description to make it more concise
Contributor
Author
|
@YuriiMotov thanks, I updated, I think changes are clearer now. |
Contributor
Author
|
@tiangolo hello, may you look please - can I do something to include patch in following releases? |
tiangolo
approved these changes
Sep 20, 2025
tiangolo
left a comment
Member
There was a problem hiding this comment.
Sounds good! 🚀
Thanks for the detailed explanation. 🍰
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello,
Current code:
dependency_cachedictionary insidesolve_dependenciesfunction if passeddependency_cacheparameter is None or empty.dependency_cache.update(solved_result.dependency_cache)is required at every iteration of dependencies solving, becausesolved_result.dependency_cachereturned by recursion can be either same asdependency_cache(if at least one dependency was already stored in cache) or new dict (on first iterations of recursion where empty cache is passed to recursive function)This behaviour adds a bit complexity to the code and spends (albeit a little) more CPU and memory resources.
My proposal:
dependency_cache.update(solved_result.dependency_cache)is not required, because onedependency_cacheis used over all dependency solving process.