-
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
made reinstall work on multiple workflows #5803
made reinstall work on multiple workflows #5803
Conversation
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.
Looks good.
I don't think that VR is quite right, sorry - I don't think it will yet take wild cards: # for name in foo foot footling fool; do cylc vip ~/cylc-src/simplest/ -n $name; done
cylc stop '*'
fool/run1
Command submitted; the scheduler will log any problems.
...
# ensure it's stopped
cylc scan
# Make some reinstall-worthy change:
sed -i 's@a => b & c => d@a => b@' ~/cylc-src/simplest/flow.cylc
cylc vr 'foo*'
WorkflowFilesError: invalid workflow name 'foo*' - can only contain: alphanumeric, `/`, `_`, `+`, `-`, `.`, `@` |
Hmm I think this must be something to do with parsing of workflow ids but Im not sure why |
@wxtim, this PR is just about |
So it's out of scope. Agreed. |
LGTM, works well. Tested |
There are some unrelated test failures on this branch which have been fixed on master. Suggest rebasing this branch to pull in the updates: $ git fetch upstream
$ git checkout reinstall-multi-workflows
$ git branch reinstall-multi-workflows-safe # make a safe copy before performing rebase
$ git rebase upstream/master
$ git push -f origin HEAD You can squash the commits on this branch down at the same time by adding the |
flake8 clean up fixed unit tests fixed cylc-combination/01-vr-reload functional test fixed cylc-reinstall/00-simple.t functional test updated change log review amends
a800925
to
8140c6f
Compare
closes #4579
Updates to code
To make the reinstall operation work for multiple workflows we need to make the main function asynchronous. To do this we switch out the
reinstall_cli
function which is called in defmain
for acall_multi
function which is passedreinstall_cli
wrapped in apartial
.We then have to go through the call stack and make sure that async and await keywords are used in the correct places - or that async versions of functions are used. So in this case the
reinstall_cli
function needed theasync
keyword added andget_option_parser
multiworkflow=True.That would be job done - however
cylc_reinstall
is also imported invalidate_reinstall
so we have to repeat the process of making everything conform with the asynchronous process here also. This includes:wrapped_main
instead of_main
vro_cli
parse_id_async
instead ofparse_id
vro_cli
asynchronous with the async keywordcylc_reinstall
andscheduler_cli
As
scheduler_cli
was now being called asynchronously but was not an async function so this needed to be implemented incylc/flow/scheduler.py
which was achieved by:parse_ids
forparse_ids_async
Updates to tests
-- updates to tests --
Check List
CONTRIBUTING.md
and added my name as a Code Contributor.setup.cfg
(andconda-environment.yml
if present).CHANGES.md
entry included if this is a change that can affect users?.?.x
branch.