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

Fix: Use flow numbers to map jobs to tasks in 8.1.x workflows. #5309

Merged

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Jan 18, 2023

Closes #5247

  • Allow Cylc Review to see all jobs with different flow nums for Cylc 8.1 workflows
  • Warn that not all jobs for different flows will be visible for Cylc 8.0 workflows

Checklist

  • 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.
  • Manually tested - it's legacy software
  • CHANGES.md entry included if this is a change that can affect users

- Handle exception and warn user for 8.0.x workflows.
- Carry on as before for 7.x workflows.
@wxtim wxtim requested a review from datamel January 18, 2023 09:31
@wxtim wxtim self-assigned this Jan 18, 2023
@wxtim wxtim added this to the cylc-7.8.x milestone Jan 18, 2023
@wxtim wxtim added the bug Something is wrong :( label Jan 18, 2023
@wxtim
Copy link
Member Author

wxtim commented Jan 18, 2023

@datamel

  • Please select a second reviewer
  • Note that I have test workflows for you to look at using you example from the issue at myaccount/x2/run1 and run2.

@wxtim wxtim linked an issue Jan 18, 2023 that may be closed by this pull request
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

Problems are fixed for me. Super thanks for the demo workflows. I have un-reproducible traceback which can be seen in my workflows:trigger-demo-new/run1.Think it is related to it being an old stale one, maybe worth a bit of defensive code?
A slightly separate issue which I noticed which may or may not be worth fixing is the job # file has 1 of 1, think this should be 1 of 2? ....
image

lib/cylc/cylc-review/static/css/cylc-review.css Outdated Show resolved Hide resolved
lib/cylc/review.py Show resolved Hide resolved
Co-authored-by: Melanie Hall <37735232+datamel@users.noreply.github.com>
@MetRonnie MetRonnie requested review from MetRonnie and removed request for oliver-sanders February 21, 2023 10:46
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.

Tested out

@MetRonnie MetRonnie merged commit 6e829df into cylc:7.8.x Feb 21, 2023
@hjoliver hjoliver modified the milestones: cylc-7.8.x, 7.8.14 and 7.9.9 Jan 15, 2024
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.

review: Mapping Task Jobs back to Tasks
5 participants