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

Scan workflow name during install. #5184

Merged
merged 11 commits into from
Oct 25, 2022
Merged

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Oct 10, 2022

Close #5127

Scan and report any running instances of a workflow at install time.

Screenshot from 2022-10-10 15-26-43

(Note I've used the WARNING color, but without saying "WARNING:" on grounds that already-running instances of an installed workflow might possibly indicate user error, but probably do not ... but I'm not particularly invested in this coloring!).

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.
  • 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, PRs raised to both master and the relevant maintenance branch.

@hjoliver hjoliver added this to the cylc-8.1.0 milestone Oct 10, 2022
@hjoliver hjoliver self-assigned this Oct 10, 2022
@hjoliver
Copy link
Member Author

(Considering an integration test for this).

Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

LGTM. Perhaps add in a templated cylc stop command to stop these additional runs.

Stop them with `cylc stop <id> ...`

@MetRonnie MetRonnie self-requested a review October 12, 2022 08:17
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, looking good. probably could do with a test as you've said.

Is there an easy way to include the state (running, paused etc) next to the listed workflows? Might help clarify what is meant by "active" in each case

@hjoliver
Copy link
Member Author

Integration test added. Workflow status icons added.

shot

Note this method doesn't catch instances installed with --workflow-name=other.

cylc/flow/scripts/install.py Outdated Show resolved Hide resolved
tests/integration/test_install.py Outdated Show resolved Hide resolved
tests/integration/test_install.py Outdated Show resolved Hide resolved
tests/integration/test_install.py Outdated Show resolved Hide resolved
tests/integration/test_install.py Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member Author

hjoliver commented Oct 20, 2022

Should be good to go now

  • restricted scan
  • integration test extended
  • changed CLI advice to allow easy cut-and-paste of the stop command

shot

@hjoliver
Copy link
Member Author

(Tutorial test failure unrelated)

tests/integration/test_install.py Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member Author

Two approvals, going in...

@hjoliver hjoliver merged commit 32d2633 into cylc:master Oct 25, 2022
@hjoliver hjoliver deleted the scan-after-install branch October 25, 2022 00:07
@hjoliver hjoliver restored the scan-after-install branch April 25, 2023 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

install: report if previous run is still running
3 participants