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

basic Asyncio support #210

Merged
merged 8 commits into from
Mar 2, 2024
Merged

basic Asyncio support #210

merged 8 commits into from
Mar 2, 2024

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented Feb 26, 2024

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
Copy link
Member

Zac-HD commented Feb 26, 2024

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

Copy link
Member

@Zac-HD Zac-HD left a comment

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?

flake8_async/visitors/visitor_utility.py Outdated Show resolved Hide resolved
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
@jakkdl
Copy link
Member Author

jakkdl commented Mar 1, 2024

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.

Copy link
Member

@Zac-HD Zac-HD left a comment

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.

tests/test_config_and_args.py Outdated Show resolved Hide resolved
@jakkdl jakkdl merged commit 5c6879a into python-trio:main Mar 2, 2024
9 checks passed
@jakkdl jakkdl deleted the asyncio_support branch March 2, 2024 13:25
@jakkdl
Copy link
Member Author

jakkdl commented Mar 2, 2024

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
Copy link
Member

Zac-HD commented Mar 2, 2024

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