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

Fix workflow-state alternate run-dir #6031

Merged
merged 10 commits into from
Apr 4, 2024
Merged

Fix workflow-state alternate run-dir #6031

merged 10 commits into from
Apr 4, 2024

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Mar 21, 2024 •
edited
Loading

Close #6030

Targeting 8.3.0 because it seems we're unlikely to get another 8.2 release out before then, and 8.2.x has some conflicting changes. (Can rebase to 8.2.x if really necessary <--- done).

TODO:

  • CLI: cylc workflow-state
  • workflow_state xtrigger

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 if present).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@hjoliver hjoliver added the bug Something is wrong :( label Mar 21, 2024
@hjoliver hjoliver added this to the cylc-8.3.0 milestone Mar 21, 2024
@hjoliver hjoliver self-assigned this Mar 21, 2024
@hjoliver hjoliver mentioned this pull request Mar 29, 2024
Closed
7 tasks
@hjoliver hjoliver force-pushed the fix-workflow-state-alt-run-dir branch from e6e5ee8 to 1712460 Compare April 2, 2024 05:45
@hjoliver hjoliver marked this pull request as ready for review April 2, 2024 05:49
dwsutherland
dwsutherland approved these changes Apr 2, 2024
View reviewed changes
Copy link
Member

@dwsutherland dwsutherland left a comment

Choose a reason for hiding this comment

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

LGTM 👍

cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
MetRonnie
MetRonnie reviewed Apr 2, 2024
View reviewed changes
cylc/flow/id_cli.py Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
@hjoliver hjoliver force-pushed the fix-workflow-state-alt-run-dir branch from e04b168 to e17337e Compare April 3, 2024 00:44
@hjoliver hjoliver changed the base branch from master to 8.2.x April 3, 2024 00:44
@hjoliver hjoliver modified the milestones: cylc-8.3.0, cylc-8.2.5 Apr 3, 2024
@hjoliver
Copy link
Member Author

hjoliver commented Apr 3, 2024

Rebased onto 8.2.x branch for 8.2.5

@hjoliver hjoliver force-pushed the fix-workflow-state-alt-run-dir branch 2 times, most recently from 670620b to e2cd756 Compare April 3, 2024 02:52
@hjoliver hjoliver force-pushed the fix-workflow-state-alt-run-dir branch from e2cd756 to 678ca20 Compare April 3, 2024 03:30
@hjoliver
Copy link
Member Author

hjoliver commented Apr 3, 2024

@dwsutherland and @MetRonnie - I've managed to simplify the code change quite a bit, and replaced the functional test with unit tests. Hoping we can bang this into 8.2.5.

@MetRonnie MetRonnie mentioned this pull request Apr 3, 2024
Closed
2 tasks
dwsutherland
dwsutherland approved these changes Apr 3, 2024
View reviewed changes
Copy link
Member

@dwsutherland dwsutherland 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.

Ok, can probably ignore the point below

cylc/flow/scripts/workflow_state.py Outdated Show resolved Hide resolved
cylc/flow/xtriggers/workflow_state.py Outdated Show resolved Hide resolved
dwsutherland
dwsutherland approved these changes Apr 4, 2024
View reviewed changes
Copy link
Member

@dwsutherland dwsutherland left a comment

Choose a reason for hiding this comment

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

Yeah, improvement on my attempt 👍

@hjoliver
Copy link
Member Author

hjoliver commented Apr 4, 2024

Thanks, I'll merge this if tests pass...

@hjoliver hjoliver merged commit 74a2ab2 into cylc:8.2.x Apr 4, 2024
24 checks passed
@hjoliver hjoliver deleted the fix-workflow-state-alt-run-dir branch April 4, 2024 02:11
@hjoliver hjoliver mentioned this pull request Apr 4, 2024
Closed
@hjoliver hjoliver mentioned this pull request Apr 19, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwsutherland dwsutherland dwsutherland approved these changes

@MetRonnie MetRonnie MetRonnie approved these changes

Assignees

@hjoliver hjoliver

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

Successfully merging this pull request may close these issues.

workflow_state xtrigger and CLI command do not work across users
3 participants
@hjoliver @dwsutherland @MetRonnie