moar asyncio support, test infra improvements #213

jakkdl · 2024-03-04T11:15:37Z

Add asyncio-specific error messages for 2xx.
Add check in tests to detect if [library]_NO_ERROR should be used, add it to several checks.
Specify why NO_[library] and [library]_NO_ERROR is specified in a lot of eval files.

Add check in tests to detect if [library]_NO_ERROR should be used, add it to several checks. Specify why NO_[library] and [library]_NO_ERROR is specified in a lot of eval files.

flake8_async/base.py

jakkdl · 2024-03-04T11:19:03Z

flake8_async/visitors/visitor2xx.py

+        "ASYNC220_asyncio": (
+            "Sync call {} in async function, use "
+            "`asyncio.create_subprocess_[exec/shell]."
+        ),
        "ASYNC221": "Sync call {} in async function, use `await {}.run_process(...)`.",
+        "ASYNC221_asyncio": (
+            "Sync call {} in async function, use "
+            "`asyncio.create_subprocess_[exec/shell]."
+        ),
        "ASYNC222": "Sync call {} in async function, wrap in `{}.to_thread.run_sync()`.",
+        "ASYNC222_asyncio": (
+            "Sync call {} in async function, use `asyncio.loop.run_in_executor`."
+        ),


educated guesses from reading asyncio docs:
https://docs.python.org/3/library/asyncio-dev.html#asyncio-multithreading
https://docs.python.org/3/library/asyncio-subprocess.html

jakkdl · 2024-03-04T11:20:23Z

flake8_async/visitors/visitors.py

-            nursery = get_matching_call(item.context_expr, "open_nursery")
+            nursery = get_matching_call(
+                item.context_expr, "open_nursery", base=("trio",)
+            )


nurseries only exist in trio. Could expand the check to work with anyio taskgroups though

Let's leave that for a future PR, but yes - it'd be great for the ecosystem to support this for anyio.TaskGroup, and at that point we might as well support asyncio.TaskGroup too.

tests/autofix_files/noqa_testing.py

tests/test_flake8_async.py

jakkdl · 2024-03-04T11:39:52Z

README.md

