-
Notifications
You must be signed in to change notification settings - Fork 11
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
Create cylc config save file (opt/rose-suite-cylc-install.conf
)
#20
Conversation
c8e649e
to
375cb92
Compare
cdc19d5
to
4636222
Compare
I think this should close #3819 wdyt? |
I didn't enjoy rebasing this - I think that there is scope for tidying up the testing a little before release, but if things are functional I should like to have this PR merged. |
Yes, and yes. This needs adding to the |
|
@oliver-sanders and @datamel Ready for review. Sorry about all the messing about. |
46507cf
to
479c87d
Compare
eeb3429
to
c23bfbf
Compare
✅ Tested
⌛ Will test the reinstall part once we have a reinstall command.
⌛ Saving this one for a future PR.
❌ via my test suite: $ find . -type f -name '*.conf' -exec echo -e '\n=== {} ===' \; -exec cat '{}' \;
=== ./opt/rose-suite-foo.conf ===
[jinja2:suite.rc]
Y="f"
=== ./opt/rose-suite-bar.conf ===
[jinja2:suite.rc]
Y="b"
=== ./rose-suite.conf ===
opts=
[jinja2:suite.rc]
Y="base" my test install: $ ROSE_SUITE_OPT_CONF_KEYS=foo cylc install -O bar -D '[env]a=b' -S 'a="b"'
INSTALLED foo from ~/roses/foo -> ~/cylc-run/foo/run1 the result: $ find ~/cylc-run/foo/runN/ -type f -name '*.conf' -exec echo -e '\n=== {} ===' \; -exec cat '{}' \;
=== ~/cylc-run/foo/runN//opt/rose-suite-foo.conf ===
[jinja2:suite.rc]
Y="f"
=== ~/cylc-run/foo/runN//opt/rose-suite-bar.conf ===
[jinja2:suite.rc]
Y="b"
=== ~/cylc-run/foo/runN//opt/rose-suite-cylc-install.conf ===
# This file records CLI Options.
!opts=foo
[env]
a=b
[jinja2:suite.rc]
a="b"
=== ~/cylc-run/foo/runN//rose-suite.conf ===
# Config Options '(cylc-install)' from CLI appended to options already in `rose-suite.conf`.
!opts=(cylc-install)
[jinja2:suite.rc]
Y="base" the diff: === ~/cylc-run/foo/runN//opt/rose-suite-cylc-install.conf ===
# This file records CLI Options.
-!opts=foo
+!opts=bar foo
[env]
a=b
[jinja2:suite.rc]
a="b"
=== ~/cylc-run/foo/runN//rose-suite.conf ===
# Config Options '(cylc-install)' from CLI appended to options already in `rose-suite.conf`.
-!opts=(cylc-install)
+opts=bar foo (cylc-install)
[jinja2:suite.rc]
Y="base" |
|
615d87c
to
c2211c3
Compare
Co-authored-by: Oliver Sanders <oliver.sanders@metoffice.gov.uk>
|
cylc-flow: tim/cli-setting-of-rose-opts@d7f6cb18214776f63a336af1f478f584e1827192
|
I just can't replicate this @oliver-sanders |
Here's an example flow:
For the record it turns out this was caused by the |
Opt conf key stuff looks good, for interest here is my test: $ lsd
# ./opt/rose-suite-y.conf
[env]
O=Y
# ./opt/rose-suite-x.conf
[env]
O=X
# ./suite.rc
# ./rose-suite.conf
[env]
O=B
# base conf installed ✅
$ cylc clean conf-test; cylc install
$ rose config --file ~/cylc-run/conf-test/runN/rose-suite.conf env O
B
# opt conf overrides base conf ✅
$ cylc clean conf-test; cylc install -O x
$ rose config --file ~/cylc-run/conf-test/runN/rose-suite.conf env O
X
# ROSE_SUITE_OPT_CONF_KEYS override base conf in the correct order ✅
$ cylc clean conf-test; ROSE_SUITE_OPT_CONF_KEYS='x y' cylc install
$ rose config --file ~/cylc-run/conf-test/runN/rose-suite.conf env O
Y
# -O applied after ROSE_SUITE_OPT_CONF_KEY ✅
$ cylc clean conf-test; ROSE_SUITE_OPT_CONF_KEYS='x y' cylc install -O x
$ rose config --file ~/cylc-run/conf-test/runN/rose-suite.conf env O
X
# -O applied in order ✅
$ cylc clean conf-test; ROSE_SUITE_OPT_CONF_KEYS='x y' cylc install -O x -O y
$ rose config --file ~/cylc-run/conf-test/runN/rose-suite.conf env O
Y |
✅ Tested
⌛ Will test the reinstall part once we have a reinstall command.
⌛ Saving this one for a future PR.
✅ via |
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.
All good except one last thing :(
The .rose-config_processors-file.db
gets created in the source directory rather than the run directory.
This means that file installation from one installation can effect file installation in another.
It may be necessary to cd
into the run dir before the file installation then back again afterwards (perhaps in a try/finally).
Otherwise all good, @datamel please review when you get time.
|
Good to go here! |
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.
@datamel could you review 2.
Cylc option parser object - we want to extract the following | ||
values: | ||
- opt_conf_keys (list or str): | ||
Equivelent of ``rose suite-run --option KEY`` |
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.
Typo
Equivelent of ``rose suite-run --option KEY`` | |
Equivalent of ``rose suite-run --option KEY`` |
- defines (list of str): | ||
Equivelent of ``rose suite-run --define KEY=VAL`` | ||
- suite_defines (list of str): | ||
Equivelent of ``rose suite-run --define-suite KEY=VAL`` |
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.
Equivelent of ``rose suite-run --define-suite KEY=VAL`` | |
Equivalent of ``rose suite-run --define-suite KEY=VAL`` |
…nal-tests * master: [Test failure may be OK] Create cylc config save file (cylc#20)
…se-conf * master: [Test failure may be OK] Create cylc config save file (cylc#20)
opt/rose-suite-cylc-install.conf
)
sibling: cylc/cylc-flow#4023
Summary
Partial response to cylc-flow:#3819
Required by a future PR to cylc-flow adding command line options similar to the following
rose suite-run
options:--opt-conf-key
--defines
--suite-defines
Some auto tests will fail if Cylc-flow is master.
Checklist
Outstanding questions
Protected filenames:
cylc install proposal
Is this the
rose-suite-cylc-install.conf
file. Does this need doing incylc/cylc-flow
?