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

Refactor interactions between JobQueue and LegacyEnsemble. #3144

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

BjarneHerland
Copy link
Contributor

Issue
At least resolves #3023 and probably #2844, but also increases general stability.

Cancellation is now triggered simply by setting the stopped-flag in JobQueue, causing the JobQueue-loop to exit by raising a CancelledError which gets propagated back to the asyncio-eventloop and handled there.

Avoids barrier, thread and event-loop in cancel() and simplifies loop in JobQueue.

jondequinor
jondequinor previously approved these changes Mar 23, 2022
Copy link
Contributor

@jondequinor jondequinor left a comment

Choose a reason for hiding this comment

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

I think this is good for many reasons. Setting us up for #3124, making the code clearer and simpler.

Thanks!

@jondequinor jondequinor dismissed their stale review March 23, 2022 11:07

didn't see the failing tests, will re-visit

@BjarneHerland
Copy link
Contributor Author

jenkins test pytest please

@BjarneHerland
Copy link
Contributor Author

jenkins test this please

@BjarneHerland BjarneHerland force-pushed the refactor_legacy_ensemble branch 2 times, most recently from 815b1b8 to 3e5bd91 Compare March 23, 2022 15:06
Avoids barrier, thread and event-loop in cancel() and simplifies loop
in JobQueue.

Cancellation is now triggered simply by setting the stopped-flag in
JobQueue, causing the JobQueue-loop to exit by raising a CancelledError
which is propagated back to the asyncio-eventloop and handled there.
Copy link
Contributor

@jondequinor jondequinor left a comment

Choose a reason for hiding this comment

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

Very good!

@BjarneHerland BjarneHerland merged commit 32b4350 into equinor:main Mar 23, 2022
@sondreso sondreso added the release-notes:improvement Automatically categorise as improvement in release notes label Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:improvement Automatically categorise as improvement in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flakyness in coverage test:
3 participants