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.

Sign up for GitHub

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

Jump to bottom

Back-compat: fix self-suicide: a:fail => !a #4970

Merged
merged 8 commits into from
Jul 7, 2022
Merged

Back-compat: fix self-suicide: a:fail => !a #4970

merged 8 commits into from
Jul 7, 2022

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Jul 6, 2022 •
edited
Loading

These changes close #4968

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • No documentation update required.

@hjoliver hjoliver added the bug Something is wrong :( label Jul 6, 2022
@hjoliver hjoliver added this to the cylc-8.0.0 milestone Jul 6, 2022
@hjoliver hjoliver self-assigned this Jul 6, 2022
@hjoliver
Copy link
Member Author

hjoliver commented Jul 7, 2022

OK the fixes didn't break any existing tests 😅 ... now to add a couple of new tests.

@wxtim
Copy link
Member

wxtim commented Jul 7, 2022

On a purely pragmatic test - this appears to fix the workflow of the person who brought the issue to our attention.

wxtim
wxtim reviewed Jul 7, 2022
View reviewed changes
cylc/flow/task_pool.py Outdated Show resolved Hide resolved
wxtim
wxtim approved these changes Jul 7, 2022
View reviewed changes
Copy link
Member

@wxtim wxtim left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • Read Change (Made a couple of suggestions about documentation.
  • Tests seem to cover the matter, and pass!
  • Works on my copy of the customer's workflow.

hjoliver and others added 4 commits July 7, 2022 22:26
@hjoliver @wxtim
Update cylc/flow/task_pool.py
0cef1b2 
Co-authored-by: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
@hjoliver @MetRonnie
Update tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/…
518ffed 
…suite.rc

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
@hjoliver @MetRonnie
Update tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-…
549299b 
…cycling/suite.rc

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
@hjoliver
Add a comment.
d3680ff
@MetRonnie MetRonnie merged commit 5e7ffad into cylc:master Jul 7, 2022
@hjoliver hjoliver deleted the fix-4968 branch July 18, 2022 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MetRonnie MetRonnie MetRonnie approved these changes

@wxtim wxtim wxtim approved these changes

@dpmatthews dpmatthews Awaiting requested review from dpmatthews

Assignees

@hjoliver hjoliver

Labels
bug Something is wrong :(
Projects
None yet
Milestone
cylc-8.0.0
Development

Successfully merging this pull request may close these issues.

Suicide trigger not handled correctly
3 participants
@hjoliver @wxtim @MetRonnie