-
Notifications
You must be signed in to change notification settings - Fork 93
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
reinstall changes to rose-suite.conf
#5125
Conversation
10c14ba
to
5cd1922
Compare
639e2b5
to
294bfc7
Compare
rose-suite.conf
8228467
to
c51fbd1
Compare
Couple of small comments, otherwise LGTM, tested as working. |
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.
Tested as working. A couple of observations which may be irrelevant...
- upon reinstalling a workflow that has had no changes, it thinks the
rose-suite.conf
has changed. This is due to the difference in the added# Config Options ' (cylc-install)' from CLI appended to options already in
rose-suite.conf.. opts=(cylc-install))
. Also, do we need to adapt this to note that it is reinstall rather than install? - Cylc (re)install does not copy a nested
rose-suite.conf
file. I don't think this is an issue for now, since support for sub-workflows is limited (I believe best practice has not been agreed), although this may need consideration in the future.
ATM there is no support for nesting so this is ok. |
CHANGES.md
Outdated
[#5125](https://github.com/cylc/cylc-flow/pull/5125) - Allow rose-suite.conf | ||
changes to be considered by ``cylc reinstall``. | ||
|
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.
Now need to move to 8.0.3
@wxtim could you rebase onto 8.0.x and fix the changelog. |
45ab9b7
to
105d3c7
Compare
105d3c7
to
45ab9b7
Compare
45ab9b7
to
5230253
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.
LGTM, tested as working.
Ensured the cylc-rose test fails on 8.0.x and passes on this branch.
* fix reversed data-store edge source-target (#5156) Co-authored-by: David Sutherland <[email protected]> * `log_vc_info`: Redirect diff straight to file to avoid blocking pipe (#5139) * log_vc_info: redirect diff straight to file to avoid blocking pipe * Update changelog * A no-flow task should not merge and retrigger incomplete children (#5146) Prevent no-flow merge. * remote-install: add "ana/" to the default install list (#5137) * The `ana/` directory is used by `rose_ana` to load comparison modules. * These are typically run where the data is generated which is often remote. * These directories typically contain a small number of Python files. * Auto bump dev version on release * Run GH Actions tests on push to `8.*.x` branches * Db store force triggered (#5023) Store force-triggered flag in the run DB. * Add a new functional test. * Remove some redundant DB updates. * Update change log. * remote: ensure all remote commands use a platform config (#5152) Remote interfaces (SSH & rsync) now all require a platform object for configuration purposes (e.g. "ssh command"). Convenience interfaces added for remote calls to Cylc servers which use the localhost platform configuration. These are now used for: * `cylc play` command re-invocation on a Cylc server. * Evaluation of host selection rankings (via `cylc psutil`). * The detect old contact file check, which tests whether a workflow is still running on the server recored in the contact file. * host select: allow unary operators in ranking expressions (#5151) * host select: allow unary operators in ranking expressions * Whitelist unary operators (required for expressions like `-1 * x`) in `[scheduler][run hosts]ranking` expressions. * Improve error formatting. * Update tests/unit/test_exceptions.py Co-authored-by: Tim Pillinger <[email protected]> Co-authored-by: Tim Pillinger <[email protected]> * reinstall changes to `rose-suite.conf` (#5125) reinstall: ensure rose-suite.conf changes trigger reinstallation Co-authored-by: David Sutherland <[email protected]> Co-authored-by: Ronnie Dutta <[email protected]> Co-authored-by: Hilary James Oliver <[email protected]> Co-authored-by: Tim Pillinger <[email protected]>
* fix reversed data-store edge source-target (cylc#5156) Co-authored-by: David Sutherland <[email protected]> * `log_vc_info`: Redirect diff straight to file to avoid blocking pipe (cylc#5139) * log_vc_info: redirect diff straight to file to avoid blocking pipe * Update changelog * A no-flow task should not merge and retrigger incomplete children (cylc#5146) Prevent no-flow merge. * remote-install: add "ana/" to the default install list (cylc#5137) * The `ana/` directory is used by `rose_ana` to load comparison modules. * These are typically run where the data is generated which is often remote. * These directories typically contain a small number of Python files. * Auto bump dev version on release * Run GH Actions tests on push to `8.*.x` branches * Db store force triggered (cylc#5023) Store force-triggered flag in the run DB. * Add a new functional test. * Remove some redundant DB updates. * Update change log. * remote: ensure all remote commands use a platform config (cylc#5152) Remote interfaces (SSH & rsync) now all require a platform object for configuration purposes (e.g. "ssh command"). Convenience interfaces added for remote calls to Cylc servers which use the localhost platform configuration. These are now used for: * `cylc play` command re-invocation on a Cylc server. * Evaluation of host selection rankings (via `cylc psutil`). * The detect old contact file check, which tests whether a workflow is still running on the server recored in the contact file. * host select: allow unary operators in ranking expressions (cylc#5151) * host select: allow unary operators in ranking expressions * Whitelist unary operators (required for expressions like `-1 * x`) in `[scheduler][run hosts]ranking` expressions. * Improve error formatting. * Update tests/unit/test_exceptions.py Co-authored-by: Tim Pillinger <[email protected]> Co-authored-by: Tim Pillinger <[email protected]> * reinstall changes to `rose-suite.conf` (cylc#5125) reinstall: ensure rose-suite.conf changes trigger reinstallation Co-authored-by: David Sutherland <[email protected]> Co-authored-by: Ronnie Dutta <[email protected]> Co-authored-by: Hilary James Oliver <[email protected]> Co-authored-by: Tim Pillinger <[email protected]>
Fixes two separate, but related bugs (one of which was partially protecting us from the other, except when using rose stem):
1. Rsync Output
cylc reinstall
relied on the rsync stdout being empty to choose not to reinstall. Under some circumstances messages such ascannot delete dirctory: opt
were causing re-installations unnecessarily.2.
rose-suite.conf
reinstallationUnder many circumstances the first bug was allowing re-installation to be triggered when only the
rose-suite.conf
had changed, despiterose-suite.conf
being on the list of files excluded from the rsync.With the first bug fixed users could change the source
rose-suite.conf
and have Cylc tell them that nothing required re-installing.Check List
CONTRIBUTING.md
and added my name as a Code Contributor.setup.cfg
andconda-environment.yml
.rose-suite.conf
[tests] cylc-rose#178CHANGES.md
entry included if this is a change that can affect users