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

Move _set_running_loop up the stack #599

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dandavison
Copy link
Contributor

@dandavison dandavison commented Aug 1, 2024

This is a draft PR, investigating where we can / should set/unset the running loop.

Currently it's set/unset on entry/exit of _run_once(). Is the purpose of that to prevent any execution of workflow coroutines at any other time during activate()?

If so: CI is green when we do the setting/unsetting at the top level in activate(): it would be great to have a failing test demonstrating the necessity of confining the execution to _run_once().

@dandavison dandavison marked this pull request as ready for review August 1, 2024 14:58
@dandavison dandavison requested a review from a team as a code owner August 1, 2024 14:58
@dandavison dandavison marked this pull request as draft August 1, 2024 17:42
@cretz
Copy link
Member

cretz commented Aug 5, 2024

I do not have a strong preference here. It was originally done because I expected all real workflow-context work to happen inside a task. I didn't want any code not running in an asyncio task to have a loop. In CPython they set the loop on the thread on asyncio.run or loop.run_forever or whatever which makes sense to me. If Python had a loop.step or something, I'd expect it to set and unset the running loop within that too, and _run_once is meant to just that missing Python library method.

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