From c69a50e5400a174acb5076ba2635fd25801af6d9 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 6 Jul 2021 19:05:28 +0100 Subject: [PATCH 01/14] Upgraded Cylc list to use cylc rose options. --- cylc/flow/scripts/list.py | 68 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/cylc/flow/scripts/list.py b/cylc/flow/scripts/list.py index 361bd29e693..020962b356b 100755 --- a/cylc/flow/scripts/list.py +++ b/cylc/flow/scripts/list.py @@ -81,6 +81,44 @@ def get_option_parser(): "initial cycle point, by default). Use '-p , ' for the default range.", metavar="[START],[STOP]", action="store", default=None, dest="prange") + # If cylc-rose plugin is available ad the --option/-O config + try: + __import__('cylc.rose') + parser.add_option( + "--opt-conf-key", "-O", + help=( + "Use optional Rose Config Setting" + "(If Cylc-Rose is installed)" + ), + action="append", + default=[], + dest="opt_conf_keys" + ) + parser.add_option( + "--define", '-D', + help=( + "Each of these overrides the `[SECTION]KEY` setting in a " + "`rose-suite.conf` file. " + "Can be used to disable a setting using the syntax " + "`--define=[SECTION]!KEY` or even `--define=[!SECTION]`." + ), + action="append", + default=[], + dest="defines" + ) + parser.add_option( + "--rose-template-variable", '-S', + help=( + "As `--define`, but with an implicit `[SECTION]` for " + "workflow variables." + ), + action="append", + default=[], + dest="rose_template_vars" + ) + except ImportError: + pass + return parser @@ -88,6 +126,34 @@ def get_option_parser(): def main(parser, options, reg): workflow, flow_file = parse_workflow_arg(options, reg) + from cylc.flow import iter_entry_points + from cylc.flow.exceptions import PluginError + from pathlib import Path + flow_file = Path(flow_file) + source = flow_file.parent + + template_vars = load_template_vars( + options.templatevars, options.templatevars_file) + if template_vars == {}: + for entry_point in iter_entry_points( + 'cylc.pre_configure' + ): + try: + if source: + ep_result = entry_point.resolve()(srcdir=source, opts=options) + else: + from pathlib import Path + ep_result = entry_point.resolve()(srcdir=Path().cwd(), opts=options) + template_vars = ep_result['template_variables'] + except Exception as exc: + # NOTE: except Exception (purposefully vague) + # this is to separate plugin from core Cylc errors + raise PluginError( + 'cylc.pre_configure', + entry_point.name, + exc + ) from None + if options.all_tasks and options.all_namespaces: parser.error("Choose either -a or -n") if options.all_tasks: @@ -125,7 +191,7 @@ def main(parser, options, reg): workflow, flow_file, options, - load_template_vars(options.templatevars, options.templatevars_file)) + template_vars) if options.tree: config.print_first_parent_tree( pretty=options.box, titles=options.titles) From b67ef3a62068e1d3ac561d8aaef5c60894b8faba Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 6 Jul 2021 19:53:24 +0100 Subject: [PATCH 02/14] refactored the adding of cylc-rose parser options into a function --- cylc/flow/scripts/install.py | 83 ++++++++++++++++++++---------------- cylc/flow/scripts/list.py | 40 +---------------- 2 files changed, 48 insertions(+), 75 deletions(-) diff --git a/cylc/flow/scripts/install.py b/cylc/flow/scripts/install.py index b789c5b73fc..3ebad36b9d5 100755 --- a/cylc/flow/scripts/install.py +++ b/cylc/flow/scripts/install.py @@ -83,6 +83,51 @@ from optparse import Values +def add_cylc_rose_options(parser): + """Add extra options for cylc-rose plugin if it is installed. + + Args: + parser: An option parser object + """ + try: + __import__('cylc.rose') + parser.add_option( + "--opt-conf-key", "-O", + help=( + "Use optional Rose Config Setting" + "(If Cylc-Rose is installed)" + ), + action="append", + default=[], + dest="opt_conf_keys" + ) + parser.add_option( + "--define", '-D', + help=( + "Each of these overrides the `[SECTION]KEY` setting in a " + "`rose-suite.conf` file. " + "Can be used to disable a setting using the syntax " + "`--define=[SECTION]!KEY` or even `--define=[!SECTION]`." + ), + action="append", + default=[], + dest="defines" + ) + parser.add_option( + "--rose-template-variable", '-S', + help=( + "As `--define`, but with an implicit `[SECTION]` for " + "workflow variables." + ), + action="append", + default=[], + dest="rose_template_vars" + ) + except ImportError: + pass + return parser + + def get_option_parser(): parser = COP( __doc__, comms=True, prep=True, @@ -127,43 +172,7 @@ def get_option_parser(): default=False, dest="no_symlinks") - # If cylc-rose plugin is available ad the --option/-O config - try: - __import__('cylc.rose') - parser.add_option( - "--opt-conf-key", "-O", - help=( - "Use optional Rose Config Setting" - "(If Cylc-Rose is installed)" - ), - action="append", - default=[], - dest="opt_conf_keys" - ) - parser.add_option( - "--define", '-D', - help=( - "Each of these overrides the `[SECTION]KEY` setting in a " - "`rose-suite.conf` file. " - "Can be used to disable a setting using the syntax " - "`--define=[SECTION]!KEY` or even `--define=[!SECTION]`." - ), - action="append", - default=[], - dest="defines" - ) - parser.add_option( - "--rose-template-variable", '-S', - help=( - "As `--define`, but with an implicit `[SECTION]` for " - "workflow variables." - ), - action="append", - default=[], - dest="rose_template_vars" - ) - except ImportError: - pass + parser = add_cylc_rose_options(parser) return parser diff --git a/cylc/flow/scripts/list.py b/cylc/flow/scripts/list.py index 020962b356b..65ac87b5f03 100755 --- a/cylc/flow/scripts/list.py +++ b/cylc/flow/scripts/list.py @@ -35,6 +35,7 @@ from cylc.flow.config import WorkflowConfig from cylc.flow.option_parsers import CylcOptionParser as COP from cylc.flow.workflow_files import parse_workflow_arg +from cylc.flow.scripts.install import add_cylc_rose_options from cylc.flow.templatevars import load_template_vars from cylc.flow.terminal import cli_function @@ -81,44 +82,7 @@ def get_option_parser(): "initial cycle point, by default). Use '-p , ' for the default range.", metavar="[START],[STOP]", action="store", default=None, dest="prange") - # If cylc-rose plugin is available ad the --option/-O config - try: - __import__('cylc.rose') - parser.add_option( - "--opt-conf-key", "-O", - help=( - "Use optional Rose Config Setting" - "(If Cylc-Rose is installed)" - ), - action="append", - default=[], - dest="opt_conf_keys" - ) - parser.add_option( - "--define", '-D', - help=( - "Each of these overrides the `[SECTION]KEY` setting in a " - "`rose-suite.conf` file. " - "Can be used to disable a setting using the syntax " - "`--define=[SECTION]!KEY` or even `--define=[!SECTION]`." - ), - action="append", - default=[], - dest="defines" - ) - parser.add_option( - "--rose-template-variable", '-S', - help=( - "As `--define`, but with an implicit `[SECTION]` for " - "workflow variables." - ), - action="append", - default=[], - dest="rose_template_vars" - ) - except ImportError: - pass - + parser = add_cylc_rose_options(parser) return parser From 8f5d87fbf6e045012a8b5da2fd0935572bd30e05 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 8 Jul 2021 09:59:13 +0100 Subject: [PATCH 03/14] refactored method for getting template vars --- cylc/flow/scripts/list.py | 30 ++------------------------- cylc/flow/templatevars.py | 43 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/cylc/flow/scripts/list.py b/cylc/flow/scripts/list.py index 65ac87b5f03..1c3654026ed 100755 --- a/cylc/flow/scripts/list.py +++ b/cylc/flow/scripts/list.py @@ -36,7 +36,7 @@ from cylc.flow.option_parsers import CylcOptionParser as COP from cylc.flow.workflow_files import parse_workflow_arg from cylc.flow.scripts.install import add_cylc_rose_options -from cylc.flow.templatevars import load_template_vars +from cylc.flow.templatevars import get_template_vars from cylc.flow.terminal import cli_function @@ -90,33 +90,7 @@ def get_option_parser(): def main(parser, options, reg): workflow, flow_file = parse_workflow_arg(options, reg) - from cylc.flow import iter_entry_points - from cylc.flow.exceptions import PluginError - from pathlib import Path - flow_file = Path(flow_file) - source = flow_file.parent - - template_vars = load_template_vars( - options.templatevars, options.templatevars_file) - if template_vars == {}: - for entry_point in iter_entry_points( - 'cylc.pre_configure' - ): - try: - if source: - ep_result = entry_point.resolve()(srcdir=source, opts=options) - else: - from pathlib import Path - ep_result = entry_point.resolve()(srcdir=Path().cwd(), opts=options) - template_vars = ep_result['template_variables'] - except Exception as exc: - # NOTE: except Exception (purposefully vague) - # this is to separate plugin from core Cylc errors - raise PluginError( - 'cylc.pre_configure', - entry_point.name, - exc - ) from None + template_vars = get_template_vars(options, flow_file) if options.all_tasks and options.all_namespaces: parser.error("Choose either -a or -n") diff --git a/cylc/flow/templatevars.py b/cylc/flow/templatevars.py index 6d8b3c07c1d..51bae44ffd4 100644 --- a/cylc/flow/templatevars.py +++ b/cylc/flow/templatevars.py @@ -16,8 +16,13 @@ """Load custom variables for template processor.""" from ast import literal_eval +from optparse import Values +from os import PathLike +from pathlib import Path +from typing import Union, Dict -from cylc.flow.exceptions import UserInputError +from cylc.flow import iter_entry_points +from cylc.flow.exceptions import UserInputError, PluginError def eval_var(var): @@ -68,3 +73,39 @@ def load_template_vars(template_vars=None, template_vars_file=None): key, val = pair.split("=", 1) res[key.strip()] = eval_var(val.strip()) return res + + +def get_template_vars( + options: Values, flow_file: Union[str, 'PathLike[str]'] +) -> Dict: + # If we are operating on an installed workflow _load_template_vars should + # Return something. + template_vars = load_template_vars( + options.templatevars, options.templatevars_file) + # If it doesn't we operate on the possibility that we might be looking at + # a cylc-src dir. + if template_vars == {}: + flow_file = Path(flow_file) + source = flow_file.parent + for entry_point in iter_entry_points( + 'cylc.pre_configure' + ): + try: + if source: + ep_result = entry_point.resolve()( + srcdir=source, opts=options + ) + else: + ep_result = entry_point.resolve()( + srcdir=Path().cwd(), opts=options + ) + template_vars = ep_result['template_variables'] + except Exception as exc: + # NOTE: except Exception (purposefully vague) + # this is to separate plugin from core Cylc errors + raise PluginError( + 'cylc.pre_configure', + entry_point.name, + exc + ) from None + return template_vars From 45ad2f30e78df211a3023e8bedb34dfae6291b8f Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 8 Jul 2021 11:28:54 +0100 Subject: [PATCH 04/14] tested centralized template var parsing logic --- cylc/flow/templatevars.py | 29 ++++++++----- tests/unit/test_templatevars.py | 73 ++++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 11 deletions(-) diff --git a/cylc/flow/templatevars.py b/cylc/flow/templatevars.py index 51bae44ffd4..13d1675170b 100644 --- a/cylc/flow/templatevars.py +++ b/cylc/flow/templatevars.py @@ -78,6 +78,21 @@ def load_template_vars(template_vars=None, template_vars_file=None): def get_template_vars( options: Values, flow_file: Union[str, 'PathLike[str]'] ) -> Dict: + """Get Template Vars from either an uninstalled or installed flow. + + Designed to allow a Cylc Script to be run on an installed workflow where + template variables have been processed and saved to file, but fallback to + evaluating templating if run on an uninstalled workflow. + + Args: + options: Options passed to the Cylc script which is using this + function. + flow_file: Path to the ``flow.cylc`` (or ``suite.rc``) file defining + this workflow. + + Returns: + template_vars: Template variables to give to a Cylc config. + """ # If we are operating on an installed workflow _load_template_vars should # Return something. template_vars = load_template_vars( @@ -85,20 +100,14 @@ def get_template_vars( # If it doesn't we operate on the possibility that we might be looking at # a cylc-src dir. if template_vars == {}: - flow_file = Path(flow_file) - source = flow_file.parent + source = Path(flow_file).parent for entry_point in iter_entry_points( 'cylc.pre_configure' ): try: - if source: - ep_result = entry_point.resolve()( - srcdir=source, opts=options - ) - else: - ep_result = entry_point.resolve()( - srcdir=Path().cwd(), opts=options - ) + ep_result = entry_point.resolve()( + srcdir=source, opts=options + ) template_vars = ep_result['template_variables'] except Exception as exc: # NOTE: except Exception (purposefully vague) diff --git a/tests/unit/test_templatevars.py b/tests/unit/test_templatevars.py index 8a95a5a6d49..83cda7f115b 100644 --- a/tests/unit/test_templatevars.py +++ b/tests/unit/test_templatevars.py @@ -14,11 +14,14 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import pytest import tempfile import unittest -from cylc.flow.templatevars import load_template_vars +from types import SimpleNamespace +from cylc.flow.exceptions import PluginError +from cylc.flow.templatevars import get_template_vars, load_template_vars class TestTemplatevars(unittest.TestCase): @@ -102,3 +105,71 @@ def test_load_template_vars_from_string_and_file_2(self): if __name__ == '__main__': unittest.main() + + +def test_get_template_vars_installed_flow(monkeypatch): + """It works on an installed flow. + + n.b. Does not attempt to test ``load_template_vars`` + """ + monkeypatch.setattr( + 'cylc.flow.templatevars.load_template_vars', + lambda templatevars, templatevars_file: {'foo': 'bar'} + ) + opts = SimpleNamespace(templatevars='', templatevars_file='') + assert get_template_vars(opts, '') == {'foo': 'bar'} + + +def test_get_template_vars_src_flow(monkeypatch): + """It works on a flow which hasn't been installed. + """ + monkeypatch.setattr( + 'cylc.flow.templatevars.load_template_vars', + lambda templatevars, templatevars_file: {} + ) + def fake_iter_entry_points(_): + class fake_ep: + name = 'Zaphod' + def resolve(): + def _inner(srcdir, opts): + return {'template_variables': {'MYVAR': 'foo'}} + return _inner + return [fake_ep] + + monkeypatch.setattr( + 'cylc.flow.templatevars.iter_entry_points', + fake_iter_entry_points + ) + opts = SimpleNamespace( + templatevars='', templatevars_file='', opt_conf_keys=[], defines=[], + rose_template_vars=[]) + assert get_template_vars(opts, '') == {'MYVAR': 'foo'} + + +def test_get_template_vars_src_flow_fails(monkeypatch): + """It fails if there is a plugin error. + """ + monkeypatch.setattr( + 'cylc.flow.templatevars.load_template_vars', + lambda templatevars, templatevars_file: {} + ) + def fake_iter_entry_points(_): + class fake_ep: + name = 'Zaphod' + def resolve(): + def _inner(srcdir, opts): + raise TypeError('Utter Drivel.') + return _inner + return [fake_ep] + + monkeypatch.setattr( + 'cylc.flow.templatevars.iter_entry_points', + fake_iter_entry_points + ) + opts = SimpleNamespace( + templatevars='', templatevars_file='', opt_conf_keys=[], defines=[], + rose_template_vars=[]) + with pytest.raises(PluginError) as exc: + get_template_vars(opts, '') + assert exc.match( + 'Error in plugin cylc.pre_configure.Zaphod\nUtter Drivel.') From fb9af40b0b1e321214465abfc45952f56a6088ae Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 8 Jul 2021 11:35:07 +0100 Subject: [PATCH 05/14] refactored testing --- tests/unit/test_templatevars.py | 37 +++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/tests/unit/test_templatevars.py b/tests/unit/test_templatevars.py index 83cda7f115b..6c0e1b65619 100644 --- a/tests/unit/test_templatevars.py +++ b/tests/unit/test_templatevars.py @@ -120,13 +120,26 @@ def test_get_template_vars_installed_flow(monkeypatch): assert get_template_vars(opts, '') == {'foo': 'bar'} -def test_get_template_vars_src_flow(monkeypatch): - """It works on a flow which hasn't been installed. - """ +@pytest.fixture(scope='module') +def provide_opts(): + """Provide a fake opts""" + return SimpleNamespace( + templatevars='', templatevars_file='' + ) + + +@pytest.fixture +def monkeypatch_load_template_vars(monkeypatch): monkeypatch.setattr( 'cylc.flow.templatevars.load_template_vars', lambda templatevars, templatevars_file: {} ) + + +def test_get_template_vars_src_flow( + monkeypatch, provide_opts, monkeypatch_load_template_vars): + """It works on a flow which hasn't been installed. + """ def fake_iter_entry_points(_): class fake_ep: name = 'Zaphod' @@ -140,19 +153,13 @@ def _inner(srcdir, opts): 'cylc.flow.templatevars.iter_entry_points', fake_iter_entry_points ) - opts = SimpleNamespace( - templatevars='', templatevars_file='', opt_conf_keys=[], defines=[], - rose_template_vars=[]) - assert get_template_vars(opts, '') == {'MYVAR': 'foo'} + assert get_template_vars(provide_opts, '') == {'MYVAR': 'foo'} -def test_get_template_vars_src_flow_fails(monkeypatch): +def test_get_template_vars_src_flow_fails( + monkeypatch, provide_opts, monkeypatch_load_template_vars): """It fails if there is a plugin error. """ - monkeypatch.setattr( - 'cylc.flow.templatevars.load_template_vars', - lambda templatevars, templatevars_file: {} - ) def fake_iter_entry_points(_): class fake_ep: name = 'Zaphod' @@ -166,10 +173,8 @@ def _inner(srcdir, opts): 'cylc.flow.templatevars.iter_entry_points', fake_iter_entry_points ) - opts = SimpleNamespace( - templatevars='', templatevars_file='', opt_conf_keys=[], defines=[], - rose_template_vars=[]) + with pytest.raises(PluginError) as exc: - get_template_vars(opts, '') + get_template_vars(provide_opts, '') assert exc.match( 'Error in plugin cylc.pre_configure.Zaphod\nUtter Drivel.') From 6d872b78d19e190cddbcc0cc805e9ef932638ccd Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 8 Jul 2021 12:57:50 +0100 Subject: [PATCH 06/14] add cylc-rose functions to cylc-graph --- cylc/flow/scripts/graph.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cylc/flow/scripts/graph.py b/cylc/flow/scripts/graph.py index bc56d5d8b37..f94079aaf5b 100755 --- a/cylc/flow/scripts/graph.py +++ b/cylc/flow/scripts/graph.py @@ -37,8 +37,9 @@ from cylc.flow.exceptions import UserInputError from cylc.flow.option_parsers import CylcOptionParser as COP from cylc.flow.workflow_files import parse_workflow_arg -from cylc.flow.templatevars import load_template_vars +from cylc.flow.templatevars import get_template_vars from cylc.flow.terminal import cli_function +from cylc.flow.scripts.install import add_cylc_rose_options def sort_integer_node(item): @@ -209,6 +210,8 @@ def get_option_parser(): action='store', ) + parser = add_cylc_rose_options(parser) + return parser @@ -222,8 +225,7 @@ def main(parser, opts, workflow=None, start=None, stop=None): 'Only the --reference and --diff use cases are supported' ) - template_vars = load_template_vars( - opts.templatevars, opts.templatevars_file) + template_vars = get_template_vars(opts, workflow) write = print flows = [(workflow, [])] From 25f00d857f913103adc0a9268245373b3bc6af01 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 9 Jul 2021 10:17:16 +0100 Subject: [PATCH 07/14] add cylc-rose commands to cylc config --- cylc/flow/scripts/config.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cylc/flow/scripts/config.py b/cylc/flow/scripts/config.py index 9c655a42a06..b1e233fc55b 100755 --- a/cylc/flow/scripts/config.py +++ b/cylc/flow/scripts/config.py @@ -56,7 +56,8 @@ from cylc.flow.option_parsers import CylcOptionParser as COP from cylc.flow.pathutil import get_workflow_run_dir from cylc.flow.workflow_files import WorkflowFiles, parse_workflow_arg -from cylc.flow.templatevars import load_template_vars +from cylc.flow.scripts.install import add_cylc_rose_options +from cylc.flow.templatevars import get_template_vars from cylc.flow.terminal import cli_function @@ -97,6 +98,8 @@ def get_option_parser(): "overrides any settings it shares with those higher up."), action="store_true", default=False, dest="print_hierarchy") + parser = add_cylc_rose_options(parser) + return parser @@ -129,7 +132,7 @@ def main(parser, options, reg=None): workflow, flow_file, options, - load_template_vars(options.templatevars, options.templatevars_file)) + get_template_vars(options, flow_file)) config.pcfg.idump( options.item, From 87f7343d95dcd43a886e8a32b337f1660718131b Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 9 Jul 2021 12:58:05 +0100 Subject: [PATCH 08/14] upgraded checks for being installed --- cylc/flow/scripts/config.py | 2 +- cylc/flow/scripts/list.py | 3 +-- cylc/flow/scripts/validate.py | 4 ++-- cylc/flow/templatevars.py | 23 ++++++++++++++--------- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cylc/flow/scripts/config.py b/cylc/flow/scripts/config.py index b1e233fc55b..f9c2baf70c3 100755 --- a/cylc/flow/scripts/config.py +++ b/cylc/flow/scripts/config.py @@ -132,7 +132,7 @@ def main(parser, options, reg=None): workflow, flow_file, options, - get_template_vars(options, flow_file)) + get_template_vars(options, flow_file, [reg, workflow])) config.pcfg.idump( options.item, diff --git a/cylc/flow/scripts/list.py b/cylc/flow/scripts/list.py index 1c3654026ed..4b1beee45fb 100755 --- a/cylc/flow/scripts/list.py +++ b/cylc/flow/scripts/list.py @@ -89,8 +89,7 @@ def get_option_parser(): @cli_function(get_option_parser) def main(parser, options, reg): workflow, flow_file = parse_workflow_arg(options, reg) - - template_vars = get_template_vars(options, flow_file) + template_vars = get_template_vars(options, flow_file, [reg, flow_file]) if options.all_tasks and options.all_namespaces: parser.error("Choose either -a or -n") diff --git a/cylc/flow/scripts/validate.py b/cylc/flow/scripts/validate.py index 036a31c4f03..f005bb73ac7 100755 --- a/cylc/flow/scripts/validate.py +++ b/cylc/flow/scripts/validate.py @@ -38,7 +38,7 @@ from cylc.flow.task_proxy import TaskProxy from cylc.flow.task_pool import FlowLabelMgr from cylc.flow.loggingutil import CylcLogFormatter -from cylc.flow.templatevars import load_template_vars +from cylc.flow.templatevars import get_template_vars from cylc.flow.option_parsers import ( CylcOptionParser as COP, Options @@ -104,7 +104,7 @@ def main(_, options, reg): workflow, flow_file, options, - load_template_vars(options.templatevars, options.templatevars_file), + get_template_vars(options, flow_file, [reg, workflow]), output_fname=options.output, mem_log_func=profiler.log_memory) # Check bounds of sequences diff --git a/cylc/flow/templatevars.py b/cylc/flow/templatevars.py index 13d1675170b..d66134fc7d2 100644 --- a/cylc/flow/templatevars.py +++ b/cylc/flow/templatevars.py @@ -19,7 +19,7 @@ from optparse import Values from os import PathLike from pathlib import Path -from typing import Union, Dict +from typing import Union, Dict, Tuple, Optional from cylc.flow import iter_entry_points from cylc.flow.exceptions import UserInputError, PluginError @@ -76,7 +76,9 @@ def load_template_vars(template_vars=None, template_vars_file=None): def get_template_vars( - options: Values, flow_file: Union[str, 'PathLike[str]'] + options: Values, + flow_file: Union[str, 'PathLike[str]'], + names: Optional[Tuple[str, str]] = None ) -> Dict: """Get Template Vars from either an uninstalled or installed flow. @@ -93,13 +95,16 @@ def get_template_vars( Returns: template_vars: Template variables to give to a Cylc config. """ - # If we are operating on an installed workflow _load_template_vars should - # Return something. - template_vars = load_template_vars( - options.templatevars, options.templatevars_file) - # If it doesn't we operate on the possibility that we might be looking at - # a cylc-src dir. - if template_vars == {}: + # We are operating on an installed workflow. + if ( + names + and names[0] == names[1] + and not Path(names[0]).is_file() + ): + template_vars = load_template_vars( + options.templatevars, options.templatevars_file) + # Else we act as if we might be looking at a cylc-src dir. + else: source = Path(flow_file).parent for entry_point in iter_entry_points( 'cylc.pre_configure' From 933ffeb2ff45ea72dd1ea25563b9db1a5c618408 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 12 Jul 2021 09:47:31 +0100 Subject: [PATCH 09/14] fix test for template var changes; update template var docstr --- cylc/flow/templatevars.py | 7 +++++-- tests/unit/test_templatevars.py | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cylc/flow/templatevars.py b/cylc/flow/templatevars.py index d66134fc7d2..dba091906c0 100644 --- a/cylc/flow/templatevars.py +++ b/cylc/flow/templatevars.py @@ -91,6 +91,9 @@ def get_template_vars( function. flow_file: Path to the ``flow.cylc`` (or ``suite.rc``) file defining this workflow. + names: reg and flow file name from + `cylc.flow.workflow_filesparse_workflow_arg`: Used to determine + whether flow is installed. Returns: template_vars: Template variables to give to a Cylc config. @@ -98,8 +101,8 @@ def get_template_vars( # We are operating on an installed workflow. if ( names - and names[0] == names[1] - and not Path(names[0]).is_file() + and names[0] == names[1] # reg == flow_file name + and not Path(names[0]).is_file() # reg is not a path ): template_vars = load_template_vars( options.templatevars, options.templatevars_file) diff --git a/tests/unit/test_templatevars.py b/tests/unit/test_templatevars.py index 6c0e1b65619..47a90e4f484 100644 --- a/tests/unit/test_templatevars.py +++ b/tests/unit/test_templatevars.py @@ -117,7 +117,8 @@ def test_get_template_vars_installed_flow(monkeypatch): lambda templatevars, templatevars_file: {'foo': 'bar'} ) opts = SimpleNamespace(templatevars='', templatevars_file='') - assert get_template_vars(opts, '') == {'foo': 'bar'} + result = get_template_vars(opts, '', names=('eg/runN', 'eg/runN')) + assert result == {'foo': 'bar'} @pytest.fixture(scope='module') From 503fb932f41ae4abf3e560192e0393eaba0618e1 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 12 Jul 2021 10:10:21 +0100 Subject: [PATCH 10/14] It works better if you import the function --- cylc/flow/scripts/validate.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cylc/flow/scripts/validate.py b/cylc/flow/scripts/validate.py index f005bb73ac7..a6f5ae2da4e 100755 --- a/cylc/flow/scripts/validate.py +++ b/cylc/flow/scripts/validate.py @@ -44,6 +44,7 @@ Options ) from cylc.flow.workflow_files import parse_workflow_arg +from cylc.flow.scripts.install import add_cylc_rose_options def get_option_parser(): @@ -71,6 +72,8 @@ def get_option_parser(): default="live", dest="run_mode", choices=['live', 'dummy', 'dummy-local', 'simulation']) + parser = add_cylc_rose_options(parser) + parser.set_defaults(is_validate=True) return parser From 51af6837da6669256aa7d8361574905b85b47058 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 12 Jul 2021 10:57:24 +0100 Subject: [PATCH 11/14] fixed what happens if cylc rose not installed --- cylc/flow/templatevars.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cylc/flow/templatevars.py b/cylc/flow/templatevars.py index dba091906c0..9a3e57a546f 100644 --- a/cylc/flow/templatevars.py +++ b/cylc/flow/templatevars.py @@ -99,6 +99,7 @@ def get_template_vars( template_vars: Template variables to give to a Cylc config. """ # We are operating on an installed workflow. + template_vars = {} if ( names and names[0] == names[1] # reg == flow_file name From 035c79e647bae269612aac21480d622f8cd44303 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 12 Jul 2021 11:40:29 +0100 Subject: [PATCH 12/14] fixed broken logic --- cylc/flow/templatevars.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cylc/flow/templatevars.py b/cylc/flow/templatevars.py index 9a3e57a546f..8ecc99bd393 100644 --- a/cylc/flow/templatevars.py +++ b/cylc/flow/templatevars.py @@ -99,7 +99,6 @@ def get_template_vars( template_vars: Template variables to give to a Cylc config. """ # We are operating on an installed workflow. - template_vars = {} if ( names and names[0] == names[1] # reg == flow_file name @@ -109,6 +108,8 @@ def get_template_vars( options.templatevars, options.templatevars_file) # Else we act as if we might be looking at a cylc-src dir. else: + template_vars = load_template_vars( + options.templatevars, options.templatevars_file) source = Path(flow_file).parent for entry_point in iter_entry_points( 'cylc.pre_configure' @@ -117,7 +118,7 @@ def get_template_vars( ep_result = entry_point.resolve()( srcdir=source, opts=options ) - template_vars = ep_result['template_variables'] + template_vars.extend(ep_result['template_variables']) except Exception as exc: # NOTE: except Exception (purposefully vague) # this is to separate plugin from core Cylc errors From 5ec95b29c20fe74a49754f529317f5f10597ab6d Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 12 Jul 2021 11:58:27 +0100 Subject: [PATCH 13/14] fix template vars --- cylc/flow/templatevars.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/templatevars.py b/cylc/flow/templatevars.py index 8ecc99bd393..9befff6e326 100644 --- a/cylc/flow/templatevars.py +++ b/cylc/flow/templatevars.py @@ -118,7 +118,7 @@ def get_template_vars( ep_result = entry_point.resolve()( srcdir=source, opts=options ) - template_vars.extend(ep_result['template_variables']) + template_vars.update(ep_result['template_variables']) except Exception as exc: # NOTE: except Exception (purposefully vague) # this is to separate plugin from core Cylc errors From de5b143b49b9920dc9d60fdf826dcd5cb17de7c8 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 13 Jul 2021 08:44:41 +0100 Subject: [PATCH 14/14] Update cylc/flow/scripts/install.py --- cylc/flow/scripts/install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/scripts/install.py b/cylc/flow/scripts/install.py index 3ebad36b9d5..c139292b795 100755 --- a/cylc/flow/scripts/install.py +++ b/cylc/flow/scripts/install.py @@ -94,7 +94,7 @@ def add_cylc_rose_options(parser): parser.add_option( "--opt-conf-key", "-O", help=( - "Use optional Rose Config Setting" + "Use optional Rose Config Setting " "(If Cylc-Rose is installed)" ), action="append",