-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Detect blocking calls in coroutines using BlockBuster #10433
Conversation
1556000
to
d0c591f
Compare
I think this is a very nice change. I'm going to run out of time to review it today. I'd add excludes for the known issues and add inline comments that they blocking I/O so they can be fixed when someone h as time to work out each one. |
CodSpeed Performance ReportMerging #10433 will improve performances by 11.57%Comparing Summary
Benchmarks breakdown
|
0e67b36
to
08e02f5
Compare
done in 08e02f5 |
32a6018
to
79b1401
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #10433 +/- ##
=======================================
Coverage 98.70% 98.70%
=======================================
Files 122 122
Lines 37191 37198 +7
Branches 2056 2060 +4
=======================================
+ Hits 36709 36716 +7
Misses 335 335
Partials 147 147
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ab7b7fb
to
2328c42
Compare
ecb84cd
to
b7cec65
Compare
It looks like the checks are running in the benchmarks which is benchmarking the time for the checks. They should be excluded from the benchmark runs |
Tests were working a few days ago, right? Maybe the version of blockbuster needs to be downgraded again? |
cc0bd42
to
845d2f1
Compare
c0f19dc
to
60f0a64
Compare
for more information, see https://pre-commit.ci
Thanks @cbornet |
Backport to 3.12: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 0c4b1c7 on top of patchback/backports/3.12/0c4b1c74cd7bffb544dacb4a4d8793f80fd4ad32/pr-10433 Backporting merged PR #10433 into master
🤖 @patchback |
@cbornet Looks like this couldn't be backported automatically, can you take a look? |
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 0c4b1c7)
What do these changes do?
This PR uses the blockbuster library to detect blocking calls made in the asyncio event loop during unit tests.
Avoiding blocking calls is hard as these can be deeply buried in the code or made in 3rd party libraries.
Blockbuster makes it easier to detect them by raising an exception when a call is made to a known blocking function (eg:
time.sleep
).Are there changes in behavior for the user?
No
Is it a substantial burden for the maintainers to support this?
I think no. In 5 years, it's just a few lines to remove if you don't want to use it anymore.
Checklist
CONTRIBUTORS.txt
CHANGES/
folder