-
-
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
Replace sync open()
calls within async functions with aiofiles.open()
in code examples
#7383
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7383 +/- ##
=======================================
Coverage 97.28% 97.28%
=======================================
Files 106 106
Lines 31440 31440
Branches 3932 3932
=======================================
Hits 30585 30585
Misses 650 650
Partials 205 205
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
This looks quite invasive and combines several separate pointless changes. For example, adding a dependency that is not actually used in the library.
Also, ruff can only be added to pre-commit, not a random workflow that only multiplies the maintenance burden.
I'm 👎 on the change as it is. Some small bits could be added if discussed and implemented correctly, but not as a bunch of commits that have no visible purpose.
e027530
to
aedb0bb
Compare
open()
calls within async functions with aiofiles.open()
open()
calls within async functions with aiofiles.open()
open()
calls within async functions with aiofiles.open()
in code examples
95b11ef
to
5ff6a7f
Compare
% [`ruff --select=ASYNC .`](https://beta.ruff.rs/docs/rules/open-sleep-or-subprocess-in-async-function) 2 ASYNC101 [ ] Async functions should not call `open`, `time.sleep`, or `subprocess` methods
Not sure why the change to examples has disappeared? |
I removed it because the current CI requires adding both |
%
ruff --select=ASYNC --statistics .
2 ASYNC101 [ ] Async functions should not call
open
,time.sleep
, orsubprocess
methods%
ruff rule ASYNC101
open-sleep-or-subprocess-in-async-function (ASYNC101)
Derived from the flake8-async linter.
What it does
Checks that async functions do not contain calls to
open
,time.sleep
, orsubprocess
methods.Why is this bad?
Blocking an async function via a blocking call will block the entire event loop, preventing it from executing other tasks while waiting for the call to complete, negating the benefits of asynchronous programming.
Instead of making a blocking call, use an equivalent asynchronous library or function.
Example
Use instead:
What do these changes do?
Are there changes in behavior for the user?
Related issue number
Checklist
CONTRIBUTORS.txt
CHANGES
folder<issue_id>.<type>
for example (588.bugfix)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not interesting to users.