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

Check job not in prep already before triggering. #4667

Merged
merged 3 commits into from
Feb 9, 2022

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Feb 9, 2022

These changes close #4665

Don't retrigger a task that is already preparing for job submission.

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.
  • Added test.
  • Appropriate change log entry included.
  • No documentation update required.

@hjoliver hjoliver added the bug Something is wrong :( label Feb 9, 2022
@hjoliver hjoliver added this to the cylc-8.0rc1 milestone Feb 9, 2022
@hjoliver hjoliver self-assigned this Feb 9, 2022
@hjoliver
Copy link
Member Author

hjoliver commented Feb 9, 2022

I'm not sure we can reliably/repeatedly test this. Tasks do not stay in the preparing state for very long. I've tentatively checked "covered by existing tests" because the code line changed is covered 🐑-ish.

@hjoliver
Copy link
Member Author

hjoliver commented Feb 9, 2022

(unrelated flaky test failure)

@oliver-sanders
Copy link
Member

oliver-sanders commented Feb 9, 2022

Looks good, makes sense.

This is the sort of thing we should be able to cover with an integration test, I'll take a look...

... done.

@oliver-sanders oliver-sanders mentioned this pull request Feb 9, 2022
7 tasks
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Confirmed the new test fails on master, passes on this branch

@oliver-sanders oliver-sanders merged commit 4313a73 into cylc:master Feb 9, 2022
@hjoliver hjoliver deleted the fix-4665 branch February 9, 2022 20:26
@hjoliver
Copy link
Member Author

hjoliver commented Feb 9, 2022

Nice test, thanks @oliver-sanders

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging this pull request may close these issues.

trigger: duplicate submit
3 participants