Skip to content

basic Asyncio support - #210

Merged
jakkdl merged 8 commits into
python-trio:mainfrom
jakkdl:asyncio_support
Mar 2, 2024
Merged

jakkdl merged 8 commits into
python-trio:mainfrom
jakkdl:asyncio_support

Conversation

@jakkdl

@jakkdl jakkdl commented Feb 26, 2024 •

Copy link
Copy Markdown
Member

first steps on #208
106, 200, 21x and 9xx are supported, though no autofix.

This is rebased on top of #209, so if reviewing you should only look at the latest commit(s).

todo:

  • update readme.md
  • update changelog
  • do a second pass through the diff, quite tired atm so suspect there may be silly stuff somewhere.
  • rewrite descriptions of --anyio / --asyncio to not be dumb. And/or have a --library flag instead.

@Zac-HD

Zac-HD commented Feb 26, 2024

Copy link
Copy Markdown
Member

Looks good so far! Will re-review when you think it's done 👍

Comment thread tests/test_flake8_async.py

@Zac-HD Zac-HD left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's rebase on master, add ASYNC102 support, and then we're probably done?

Comment thread flake8_async/visitors/visitor_utility.py Outdated
jakkdl added 5 commits March 1, 2024 16:51
…ssing on asyncio, though not necessarily for good reasons
…rk with which library, and minor updates. 102/103/104 now sees asyncio.exceptions.CancelledError as a critical exception. Fix import/arg detection of asyncio. add asyncio102_asyncio. add async103_all_imported. Finally make test_anyio_from_config autodetect the correct line number. Fix BASE_LIBRARY marker being interpreted as a bool. Make #NOTRIO/#NOASYNCIO/#NOANYIO run the visitor but ignore the result, instead of skipping, to check it doesn't crash. Generalize error-message-library-check.
@jakkdl
jakkdl force-pushed the asyncio_support branch from bf59dd6 to ced4570 Compare March 1, 2024 15:52
Comment thread flake8_async/visitors/visitor_utility.py
@jakkdl

jakkdl commented Mar 1, 2024

Copy link
Copy Markdown
Member Author

Uh, I ended up doing A Lot:tm:. But the diff didn't actually end up being all that big. The rebase might break viewing only new changes though, so you might want to review commit-by-commit.

Only ones listed in #208 not supported now is:

  • 22X/23X/24X not having tailored error messages (and therefore being untested)
  • not detecting asyncio-shielded-timeouts

So I'm now comfortable saying this has decent asyncio support and we could push a release.

TODO: Oh I did rename the repository, so should update links.

@Zac-HD Zac-HD left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM; agree that we could release this and should release whatever we have on the weekend (when I set up release automation again).

I'll let you merge after any docs/links updates you want to make.

Comment thread tests/test_config_and_args.py Outdated
jakkdl and others added 2 commits March 2, 2024 13:45
Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>
@jakkdl
jakkdl merged commit 5c6879a into python-trio:main Mar 2, 2024
@jakkdl
jakkdl deleted the asyncio_support branch March 2, 2024 13:25
@jakkdl

jakkdl commented Mar 2, 2024

Copy link
Copy Markdown
Member Author

ah oops, I realized I forgot to ask your opinion on use [trio|anyio].lowlevel.checkpoint() vs use [trio/anyio].lowlevel.checkpoint() (or something else). Both are used in different places in documentation/comments/error messages

@Zac-HD

Zac-HD commented Mar 2, 2024

Copy link
Copy Markdown
Member

I'd probably go with the slash, but don't really care so long as we're consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants