Recommend async alternatives to blocking IO calls

Background

This is going to have a fair bit of overlap with Zac-HD/flake8-async-archive#6 and Zac-HD/flake8-async-archive#9, precisely because I hoped for a framework-agnostic approach. However, I've ultimately changed my mind because Trio-specific checks can be more specific, and apply to cases where there are no generic warnings or recommendations. Using trio.wrap_file(os.fdopen(..)) is an example of both!

Specific recommendations

I think each cluster of these should probably use a separate TRIO2xx code, so they can be individually suppressed - it's useful for incremental adoption at least. Let's reserve TRIO200 for user-supplied recommendations (see below).

HTTP requests: 21X and 22X, blocking sync calls in async function #94
- requests and sync-httpx - as for flake8-async
- urllib3.request(), but not bare request() (too generic) - use equivalent method with an httpx.AsyncClient
- urllib.request.urlopen() (stdlib) - likewise, including request.urlopen() and urlopen()
- httpx.Client() add TRIO212: sync call on httpx.Client, move type-tracking logic #118
Process invocations: 21X and 22X, blocking sync calls in async function #94
- subprocess.Popen and os.popen - await nursery.start(trio.run_process, ...)
- other subprocess functions, os.system - await trio.run_process(...)
- os.spawn[vl]p?e? family - usually as for system; if mode= is given and not [os.]P_WAIT then as for popen.

File IO:

Use trio.open_file() instead of open(), io.open, io.open_code, or os.fdopen in async functions Add TRIO23x, sync IO call in async function #101
Use of blocking methods on file objects - wrap in trio.wrap_file() to get an async file object (see below)
Use of blocking methods on pathlib.Path objects - wrap in trio.Path() to get an async path object (again, see below) Add TRIO232: blocking sync call on file objects #104 trio232 now also handles annotated assignments, and added lots of tests #111

Path manipulation:

Any use of os.path module - prefer using trio.Path objects (it's a copout but good advice IMO) add trio240, usage of os.path in async function #100
TBD: os.walk, os.pidfd_open - don't have exact equivalents on trio.Path? 🤔

OS functionality:

os.wait.*() (except waitstatus_to_exitcode) - must be wrapped in trio.to_thread.run_sync()
os.pidfd_open - must be wrapped in trio.wrap_file() immediately
os.remove, os.unlink - prefer trio.Path methods or wrap in trio.to_thread.run_sync()
...you know what there's too much to list in the os module and it's rare in async code, let's revisit it later.

I haven't observed antipatterns for use of the socket library in sync code and don't want to speculate, but that's one of several potential future categories here, along with various websocket libraries. Note also that this is large enough that it should probably be split into multiple PRs, and leave anything nontrivial for after we get the basics in.

The problem with Path (and file) objects

...

is that they're often passed in as arguments to a function, so all the linter can see is varname.read() - and we can't warn on that without an unacceptable level of false alarms. Of course warning on Path("log.txt").write_text(data) is reasonable, but I think we can do better by checking for arguments which are explicitly annotated as (non-Trio!) Paths and warning on those. This should be left to a future PR, I'm just writing it up here so I can see all the design considerations in one place.

Config option to add simple recommendations

Add TRIO200, user-configured warnings for blocking sync calls in async function #80
It'd be nice if end users could check for blocking uses of nonpublic code, and also nice to try out new warnings without needing to ship an updated version. To that end, let's make each collection of things-to-check extensible where that's easy-ish to do; but to ease maintainence we'll only support configuring the simple patterns for now:

# Warn if we see the thing before ` -> `, and recommend the thing after as a literal string.
# Error out on any entry which doesn't have that delimiter.

trio200-blocking-calls =
    # If you want to support with-or-without library name, list each case.
    # It's verbose but a simple implementation is worth a lot!
    bar -> qux.bar,
    foo.bar -> qux.bar,
    
	# Later: syntax for 'method call' warnings.   (literal `(...)` - empty parens will be an error)
    CustomPath(...).read_text -> AsyncCustomPath(...).read_text

The (...) method-call syntax should also be used from type annotations once we get that working. Complicated logic like "trio.wrap_file(os.fdopen(...)) is OK" just isn't user-configurable.

# If you want to support with-or-without library name, list each case.
# It's verbose but a simple implementation is worth a lot!```

It's actually supporting recursive dotting that's the most complicated part, since that requires recursively stepping through the attribute structure and building up the name, so supporting wildcard isn't gonna complicate it more at all. If anything explicitly not supporting it here means I add a parameter to fnmatch_qualified_name and making that function more complicated :P
But at least until Path/file objects are implemented it feels like a user should be able to configure *.read if they really want to. If you don't want to use wildcards you can just refrain from configuring that, or I suppose one could have a flag to enforce never using *.

EDIT: oh nevermind, you can use ast.unparse to build the name!!
EDIT2: except ast.unparse isn't available in 3.8.... 🙃

Possible future addition to TRIO211 - track the variable names of httpx.Client() and urllib3 pools so the following code can be warned on w/o excessive false alarms:

client = httpx.Client()
client.get('https://example.org')

(Adding it here so it's not forgotten, from discussion in #94)

Edit: This is part of "The problem with Path (and file) objects"

I think I have open PR's for all the simple warnings now, I reformatted the initial post so it was easier to see what is done and what's left.

Prompted by #103 (comment)

The following os functions are checked by flake8-async, and not yet by us:

"posix_spawn",
"posix_spawnp",
"wait",
"wait3",
"wait4",
"waitid",
"waitpid",

So might be time to revisit the os module, or can just implement these ones specifically so that flake8-asyncio is fully redundant. (and once that's done, update __init__.py to remove the reference)

Let's just do those ones to start with, and revisit the rest later 👍

Let's just do those ones to start with, and revisit the rest later +1

Should os.posix_spawn[p] be replaced by await nursery.start(trio.run_proces, ...) or await trio.run_process(...) ?

And what's the replacement for the wait family?

Updated the first post.
User-configured method-call warnings should be relatively straightforward after #118, otherwise I'm starting to be done with everything that's been specified.

I don't actually have a use for user-configured method call warnings at the moment and that sounds pretty complicated, so let's call this one complete as it stands - the remaining os.___ stuff is also rare, and I'd rather get #61 for the benefit of other OSS packages 🙂

With TypeTrackerVisitor200 implementing user-configured method call warnings isn't going to be hard, it's mostly just about parsing the user-specified data and to what complexity. But that's probably better done once there's a need for it.
I'll make a final PR with the os functions warned about in flake8-async so it's fully redundant, but otherwise 🥳

Merge pull request #119 from jakkdl/flake8_async_os_functions

Navigation Menu

Repository navigation

Recommend async alternatives to blocking IO calls #58

Background

Specific recommendations

The problem with Path (and file) objects

Config option to add simple recommendations

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Recommend async alternatives to blocking IO calls #58

Description

Background

Specific recommendations

The problem with Path (and file) objects

Config option to add simple recommendations

Activity

jakkdl commented on Dec 8, 2022

jakkdl commented on Jan 12, 2023

jakkdl commented on Jan 13, 2023

jakkdl commented on Jan 27, 2023

Zac-HD commented on Jan 27, 2023

jakkdl commented on Jan 31, 2023

jakkdl commented on Jan 31, 2023

Zac-HD commented on Feb 1, 2023

jakkdl commented on Feb 1, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions