Skip to content

Enable ruff ASYNC rules #7441

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.

Sign up for GitHub

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

Open
danielhollas wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Enable ruff ASYNC rules #7441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9795c1
Enable ruff ASYNC rules
danielhollas Nov 5, 2025
58b6adb
Merge branch 'main' into ruff-async
danielhollas Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,8 @@ quote-style = 'single'

[tool.ruff.lint]
ignore = [
'ASYNC240', # blocking-path-method-in-async-function

@danielhollas danielhollas Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What it does

Checks that async functions do not call blocking os.path or pathlib.Path
methods.

Why is this bad?

Calling some os.path or pathlib.Path methods in an async function will block
the entire event loop, preventing it from executing other tasks while waiting
for the operation. This negates the benefits of asynchronous programming.

Instead, use the methods' async equivalents from trio.Path or anyio.Path.

@danielhollas danielhollas Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In order to resolve ASYNC230 and ASYNC240 we would need to take a dependency on a third party library that enables async file IO.

'ASYNC230', # blocking-open-call-in-async-function
'F403', # Star imports unable to detect undefined names
'F405', # Import may be undefined or defined from star imports
'PLC0415', # import-outside-top-level
Expand All @@ -505,6 +507,7 @@ select = [
'UP', # pyupgrade
'I', # isort
'N', # pep8-naming
'ASYNC', # flake8-async
'PLC', # pylint-convention
'PLE', # pylint-error
'PLR', # pylint-refactor
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/engine/processes/futures.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def cleanup(self) -> None:
async def _poll_process(self, node: Node, poll_interval: int | float) -> None:
"""Poll whether the process node has reached a terminal state."""
print('polling', node)
while not self.done() and not node.is_terminated:
while not self.done() and not node.is_terminated: # noqa: ASYNC110
await asyncio.sleep(poll_interval)

if not self.done():
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/transports/plugins/ssh_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ async def open_async(self):
raise InvalidOperation('Cannot open the transport twice')

if self.auth_script != 'None':
result = subprocess.run(self.auth_script, shell=True, capture_output=True, text=True, check=False)
result = subprocess.run(self.auth_script, shell=True, capture_output=True, text=True, check=False) # noqa: ASYNC221

@danielhollas danielhollas Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This looks like a real issue that should be fixed. Quoting from ruff:

ASYNC221

Checks that async functions do not run processes with blocking 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, like trio.run_process() or anyio.run_process().

Example

import subprocess


async def foo():
    subprocess.run(cmd)

Use instead:

import asyncio


async def foo():
    asyncio.create_subprocess_shell(cmd)

@agoscinski agoscinski Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think in general to asynchronize this function is good direction for further performance but I don't think it is safe to suspend within this open coroutine without any further changes. In this code we are already pass the if self._is_open: check, so the next coroutine could be another open and open another connection. So we would need a variable expressing that a connection is in the progress of being opened that need to be checked. In 2.10 we anyway wanted to make more functions async, so I think about it after release.

@coderabbitai coderabbitai Bot Jul 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -A10 -B10 'def validate_script|authentication_script|auth_script' src tests

Repository: aiidateam/aiida-core

Length of output: 12873

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== surrounding code =="
sed -n '1,220p' src/aiida/transports/plugins/ssh_async.py

echo
echo "== similar subprocess usage =="
rg -n "subprocess\.run\(" src/aiida/transports -g '*.py'

Repository: aiidateam/aiida-core

Length of output: 9775

Avoid shell=True for authentication_script.
validate_script only checks that this is an absolute, executable path; it still passes the raw string to a shell, so a path containing shell metacharacters can run extra commands. Use subprocess.run([self.auth_script], ...) if this option is path-only.

🧰 Tools
🪛 OpenGrep (1.25.0)

[ERROR] 171-171: Dynamic command passed to subprocess with shell=True. Use a command list without shell=True, or use shlex.quote() to sanitize input.

(coderabbit.command-injection.python-shell-true)

🪛 Ruff (0.15.21)

[error] 171-171: subprocess call with shell=True identified, security issue

(S602)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/aiida/transports/plugins/ssh_async.py` at line 171, Update the subprocess
invocation in the authentication flow to pass auth_script as a single executable
argument rather than using shell=True. Preserve the existing capture_output,
text, and check behavior, and rely on validate_script’s path-only validation.

Source: Linters/SAST tools

if result.returncode != 0:
self.async_backend.logger.error(
f'Authentication script {self.auth_script} failed with exit code {result.returncode}\n'
Expand Down
2 changes: 1 addition & 1 deletion tests/engine/test_daemon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@


async def reach_waiting_state(process):
while process.state != ProcessState.WAITING:
while process.state != ProcessState.WAITING: # noqa: ASYNC110
await asyncio.sleep(0.1)


Expand Down
6 changes: 3 additions & 3 deletions tests/engine/test_rmq.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_pause(self):

async def do_pause():
calc_node = self.runner.submit(test_processes.WaitProcess)
while calc_node.process_state != ProcessState.WAITING:
while calc_node.process_state != ProcessState.WAITING: # noqa: ASYNC110
await asyncio.sleep(0.1)

assert not calc_node.paused
Expand All @@ -107,7 +107,7 @@ def test_pause_play(self):
async def do_pause_play():
calc_node = self.runner.submit(test_processes.WaitProcess)
assert not calc_node.paused
while calc_node.process_state != ProcessState.WAITING:
while calc_node.process_state != ProcessState.WAITING: # noqa: ASYNC110
await asyncio.sleep(0.1)

pause_message = 'Take a seat'
Expand Down Expand Up @@ -139,7 +139,7 @@ def test_kill(self):
async def do_kill():
calc_node = self.runner.submit(test_processes.WaitProcess)
assert not calc_node.is_killed
while calc_node.process_state != ProcessState.WAITING:
while calc_node.process_state != ProcessState.WAITING: # noqa: ASYNC110
await asyncio.sleep(0.1)

kill_message = 'Sorry, you have to go mate'
Expand Down
6 changes: 3 additions & 3 deletions tests/engine/test_zeromq.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def test_pause(self):

async def do_pause():
calc_node = self.runner.submit(test_processes.WaitProcess)
while calc_node.process_state != ProcessState.WAITING:
while calc_node.process_state != ProcessState.WAITING: # noqa: ASYNC110

@danielhollas danielhollas Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I suspect that in order to avoid sleeping these tests could be refactored to use the plumpy.process_listener.ProcessListener callback based solution? But I haven't gone deep enough to see if that is viable here.

https://plumpy.readthedocs.io/en/latest/apidoc/plumpy.process_listener.html

await asyncio.sleep(0.1)

assert not calc_node.paused
Expand All @@ -107,7 +107,7 @@ def test_pause_play(self):
async def do_pause_play():
calc_node = self.runner.submit(test_processes.WaitProcess)
assert not calc_node.paused
while calc_node.process_state != ProcessState.WAITING:
while calc_node.process_state != ProcessState.WAITING: # noqa: ASYNC110
await asyncio.sleep(0.1)

pause_message = 'Take a seat'
Expand Down Expand Up @@ -136,7 +136,7 @@ def test_kill(self):
async def do_kill():
calc_node = self.runner.submit(test_processes.WaitProcess)
assert not calc_node.is_killed
while calc_node.process_state != ProcessState.WAITING:
while calc_node.process_state != ProcessState.WAITING: # noqa: ASYNC110
await asyncio.sleep(0.1)

kill_message = 'Sorry, you have to go mate'
Expand Down
Loading