+- **ASYNC220**: Sync process call in async function, use `await nursery.start([trio|anyio].run_process, ...)`. `asyncio` users can use [`asyncio.create_subprocess_[exec/shell]`](https://docs.python.org/3/library/asyncio-subprocess.html).
+- **ASYNC221**: Sync process call in async function, use `await [trio|anyio].run_process(...)`. `asyncio` users can use [`asyncio.create_subprocess_[exec/shell]`](https://docs.python.org/3/library/asyncio-subprocess.html).


ugh, starting to feel like we might consider generating proper docs with interlinks and stuff.

Let's open an issue for that.

Zac-HD

Merge when you feel it's ready!

Zac-HD · 2024-03-04T19:23:57Z

flake8_async/visitors/visitors.py

-            nursery = get_matching_call(item.context_expr, "open_nursery")
+            nursery = get_matching_call(
+                item.context_expr, "open_nursery", base=("trio",)
+            )


Let's leave that for a future PR, but yes - it'd be great for the ecosystem to support this for anyio.TaskGroup, and at that point we might as well support asyncio.TaskGroup too.

Zac-HD · 2024-03-04T19:26:14Z

tests/eval_files/async101.py

@@ -1,4 +1,4 @@
-# NOASYNCIO
+# ASYNCIO_NO_ERROR - no nursery/cancelscope in asyncio


See above for a future PR; there is asyncio.TaskGroup.

I also suspect that async with timeout() is doing something similar? But at this point users really just have to switch to structured concurrency if they want sane error handling.

Oh right right, trio.[move_on/fail]_[after/at] are just wrapper functions for cancelscopes.
It might still be good (and fairly easy) to raise errors for an asyncio user doing it, even if our real suggestion is for them to switch to anyio.

Secret plan is for asyncio users to start running our checks, then when getting tons of useful errors realize they should switch to anyio 😈

Zac-HD · 2024-03-04T19:27:29Z

tests/eval_files/async111.py

-# NOASYNCIO
+# ASYNC111: Variable, from context manager opened inside nursery, passed to start[_soon] might be invalidly accessed while in use, due to context manager closing before the nursery. This is usually a bug, and nurseries should generally be the inner-most context manager.
+# It's possible there's an equivalent asyncio construction/gotcha, but methods are differently named, so this file will not raise any errors
+# ASYNCIO_NO_ERROR # no nurseries in asyncio. Though maybe the bug is relevant for TaskGroups


Plausibly, yeah.

Zac-HD · 2024-03-04T19:29:35Z

tests/eval_files/async113_trio.py

@@ -11,7 +11,9 @@
 import trio

 # ARG --startable-in-context-manager=custom_startable_function
-# NOANYIO
+
+# ANYIO_NO_ERROR - anyio uses TaskGroups, not nurseries


We should support anyio for this though, open an issue?

This one, as opposed to 101, 111, and 112 is supported. Updated comment to refer to async113.py which tests anyio functionality.

Zac-HD · 2024-03-04T21:20:07Z

tests/eval_files/async232_asyncio.py

+async def file_binary_write(f: io.BufferedWriter):
+    f.read()  # ASYNC232_asyncio: 4, 'read', 'f'


f.write()? And below for the bare name.

This is tested by row 16/17, any call on a file object will trigger the error. Added clarifying comment and a more obvious nonsense-name example.

(pretty much all your comments in this vein make me go "yeah duh", change it, then after a minute realize/remember the reason for the status quo 😅)

Zac-HD · 2024-03-04T21:21:45Z

tests/eval_files/async23x_asyncio.py

+    # asyncio.wrap_file does not exist
+    await asyncio.wrap_file(open(""))  # ASYNC230_asyncio: 28, 'open'
+    await asyncio.wrap_file(os.fdopen(0))  # ASYNC231_asyncio: 28, 'os.fdopen'


Doesn't make sense to test this if it doesn't exist, IMO.

The test is to make sure the protection that wrap_file() would give doesn't trigger. Not that it would matter much since it doesn't exist, but I constructed the file by copypasting and replacing - so /shrug. I'll clarify the comment

Zac-HD · 2024-03-04T21:24:37Z

README.md

+- **ASYNC220**: Sync process call in async function, use `await nursery.start([trio|anyio].run_process, ...)`. `asyncio` users can use [`asyncio.create_subprocess_[exec/shell]`](https://docs.python.org/3/library/asyncio-subprocess.html).
+- **ASYNC221**: Sync process call in async function, use `await [trio|anyio].run_process(...)`. `asyncio` users can use [`asyncio.create_subprocess_[exec/shell]`](https://docs.python.org/3/library/asyncio-subprocess.html).


Let's open an issue for that.

* split off anyio_create_task_group testing into async113_anyio.py * improved comments

Add asyncio-specific error messages for 2xx.

a57b82b

Add check in tests to detect if [library]_NO_ERROR should be used, add it to several checks. Specify why NO_[library] and [library]_NO_ERROR is specified in a lot of eval files.

jakkdl commented Mar 4, 2024

View reviewed changes

jakkdl added 2 commits March 4, 2024 12:26

minor fixes after self-review

f6b82f3

update error codes in readme

5297f4b

jakkdl commented Mar 4, 2024

View reviewed changes

Merge branch 'main' into more_asyncio_support

30f969d

Zac-HD approved these changes Mar 4, 2024

View reviewed changes

jakkdl added 4 commits March 5, 2024 10:58

* added 113 support for asyncio

63894e0

* split off anyio_create_task_group testing into async113_anyio.py * improved comments

fix typos

28c9bc1

update __version__

942cc12

oops, you specify patch, not day, in CalVer

00b637d

jakkdl merged commit 8b1a1cf into python-trio:main Mar 8, 2024
9 checks passed

jakkdl deleted the more_asyncio_support branch March 8, 2024 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

moar asyncio support, test infra improvements #213

moar asyncio support, test infra improvements #213

jakkdl commented Mar 4, 2024 •

edited

Loading

jakkdl Mar 4, 2024 •

edited

Loading

jakkdl Mar 4, 2024

Zac-HD Mar 4, 2024

jakkdl Mar 4, 2024

Zac-HD Mar 4, 2024

Zac-HD left a comment

Zac-HD Mar 4, 2024

Zac-HD Mar 4, 2024 •

edited

Loading

jakkdl Mar 5, 2024

Zac-HD Mar 4, 2024

Zac-HD Mar 4, 2024

jakkdl Mar 5, 2024

Zac-HD Mar 4, 2024

jakkdl Mar 5, 2024

Zac-HD Mar 4, 2024

jakkdl Mar 5, 2024

Zac-HD Mar 4, 2024

		- ASYNC220: Sync process call in async function, use `await nursery.start([trio\|anyio].run_process, ...)`. `asyncio` users can use [`asyncio.create_subprocess_[exec/shell]`](https://docs.python.org/3/library/asyncio-subprocess.html).
		- ASYNC221: Sync process call in async function, use `await [trio\|anyio].run_process(...)`. `asyncio` users can use [`asyncio.create_subprocess_[exec/shell]`](https://docs.python.org/3/library/asyncio-subprocess.html).

		@@ -1,4 +1,4 @@
		# NOASYNCIO
		# ASYNCIO_NO_ERROR - no nursery/cancelscope in asyncio

		async def file_binary_write(f: io.BufferedWriter):
		f.read() # ASYNC232_asyncio: 4, 'read', 'f'

moar asyncio support, test infra improvements #213

moar asyncio support, test infra improvements #213

Conversation

jakkdl commented Mar 4, 2024 • edited Loading

jakkdl Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zac-HD left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zac-HD Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakkdl commented Mar 4, 2024 •

edited

Loading

jakkdl Mar 4, 2024 •

edited

Loading

Zac-HD Mar 4, 2024 •

edited

Loading