-
Notifications
You must be signed in to change notification settings - Fork 94
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
cylc vr
#5229
cylc vr
#5229
Conversation
15e056e
to
66e4b2d
Compare
9534b52
to
3483102
Compare
Validation against source for installed workflows.
Co-authored-by: Oliver Sanders <[email protected]>
Co-authored-by: Oliver Sanders <[email protected]> simplify integration tests, writing proper test harness fixtures response to review
Co-authored-by: Oliver Sanders <[email protected]>
a0068e7
to
86acb05
Compare
108ee54
to
6fc3ca3
Compare
6fc3ca3
to
4ce97a2
Compare
Co-authored-by: Oliver Sanders <[email protected]>
914701e
to
a620f92
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty file tests/unit/scripts/test_validate.py?
65690e7
to
b24d54e
Compare
b24d54e
to
7f7a716
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From testing out:
- When doing
cylc vr
on a stopped workflow, there is no indication that the workflow is now running$ cylc vr temp4 $ cylc validate --against-source temp4/run1 Valid for cylc-8.1.0.dev $ cylc reinstall temp4/run1 REINSTALL temp4/run1 from ~/cylc-src/temp4 temp4/run1 up to date with ~/cylc-src/temp4
cylc vr --no-detach
is missing the timestamps - not a biggie so probably address later
This is the output from Cylc reinstall saying there's nothing to be reinstalled. Having had a quick chat with @MetRonnie I've added a warning, which seems a bit much, until you realize that this scenario produces a non-zero return code from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
source='', # Intentionally blank | ||
) | ||
log_subcommand('play', workflow_id) | ||
scheduler_cli(options, workflow_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might need a return 0
at the end of this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think this is fine, turns out sys.exit(None)
is equivalent to sys.exit(0)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Nice!
Closes #3896
Branch based upon a rebase of #5214 onto #5121, which it has now replaced.
Depending on what Reviewers thing it may replace #5214 once #5121 has been merged.Purpose
Check List
CONTRIBUTING.md
and added my name as a Code Contributor.setup.cfg
andconda-environment.yml
.CHANGES.md
entry included if this is a change that can affect users?.?.x
branch.