Superseedes: #2960
Related to: #1885
Write a Cylc plugin which detects the rose-suite.conf file and acts accordingly.
This plugin will call out to Rose2 libraries, this functionality already exists in the Rose it's just a matter of injecting it in the correct places in the Cylc logic.
rose-suite.conf FileThe rose-suite.conf file has the following sections:
opts - Optional configuration keys.root-dir - Affects suite installation, symlinks Cylc directories.[env] - Environment variables.[jinja2:suiterc] - Jinja2 variables.[empy:suiterc] - Empy variavles.[file:*] - File installation.optsRose should use this variable to determine which optional configuration files to load. This is already implemented in the existing Rose configuration logic so shouldn't require any extra work on the Cylc side.
root-dirObsolete, replaced by https://github.com/cylc/cylc-flow/issues/3688, log a warning and do nothing.
Questions:
[symlink dirs][<install target>]run configs?~ No.[env]~Export these environment variables into the Cylc environment (i.e. os.environ[key] = var).~
Note: We should not edit the
flow.cylcfile, if exporting the env vars is insufficient we could consider adding a command line option tocylc (run|restart).
For now the path of least resistance is probably to just go ahead and meddle with the flow.cylc file, we can keep this logic in the Cylc part (rather than the plugin part) and improve it later, perhaps with a new Cylc options file or an environment variable table in the database (to match the template variables table).
Questions:
[cylc][evironment]flow.cylc file (really don't want to go down that route again)?[jinja2:suiterc]Note: I think we will probably keep this as
:suite.rcfor now.
With cylc7/rose-2019 Rose prepends these variables to the top of the suite.rc file. It should not be necessary to edit any Cylc files to achieve this in Cylc8.
Note: Look at implementing this logic in the parsec layer so that it doesn't have to be manually implemented on a command by command basis.
Fallback: Otherwise we have the ability to set "suite variables" (i.e. Jinja2/empy varaibles) on the command line using
-s KEY=valueor-f file-containing-variables. Load these variables and prepend them to the list of variables passed in on the command line.
[empy:suiterc]Same as for Jinja2.
flow.cylc file to make sure the rose-suite.conf file is configured for the correct template parser and raise an error if it is not or if no template parser is being used.[file:*]Call out to the Rose file installation system as the current rose suite-run logic does.
Questions:
At a glance I think the following commands will require loading the rose-suite.conf:
optsjinja2:suite.rcempy:suite.rccylc run|restart command has been re-executed on the suite server (if the suite server is not localhost)Create two new Python entry points to keep this functionality separate from Cylc:
[env]) and template ([*:suite.rc]) variables.[file:*]) for suite installation.The plugin interfaces should probably look something like this:
set_environment(src_dir: Path) -> dict # {'environment': {}, 'template-vars': {}}
install(src_dir: Path, run_dir: Path, run_mode: str) -> None
The plugins should output any required information using the Cylc log.
This work can be attempted in stages implementing one section at a time. The [env] section is probably the easiest to get started with.
Plugins:
Commands:
cylc get-suite-configcylc runcylc restartcylc validatecylc viewcylc diffcylc viewcylc graph - perhaps skip this one as the command only exists for test functionalitycylc reload - perhaps leave this till last as it may be a bit more involvedThe big question: where do we put this?
I'm happy with whatever, it might be easier for the moment to develop in Cylc Flow and transfer later. The entry point should keep the codebases sufficiently separate.
(I think the questions in the Issue description are mainly aimed at the UK Rose experts).
The big question: where do we put this?
I'm not too bothered either, if you want to do in cylc-flow now then move it out later.
Do we want to develop this functionality in the Cylc Flow repo (making Rose2 an optional dependency of Cylc).
This seems the most logical to me, at least for the time being.
[jinja2:suiterc]_Note:_ I think we will probably keep this as
:suite.rcfor now.With cylc7/rose-2019 Rose prepends these variables to the top of the
suite.rcfile. It should not be necessary to edit any Cylc files to achieve this in Cylc8.We have the ability to set "suite variables" (i.e. Jinja2/empy varaibles) on the command line using
-s KEY=valueor-f file-containing-variables.Load these variables and prepend them to the list of variables passed in on the command line.
Due to the current implementation there's a significant difference between rose and cylc suite variables in that rose ones are treated as Jinja2 code, whereas cylc ones only as strings. Perhaps it would make sense to look first at implementing equivalent of #3169 to get these two on equal footing and treat them the same.
[file:*]Call out to the Rose file installation system as the current
rose suite-runlogic does.Questions:
- Should we perform file installation logic on reloads?
Reload is often used to update files in the suite, so I'd say yes. Unless you want to use this as an "incentive" for people to migrate off rose 馃槈.
rose ones are treated as Jinja2 code, whereas cylc ones only as strings
Under the hood Cylc passes these variables straight through to Jinja2, it's just the CLI that enforces string arguments so this approach should support typing from the off.
rose ones are treated as Jinja2 code, whereas cylc ones only as strings
Under the hood Cylc passes these variables straight through to Jinja2, it's just the CLI that enforces string arguments so this approach should support typing from the off.
Not enough. If you're not going to write the rose vars into the template any more, you'll need to eval them somewhere before passing into jinja to reproduce cylc7 behaviour.
Well yes, we will have to eval them so that we are passing Python objets rather than strings (we won't be able to just strip away the outer quotes and let Jinja2 implicitly eval them). This shouldn't be an issue with the internal interface.
I had a proof-of-concept go at implementing the basic functionality of the [jinja2:suite.rc] section in this branch.
The logic to support optional configs is present in the function get_rose_vars and I tested it, but for the PoC I didn't bother plugging it into the cylc command line.
Also, I haven't tried the literal_eval yet - can anyone give me some examples of the sort of thing we might expect to need evaluating?
Any thoughts.
The [jinja2:suite.rc] section is typically used to inject basic Python data types into the suite.rc file. Lists are often used and, although unsupported in the Rose GUI, dicts are pretty common as well.
At present anything you configure just gets dumped straight into the suite.rc file and left for Jinja2 to evaluate so any types that Jinja2 supports could be used.
Had a quick look at the jinja2 code not expecting any surprises from that.
>>> from ast import literal_eval
>>> literal_eval('42')
42
>>> literal_eval('True')
True
>>> literal_eval('"True"')
'True'
>>> literal_eval('[1, 2, 3]')
[1, 2, 3]
>>> literal_eval("{'a': 1, 'b': 2, 'c': 3}")
{'a': 1, 'b': 2, 'c': 3}
I think that I have now got the literal eval working in that branch.
Further details on how the [env] section is used.
Some users have accessed [env] variables in the suite.rc via jinja2, e.g.
{{ environ['WORKING_COPY_PATH'] }}
Note that this currently only works if you run the suite locally rather than cylc spawning on another server. Should we support this?
Some users reference [env] variables in other sections of the rose-suite.conf file, e.g.
[env]
APPS_BASE="fcm:foo_tr"
APPS_RVN="@1234"
[file:apps/bar]
source = ${APPS_BASE}/apps/bar${APPS_RVN}
This is only way of defining a single variable which can be referenced in multiple places within the rose-suite.conf file.
It is a "supported" way of working (although I can't find it documented anywhere - we should fix that)
All defined [env] variables get added to the [cylc][[environment]] section.
[runtime]
[[root]]
[[[remote]]]
host = $ROSE_ORIG_HOST
(Note that this isn't made clear in the documentation: https://cylc.github.io/doc/built-sphinx/appendices/suiterc-config-ref.html#cylc-environment)
Note that $ROSE_ORIG_HOST and $ROSE_SITE get defined automatically and we should continue to do this.
Setting CYLC_VERSION and ROSE_VERSION variables in rose-suite.conf currently allows rose suite-run to:
a) Change the version of Cylc and check the version of Rose used to run the suite;
b) Prevent suites accidentally upgrading or downgrading versions on restart.
Review whether we want to support this in some way later.
For reference here is some example output (suite.rc) showing the Jinja2 and environment meddling that Rose currently does and the default variables it provides:
#!jinja2
{# Rose Configuration Insertion: Init #}
{% set CYLC_VERSION="7.8.6" %}
{% set ROSE_ORIG_HOST="myhostname" %}
{% set ROSE_SITE="sitename" %}
{% set ROSE_VERSION="2019.01.3" %}
{% set ROSE_SUITE_VARIABLES={
'CYLC_VERSION': CYLC_VERSION,
'ROSE_ORIG_HOST': ROSE_ORIG_HOST,
'ROSE_SITE': ROSE_SITE,
'ROSE_VERSION': ROSE_VERSION,
} %}
[cylc]
[[environment]]
CYLC_VERSION=7.8.6
ROSE_ORIG_HOST=myhostname
ROSE_SITE=
ROSE_VERSION=2019.01.3
{# Rose Configuration Insertion: Done #}
Some users have accessed [env] variables in the suite.rc via jinja2, e.g.
{{ environ['WORKING_COPY_PATH'] }}Note that this currently only works if you run the suite locally rather than cylc spawning on another server. Should we support this?
I guess we can't prevent this, even though it is not advisable, except by not exporting rose-suite.conf variables in the scheduler environment at all - is that what you're suggesting?
I guess we can't prevent this, even though it is not advisable, except by not exporting
rose-suite.confvariables in the scheduler environment at all - is that what you're suggesting?
It only works if we add these variables to the environment before we start to process the cylc.flow file (just adding them to the [cylc][[environment]] section isn't enough). So, I think we would have to choose to deliberately support this.
Is this still in the plan https://github.com/cylc/cylc-flow/pull/3169#issuecomment-505398634:
From current discussions, in Cylc8 we will likely have a new file for setting suite variables which will be a Python file (allowing variables to be directly imported) which would provide a much nicer solution for this.
I would really like to be able to run a generic, custom logic in a standard way at start up, not just literal_eval.
Is this still in the plan #3169 (comment):
Not officially tagged against Cylc8, however, pretty trivial to implement using the interface provided by this plugin.
With rose suite-run it is common to pass optional configs with -O opt_conf command line options. I guess not something that can be done with the plugin, or can it?
A lot of people use this so we will need to find a way to supporting -O. I expect the plugin can provide an optparse object or else just some basic options in a dictionary.
(Note it's not worth building too-advanced an interface for it now as the CLI library is likely to change)
A lot of people use this so we will need to find a way to supporting
-O. I expect the plugin can provide an optparse object or else just some basic options in a dictionary.
Interesting challenge for CLI design...
Yeah not the nicest, perhaps (one day) we should use a load method interface for plugins, pytest does this quite nicely.
Assigning @wxtim as he has some some prelim investigation work with Jinja2 vars.
With Rose 2019.x optional config keys can be configured in three ways:
opts configuration in the rose-suite.conf file.-O CLI option to rose suite-run.ROSE_SUITE_OPT_CONF_KEYS env var visible to rose suite-run.All three should remain supported in Cylc8/Rose2.
cylc install.cylc play) pick up the optional config(s) that the flow was installed with.log/rose-suite.conf.opts config in the installed rose-suite.conf file if required.Subsequent re-installation should preserve optional configs specified by previous installations.
I.E:
cat > rose-suite.conf <<< opts=foo
cylc install -O bar -O baz
# optional configs foo, bar and baz applied
cylc reinstall
#聽optional configs foo, bar and baz still applied
However, additional CLI options will be provided to permit the removal of targeted optional configs or permit rebuilding the list from scratch (as done by the original install).
I.E:
cylc reinstall -N bar
# optional configs foo and baz still applied, bar removed
cylc reinstall --rebuild-opt-confs
#聽optional config foo applied (as it is specified in the rose-suite.conf)
# bar and baz removed (as they were specified by other means)
CLI arguments TBC but should be something along the lines of:
-O, --opt-conf-key - To match current Rose interface-N, --remove-opt-conf-key--reset-opt-conf-keys@TomekTrzeciak - In reply to https://github.com/cylc/cylc-flow/pull/4023#issuecomment-757913510
We are planning to keep Rose and Cylc templatevars separate but have unified their syntax as much as possible:
cylc-install optional config in the installed workflow.rose config) or the Python API (metomi.rose.config).-s, --set-file.We have opted not to unify Rose and Cylc template variables because they are not directly equivalent and don't quite have the same scope:
Unifying Rose and Cylc templatevars is not all that straight forward and I'm not entirely sure it makes sense:
More context:
[env] section.true rather than True) for us to drop support in Rose completely.a, b, c rather than (a, b, c) which are in very wide use and hard to replace in metadata.range(5), [0, 1] + [2, 3]) as planned.{a, b, c} which Cylc already does.For more information see:
@wxtim closed by cylc-rose PRs?
https://github.com/cylc/cylc-rose/issues/37 to replace outstanding section of this issue.