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

fileinstall: run coros in the background if loop already running #2744

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Dec 6, 2023

One out of the triplet of PRs which closes cylc/cylc-rose#274


  • Allow Rose file installation to be called by code which already has an event loop running by scheduling coroutines to run in the background (i.e. schedule but don't await).
  • The calling code can list these tasks using asyncio.all_tasks() and await them as appropriate.
  • Addresses reinstall + fileinstall: async clash cylc/cylc-rose#274

For testing instructions see the cylc-flow PR.

Comment on lines +24 to +28
# set containing references to "background" coroutines that are not referenced
# from any code (i.e. are not directly awaited), adding them to this list
# avoids the potential for garbage collection to delete them whilst they are
# running
_BACKGROUND_TASKS = set()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that background tasks are always referenced whilst they are running.

That is so long as the module doesn't get housekept 🙈

For details: https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task

@wxtim
Copy link
Contributor

wxtim commented Feb 5, 2024

MyPy Failure is the same as on master!

* Allow Rose file installation to be called by code which already
  has an event loop running by scheduling coroutines to run in the
  background (i.e. schedule but don't await).
* The calling code can list these tasks using `asyncio.all_tasks()`
  and await them as appropriate.
* Addresses cylc/cylc-rose#274
Copy link
Contributor

@wxtim wxtim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Read changes
  • Run tests locally
  • Tried to break various components of this

@MetRonnie MetRonnie merged commit e19f287 into metomi:master Feb 22, 2024
4 of 6 checks passed
@oliver-sanders oliver-sanders deleted the cylc-rose-274 branch February 23, 2024 09:22
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.

reinstall + fileinstall: async clash
3 participants