[Fix] Add support for task-like objects without strict asyncio. #843
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ecks Expects to fix agronholm#840 This commit removes the strict `isinstance(key, asyncio.Task)` checks in the TaskStateStore implementation and replaces them with a more flexible approach based on the presence of `get_coro()` and `asyncio.iscoroutine()`. By doing so, we eliminate AssertionErrors that arise when alternative task implementations (e.g., due to `nest_asyncio`, custom factories, or debugger wrappers) return objects that are not instances of `asyncio.Task` but still behave like them. This change significantly improves compatibility with monkey-patched environments and non-standard asyncio task implementations.
…scenarios This commit introduces a series of new tests to verify the robustness of our TaskStateStore logic in various non-standard asyncio environments. - Tests for when `nest_asyncio` is applied, ensuring that task-like objects replaced by nest_asyncio do not trigger AssertionErrors. - Tests for `asyncio.eager_task_factory` usage on Python 3.12+, confirming correct handling of task creation timing differences. - Tests replacing `asyncio.Task` with `_PyTask` and custom task-like objects that expose `get_coro()`, confirming that our new approach gracefully manages these non-standard cases without relying on strict type checks.
for more information, see https://pre-commit.ci
|
This is a duplicate of #841, isn't it? |
sigridjineth
changed the title
sigridjineth
changed the title
|
@agronholm aha, thanks for noticing it. it is fine to close this but I want to clarify that rather than only adjusting the type check, the implementation would be better by introducing some accomodity in the variety of task-like objects (testing for nest_asyncio, custom task factories, debugger level monkey patching) that my intention behind it. |
|
I'm not going to merge this because:
All in all, #841 is a much leaner and more straightforward fix for the issue. |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Instead of asserting isinstance(key, asyncio.Task), the store now relies on the presence of get_coro() and asyncio.iscoroutine() checks. This ensures that objects behaving like tasks (e.g., those provided by nest_asyncio or PyCharm’s debugger) are handled gracefully.
Additioanlly, by removing the strict type checks, this PR improves interoperability with non-standard or monkey-patched asyncio environments, as well as newer Python features like asyncio.eager_task_factory.
Fixes:
current_task()returns an instance of_asyncio.Task#840Checklist
tests/) added which would fail without your patchdocs/, in case of behavior changes or newfeatures)
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:
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.