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

ASYNC100 does not match upstream flake8-async #12873

Closed
Zac-HD opened this issue Aug 14, 2024 · 1 comment · Fixed by #12896
Closed

ASYNC100 does not match upstream flake8-async #12873

Zac-HD opened this issue Aug 14, 2024 · 1 comment · Fixed by #12896
Labels
bug Something isn't working help wanted Contributions especially welcome

Comments

@Zac-HD
Copy link

Zac-HD commented Aug 14, 2024

ASYNC100 (ruff, upstream) warns when a timeout context manager doesn't contain any checkpoints - and therefore can't be cancelled by the async framework. However, if you're implementing a context manager, then it's entirely reasonable to wrap such a timeout around a yield, and we should treat that as a checkpoint - as in python-trio/flake8-async#228

(if you wrap a timeout around the yield in a generator, see PEP-789 and the ASYNC9xx rules, with my condolences)

from contextlib import asynccontextmanager
import anyio

async def bad_code():
    with anyio.fail_after(10):
        do_something_slow()  # can't cancel non-async work!


@asynccontextmanager
async def good_code():
    with anyio.fail_after(10):
        # There's no await keyword here, but we presume that there
        # will be in the caller we yield to, so this is safe.
        yield

The current version of Ruff will warn on both fail_after() contexts, but only the former is problematic.

See also: #8451

@charliermarsh
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Contributions especially welcome
Projects
None yet
3 participants