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

Fixed AssertionError when using nest_asyncio #841

Merged
merged 6 commits into from
Dec 17, 2024
Merged

Fixed AssertionError when using nest_asyncio #841

merged 6 commits into from
Dec 17, 2024

Conversation

agronholm
Copy link
Owner

@agronholm agronholm commented Dec 15, 2024

Changes

Refactored TaskStateStore to not check explicitly against asyncio.Task, as nest-asyncio replaces the asyncio modules.

Fixes #840.

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

  • You've added tests (in tests/) added which would fail without your patch
  • You've updated the documentation (in docs/, in case of behavior changes or new
    features)
  • You've added a new changelog entry (in docs/versionhistory.rst).

If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.

Updating the changelog

If there are no entries after the last release, use **UNRELEASED** as the version.
If, say, your patch fixes issue #123, the entry should look like this:

- Fix big bad boo-boo in task groups
  (`#123 <https://github.com/agronholm/anyio/issues/123>`_; PR by @yourgithubaccount)

If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.

This stems from the incorrect placement of `nest_asyncio.apply()`, as it should be called before `asyncio.run()`.

Fixes #840.
@agronholm
Copy link
Owner Author

I don't know how to write a test for this, as nest_asyncio.apply() is too invasive to be used in the test suite.

@graingert
Copy link
Collaborator

you could try with:

with mock.patch("asyncio.Task", asyncio.tasks._PyTask):
    ...

@agronholm
Copy link
Owner Author

I've added a test and verified that it fails without this patch.

@edgarrmondragon
Copy link

I can confirm that this patch fixes the problem for us

@agronholm agronholm requested a review from graingert December 16, 2024 14:36
@sigridjineth
Copy link

@agronholm I just have came across this, but would be better implementing the logic for accomodating some varities of task-like objects I think. what's your opinion?

@agronholm
Copy link
Owner Author

@agronholm I just have came across this, but would be better implementing the logic for accomodating some varities of task-like objects I think. what's your opinion?

If you find a problem not fixed by this PR, let me know. Otherwise I'm not inclined to add any more code just because.

@agronholm agronholm merged commit c518300 into master Dec 17, 2024
16 checks passed
@agronholm agronholm deleted the fix-840 branch December 17, 2024 22:07
@sandangel
Copy link

Hi, thank you so much for providing the fix for this issue. I wonder if we can create a release for this so I can download?

@agronholm
Copy link
Owner Author

I'm hoping to tackle #805 and #847 and then cut a new release. If those take too long, however, I'll just defer them.

@sandangel
Copy link

Please take your time. I pin to 4.6.2 at the moment.

@agronholm
Copy link
Owner Author

Please take your time. I pin to 4.6.2 at the moment.

All done now; v4.8.0 is out.

@sandangel
Copy link

awesome. Thank you so much.

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.

Assertion error when current_task() returns an instance of _asyncio.Task
5 participants