Reinstall picks up installed config.

cylc / cylc-rose

A Cylc plugin providing support for the Rose rose-suite.conf file.

https://cylc.github.io/cylc-doc/latest/html/plugins/cylc-rose.html

GNU General Public License v3.0

6 stars 11 forks source link

Reinstall picks up installed config. #345

Closed wxtim closed 3 weeks ago

wxtim commented 1 month ago

Closes https://github.com/cylc/cylc-flow/issues/6286

Partner of https://github.com/cylc/cylc-flow/pull/6404, required to close https://github.com/cylc/cylc-flow/issues/6286

Use options.against_source = rundir to merge new CLI options with those stored in an earlier install's rose-suite-cylc-install.conf.

Check List

[x] I have read CONTRIBUTING.md and added my name as a Code Contributor.
[x] Contains logically grouped changes (else tidy your branch by rebase).
[x] Does not contain off-topic changes (use other PRs for other changes).
[x] Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
[x] Tests are included, and will fail until the companion PR is merged.
[x] Changelog entry included if this is a change that can affect users
[x] Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
[x] If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

oliver-sanders commented 1 month ago

LGTM, tested as working, one question (above).

wxtim commented 1 month ago

Need to check whether I need to bump the version number - does this work/can this be made safe with Cylc<8.3.6

oliver-sanders commented 1 month ago

As reported by Mark the other day, the "WARNING [env]ROSE_ORIG_HOST will be ignored" messages which appear on this branch are not very helpful:

The environment variable has not changed so the warning is invalid.
This is just an internal thing due to the way this env var is set on installation, it's nothing to do with the options the user specified.
There is nothing the user can do to avoid this warning.

WARNING - [env]ROSE_ORIG_HOST=<myhost> from rose-suite.conf will be ignored: ROSE_ORIG_HOST will be:
    <myhost>
WARNING - [template variables]ROSE_ORIG_HOST=<myhost> from rose-suite.conf will be ignored: ROSE_ORIG_HOST will
    be: <myhost>

Hopefully a simple fix.

oliver-sanders commented 1 month ago

As reported by Mark the other day

I think you've turned off this warning completely for cylc vr? It (now?) doesn't show when it should?:

$ cylc reinstall -S 'ROSE_ORIG_HOST="elephant"' tmp.2EYla85JQJ
...
WARNING - [template variables]ROSE_ORIG_HOST="elephant" from rose-suite.conf will be ignored: ROSE_ORIG_HOST will be:
    cazld000020.spice.sc.metoffice.gov.uk
REINSTALLED tmp.2EYla85JQJ/run2 from /var/tmp/tmp.2EYla85JQJ
Successfully reinstalled.
Run "cylc reload tmp.2EYla85JQJ/run2" to pick up changes.
Done

$ cylc vr -S 'ROSE_ORIG_HOST="elephant"' tmp.2EYla85JQJ
...
REINSTALLED tmp.2EYla85JQJ/run2 from /var/tmp/tmp.2EYla85JQJ
Successfully reinstalled.
$ cylc reload tmp.2EYla85JQJ/run2

wxtim commented 3 weeks ago

@oliver-sanders - uncommented a previous fix - should work!

~I'll have a look at a bit more testing - this has been driving me crackers~

Have now added a bit more in the way of testing.

wxtim commented 3 weeks ago

Closed and re-opened to kick the tests onward. The one failure passes with the paired Cylc-Flow branch checked out. 😄

oliver-sanders commented 3 weeks ago

Upstream PR merged, poking CI.

oliver-sanders commented 3 weeks ago

@wxtim, I think the Python 3.7 failure is a legit back-compat issue, hopefully an easy fix.

wxtim commented 3 weeks ago

Tried distutils but this seems to be something Cylc 3.8 and 3.9 were unhappy with. Gone for a really clunky run(cp).

oliver-sanders commented 3 weeks ago

https://github.com/cylc/cylc-flow/issues/6076

Fine.