-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Create a task per request handling. #3442
Conversation
Fixes #3406 A separate task for every request doesn't produce any performance degradation on benchmarks. Tests update is needed to wait not only a task started by connection_made() but also a nexted task.
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.
LGFM. Indeed, it doesn't affects performance (or it does, but those numbers get lost in statistical error).
Looks good. We should document this and state prominently that aiohttp supports ContextVar? Perhaps with an example of how to use it. |
@samuelcolvin what docs part do you suggest? |
I guess. Not sure if really matters, just if someone searches "aiohttp ContextVar" they should get a result in docs. |
Codecov Report
@@ Coverage Diff @@
## master #3442 +/- ##
==========================================
+ Coverage 97.93% 97.93% +<.01%
==========================================
Files 44 44
Lines 8538 8539 +1
Branches 1378 1378
==========================================
+ Hits 8362 8363 +1
Misses 72 72
Partials 104 104
Continue to review full report at Codecov.
|
I've started to write a doc but found that there are questions about contextvars initialization:
Let me merge the PR as is and solve the questions in separate one (along with documentation update). Maybe |
Fixes #3406
A separate task for every request doesn't produce any performance degradation on benchmarks.
Tests update is needed to wait for not only a task started by connection_made() but also a nested task.