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

Dependencies with yield (used as context managers) #595

Merged
merged 16 commits into from
Oct 9, 2019

Conversation

tiangolo
Copy link
Owner

@tiangolo tiangolo commented Oct 6, 2019

Add support for context manager dependencies using functions with yield. Sync or async.

With inspiration from @dmontagu 's #570.

This handles dependencies that yield by converting them into a sync or async context manager, depending on the function. And runs them in a "context manager stack".

This way, the order of dependencies is preserved and cleanup/closing happens in the correct order (the first dependency called is cleaned last).

This also means that dependencies that require other dependencies will still have them on scope during cleanup. So, contextmanager-style dependencies that require other contextmanager dependencies will have access to them during cleanup.

When a dependency function is a normal function (not async), the context manager is called in a threadpool (__enter__), so, the same as with normal dependencies, it's safe to do non-async blocking IO in contextmanager style dependencies. E.g. start a DB connection, probably the main use-case for this. And the same for cleaning it up (__exit__).

This works by default in Python 3.7.

And works in Python 3.6 if the backports are installed:

  • async-exit-stack
  • async-generator
pip install async-exit-stack async-generator

But they are not forced/required dependencies. If an app doesn't use context managers and still runs in Python 3.6, it will still work without the backports.

...Although I suspect most of the apps will use this for DB connections.

These backport dependencies are checked on app initialization, so, if a contextmanager-style dependency function (with yields) is found, the environment is not Python 3.7+, and the backports are not installed, it will raise a RuntimeError right away on app initialization (so, no risk for the error lurking into a prod environment).


TODO:

  • Fix mypy types.
  • Extend tests to check for correct order of contextmanager dependencies with subdependencies.
  • Extend tests to verify that background tasks can still access the dependencies from context managers (e.g. DB connections).
  • Add docs.

@dmontagu
Copy link
Collaborator

dmontagu commented Oct 6, 2019

This is looking really great!

@codecov
Copy link

codecov bot commented Oct 6, 2019

Codecov Report

Merging #595 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #595    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files         264    269     +5     
  Lines        6429   6823   +394     
======================================
+ Hits         6429   6823   +394
Impacted Files Coverage Δ
docs/src/sql_databases/sql_app/main.py 100% <100%> (ø) ⬆️
...est_sql_databases/test_sql_databases_middleware.py 100% <100%> (ø)
..._tutorial/test_sql_databases/test_sql_databases.py 100% <100%> (ø) ⬆️
fastapi/applications.py 100% <100%> (ø) ⬆️
tests/test_fakeasync.py 100% <100%> (ø)
fastapi/concurrency.py 100% <100%> (ø)
fastapi/dependencies/utils.py 100% <100%> (ø) ⬆️
docs/src/sql_databases/sql_app/alt_main.py 100% <100%> (ø)
tests/test_dependency_contextmanager.py 100% <100%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b65be5d...553f503. Read the comment docs.

fastapi/concurrency.py Outdated Show resolved Hide resolved
@tiangolo
Copy link
Owner Author

tiangolo commented Oct 6, 2019

@dmontagu this is ready. 🚀 🎉

I used some sections from your original PR 😅

Do you wanna give it a last check before I merge?

@tiangolo tiangolo changed the title WIP: Context manager dependencies Dependencies with yield (used as context managers) Oct 6, 2019
Copy link
Collaborator

@dmontagu dmontagu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly just minor docs fixes. Some are typos, some are more opinion-based; feel free to ignore anything you disagree with!

docs/tutorial/sql-databases.md Outdated Show resolved Hide resolved
docs/tutorial/dependencies/dependencies-with-yield.md Outdated Show resolved Hide resolved
docs/tutorial/dependencies/dependencies-with-yield.md Outdated Show resolved Hide resolved
docs/tutorial/dependencies/dependencies-with-yield.md Outdated Show resolved Hide resolved
docs/tutorial/sql-databases.md Outdated Show resolved Hide resolved
docs/tutorial/sql-databases.md Outdated Show resolved Hide resolved
docs/tutorial/sql-databases.md Outdated Show resolved Hide resolved
docs/tutorial/sql-databases.md Show resolved Hide resolved
fastapi/dependencies/utils.py Show resolved Hide resolved
Co-Authored-By: dmontagu <35119617+dmontagu@users.noreply.github.com>
@tiangolo
Copy link
Owner Author

tiangolo commented Oct 9, 2019

Thanks a lot for the thorough review @dmontagu ! You have an eagle eye! 🦅 🔍 🕵️‍♂️

@tiangolo tiangolo merged commit 3f9f4a0 into master Oct 9, 2019
@tiangolo tiangolo deleted the context-manager-dependencies branch October 9, 2019 18:02
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