restore support for yield unsafe Client context managers and deprecate that support #7987
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 20 files ± 0 20 suites ±0 11h 34m 43s ⏱️ + 4m 23s For more details on these failures, see this check. Results for commit 5952e30. ± Comparison against base commit 6c37907. ♻️ This comment has been updated with latest results. |
graingert
force-pushed
the
restore-support-for-yield-unsafe-client-context
branch
from
f157a1c
to
5952e30
Compare
graingert
changed the title
distributed/client.py
Outdated
| _current_client.reset(self._previous_as_current) | ||
| except ValueError as e: | ||
| if not e.args[0].endswith(" was created in a different Context"): | ||
| raise |
There was a problem hiding this comment.
Would it make sense to add # pragma: nocover or add a test?
| raise | ||
| warnings.warn( | ||
| "It is deprecated to enter and exit the Client context " | ||
| "manager from different tasks", |
There was a problem hiding this comment.
Can we phrase this in a way that tells the user how to replace their anti-pattern?
There was a problem hiding this comment.
I'm not sure how to phrase that, it's usually because someone has used async with Client( inside an async generator, the specific case it's likely to occur is in pytest-asyncio where async fixtures are not run in a way that can support ContextVars, TaskGroups and Timeouts
related discussion is here: https://discuss.python.org/t/preventing-yield-inside-certain-context-managers/1091
There was a problem hiding this comment.
Yeah, I don't have any suggestions, either. Let's see if this pops up frequently then and add more context/a link to some Github discussion if need be.
| if not e.args[0].endswith(" was created in a different Context"): | ||
| raise | ||
| warnings.warn( | ||
| "It is deprecated to enter and exit the Client context " |
distributed/client.py
Outdated
| _current_client.reset(self._previous_as_current) | ||
| except ValueError as e: | ||
| if not e.args[0].endswith(" was created in a different Context"): | ||
| raise |
| raise | ||
| warnings.warn( | ||
| "It is deprecated to enter and exit the Client context " | ||
| "manager from different tasks", |
There was a problem hiding this comment.
Yeah, I don't have any suggestions, either. Let's see if this pops up frequently then and add more context/a link to some Github discussion if need be.
Closes #7984
pre-commit run --all-files