diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 1f8680bd..2389a139 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -21,7 +21,9 @@ from cylc.rose.utilities import ( copy_config_file, dump_rose_log, - get_rose_vars, + export_environment, + load_rose_config, + process_config, record_cylc_install_options, rose_config_exists, ) @@ -32,12 +34,22 @@ def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: if not srcdir: # not sure how this could happen return { + # default return value 'env': {}, 'template_variables': {}, 'templating_detected': None } - srcdir: Path = Path(srcdir) - return get_rose_vars(srcdir=srcdir, opts=opts) + + # load the Rose config + config_tree = load_rose_config(Path(srcdir), opts=opts) + + # extract plugin return information from the Rose config + plugin_result = process_config(config_tree) + + # set environment variables + export_environment(plugin_result['env']) + + return plugin_result def post_install(srcdir=None, opts=None, rundir=None) -> Union[dict, bool]: diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 514d06d0..1be4cfdd 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -39,6 +39,7 @@ def rose_fileinstall(rundir: 'Path', opts: 'Values'): return False # Load the config tree + # TODO: private config_tree = rose_config_tree_loader(rundir, opts) if any(i.startswith('file') for i in config_tree.node.value): diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index 47e816a1..2a556cc0 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -78,8 +78,14 @@ from metomi.rose.reporter import Event, Reporter from metomi.rose.resource import ResourceLocator -from cylc.rose.entry_points import get_rose_vars -from cylc.rose.utilities import id_templating_section +from cylc.rose.entry_points import ( + export_environment, + load_rose_config, +) +from cylc.rose.utilities import ( + id_templating_section, + process_config, +) EXC_EXIT = cparse('{name}: {exc}') DEFAULT_TEST_DIR = 'rose-stem' @@ -456,8 +462,12 @@ def process(self): self.opts.project.append(project) if i == 0: - template_type = get_rose_vars( - Path(url) / "rose-stem")["templating_detected"] + config_tree = load_rose_config(Path(url) / "rose-stem") + plugin_result = process_config(config_tree) + # set environment variables + export_environment(plugin_result['env']) + template_type = plugin_result['templating_detected'] + self.template_section = id_templating_section( template_type, with_brackets=True) diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index c4019cc3..b2ea9905 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -24,7 +24,7 @@ import re import shlex import shutil -from typing import Any, List, Optional, Tuple, Union +from typing import Any, Dict, List, Optional, Tuple, Union from cylc.flow import LOG from cylc.flow.exceptions import CylcError @@ -39,6 +39,7 @@ ConfigNodeDiff, ) from metomi.rose.config_processor import ConfigProcessError +from metomi.rose.config_tree import ConfigTree from metomi.rose.env import UnboundEnvironmentVariableError, env_var_process from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros @@ -73,28 +74,41 @@ class InvalidDefineError(CylcError): ... -def get_rose_vars_from_config_node(config, config_node, environ=os.environ): - """Load template variables from a Rose config node. +def process_config( + config_tree: 'ConfigTree', + environ=os.environ, +) -> Dict[str, Any]: + """Process template and environment variables. - This uses only the provided config node and environment variables - - there is no system interaction. + Note: + This uses only the provided config node and environment variables, + there is no system interaction. Args: - config (dict): - Object which will be populated with the results. - config_node (metomi.rose.config.ConfigNode): + config_tree: Configuration node representing the Rose suite configuration. - environ (dict): + environ: Dictionary of environment variables (for testing). """ + plugin_result: Dict[str, Any] = { + # default return value + 'env': {}, + 'template_variables': {}, + 'templating_detected': None + } + config_node = config_tree.node + # Don't allow multiple templating sections. templating = identify_templating_section(config_node) if templating != 'template variables': - config['templating_detected'] = templating.replace(':suite.rc', '') + plugin_result['templating_detected'] = templating.replace( + ':suite.rc', + '', + ) else: - config['templating_detected'] = templating + plugin_result['templating_detected'] = templating # Create env section if it doesn't already exist. if 'env' not in config_node.value: @@ -151,29 +165,29 @@ def get_rose_vars_from_config_node(config, config_node, environ=os.environ): # For each of the template language sections extract items to a simple # dict to be returned. - config['env'] = { + plugin_result['env'] = { item[0][1]: item[1].value for item in config_node.value['env'].walk() if item[1].state == ConfigNode.STATE_NORMAL } - config['template_variables'] = { + plugin_result['template_variables'] = { item[0][1]: item[1].value for item in config_node.value[templating].walk() if item[1].state == ConfigNode.STATE_NORMAL } - # Add the entire config to ROSE_SUITE_VARIABLES to allow for programatic - # access. + # Add the entire plugin_result to ROSE_SUITE_VARIABLES to allow for + # programatic access. with patch_jinja2_leading_zeros(): # BACK COMPAT: patch_jinja2_leading_zeros # back support zero-padded integers for a limited time to help # users migrate before upgrading cylc-flow to Jinja2>=3.1 parser = Parser() - for key, value in config['template_variables'].items(): + for key, value in plugin_result['template_variables'].items(): # The special variables are already Python variables. if key not in ['ROSE_ORIG_HOST', 'ROSE_VERSION', 'ROSE_SITE']: try: - config['template_variables'][key] = ( + plugin_result['template_variables'][key] = ( parser.literal_eval(value) ) except Exception: @@ -185,9 +199,11 @@ def get_rose_vars_from_config_node(config, config_node, environ=os.environ): ' (note strings "must be quoted").' ) from None - # Add ROSE_SUITE_VARIABLES to config of templating engines in use. - config['template_variables'][ - 'ROSE_SUITE_VARIABLES'] = config['template_variables'] + # Add ROSE_SUITE_VARIABLES to plugin_result of templating engines in use. + plugin_result['template_variables'][ + 'ROSE_SUITE_VARIABLES'] = plugin_result['template_variables'] + + return plugin_result def identify_templating_section(config_node): @@ -789,19 +805,27 @@ def deprecation_warnings(config_tree): LOG.warning(info[MESSAGE]) -def get_rose_vars(srcdir=None, opts=None): - """Load template variables from Rose suite configuration. +def load_rose_config( + srcdir: Path, + opts=None, + warn: bool = True, +) -> 'ConfigTree': + """Load rose configuration from srcdir. + + Load template variables from Rose suite configuration. Loads the Rose suite configuration tree from the filesystem using the shell environment. Args: - srcdir(pathlib.Path): + srcdir: Path to the Rose suite configuration (the directory containing the ``rose-suite.conf`` file). opts: Options object containing specification of optional configuarations set by the CLI. + warn: + Log deprecation warnings if True. Returns: dict - A dictionary of sections of rose-suite.conf. @@ -815,22 +839,18 @@ def get_rose_vars(srcdir=None, opts=None): } } """ - # Set up blank page for returns. - config = { - 'env': {}, - 'template_variables': {}, - 'templating_detected': None - } - # Return a blank config dict if srcdir does not exist if not rose_config_exists(srcdir): if ( - getattr(opts, "opt_conf_keys", None) - or getattr(opts, "defines", None) - or getattr(opts, "rose_template_vars", None) + opts + and ( + getattr(opts, "opt_conf_keys", None) + or getattr(opts, "defines", None) + or getattr(opts, "rose_template_vars", None) + ) ): raise NotARoseSuiteException() - return config + return ConfigTree() # Check for definitely invalid defines if opts and hasattr(opts, 'defines'): @@ -838,20 +858,17 @@ def get_rose_vars(srcdir=None, opts=None): # Load the raw config tree config_tree = rose_config_tree_loader(srcdir, opts) - deprecation_warnings(config_tree) + if warn: + deprecation_warnings(config_tree) - # Extract templatevars from the configuration - get_rose_vars_from_config_node( - config, - config_tree.node, - ) + return config_tree + +def export_environment(environment): # Export environment vars - for key, val in config['env'].items(): + for key, val in environment.items(): os.environ[key] = val - return config - def record_cylc_install_options( rundir=None, diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index baecc6df..7785735b 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -27,7 +27,7 @@ import pytest from pytest import param -from cylc.rose.utilities import NotARoseSuiteException, get_rose_vars +from cylc.rose.utilities import NotARoseSuiteException, load_rose_config @pytest.mark.parametrize( @@ -121,7 +121,7 @@ def test_process(srcdir, envvars): def test_warn_if_root_dir_set(root_dir_config, tmp_path, caplog): """Test using unsupported root-dir config raises error.""" (tmp_path / 'rose-suite.conf').write_text(root_dir_config) - get_rose_vars(srcdir=tmp_path) + load_rose_config(tmp_path) msg = 'rose-suite.conf[root-dir]' assert msg in caplog.records[0].msg @@ -150,7 +150,7 @@ def test_warn_if_old_templating_set( 'cylc.rose.utilities.cylc7_back_compat', compat_mode ) (tmp_path / 'rose-suite.conf').write_text(f'[{rose_config}]') - get_rose_vars(srcdir=tmp_path) + load_rose_config(tmp_path) msg = "Use [template variables]" if compat_mode: assert not caplog.records @@ -174,7 +174,7 @@ def test_fail_if_options_but_no_rose_suite_conf(opts, tmp_path): NotARoseSuiteException, match='^Cylc-Rose CLI' ): - get_rose_vars(tmp_path, opts) + load_rose_config(tmp_path, opts) def generate_params(): diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py index 848815e3..8c697dad 100644 --- a/tests/unit/test_config_node.py +++ b/tests/unit/test_config_node.py @@ -20,6 +20,7 @@ from metomi.isodatetime.datetimeoper import DateTimeOperator from metomi.rose import __version__ as ROSE_VERSION from metomi.rose.config import ConfigNode +from metomi.rose.config_tree import ConfigTree from metomi.rose.config_processor import ConfigProcessError import pytest @@ -29,7 +30,7 @@ add_cylc_install_to_rose_conf_node_opts, deprecation_warnings, dump_rose_log, - get_rose_vars_from_config_node, + process_config, id_templating_section, identify_templating_section, ) @@ -37,9 +38,7 @@ def test_blank(): """It should provide only standard vars for a blank config.""" - ret = {} - node = ConfigNode() - get_rose_vars_from_config_node(ret, node, {}) + ret = process_config(ConfigTree(), {}) assert set(ret.keys()) == { 'template_variables', 'templating_detected', 'env' } @@ -51,37 +50,40 @@ def test_blank(): def test_invalid_templatevar(): """It should wrap eval errors as something more informative.""" - ret = {} + tree = ConfigTree() node = ConfigNode() node.set(['jinja2:suite.rc', 'X'], 'Y') + tree.node = node with pytest.raises(ConfigProcessError): - get_rose_vars_from_config_node(ret, node, {}) + process_config(tree, {}) -def test_get_rose_vars_from_config_node__unbound_env_var(caplog): +def test_get_plugin_result__unbound_env_var(caplog): """It should fail if variable unset in environment. """ - ret = {} + tree = ConfigTree() node = ConfigNode() node.set(['env', 'X'], '${MYVAR}') + tree.node = node with pytest.raises(ConfigProcessError) as exc: - get_rose_vars_from_config_node(ret, node, {}) + process_config(tree, {}) assert exc.match('env=X: MYVAR: unbound variable') @pytest.fixture def override_version_vars(caplog, scope='module'): - """Set up config tree and pass to get_rose_vars_from_config_node + """Set up config tree and pass to process_config Yields: node: The node after manipulation. message: A string representing the caplog output. """ - ret = {} + tree = ConfigTree() node = ConfigNode() node.set(['template variables', 'ROSE_VERSION'], 99) node.set(['template variables', 'CYLC_VERSION'], 101) - get_rose_vars_from_config_node(ret, node, {}) + tree.node = node + process_config(tree, {}) message = '\n'.join([i.message for i in caplog.records]) yield (node, message) @@ -268,11 +270,12 @@ def test_id_templating_section(input_, expect): @pytest.fixture def node_with_ROSE_ORIG_HOST(): def _inner(comment=''): - ret = {} + tree = ConfigTree() node = ConfigNode() node.set(['env', 'ROSE_ORIG_HOST'], 'IMPLAUSIBLE_HOST_NAME') node['env']['ROSE_ORIG_HOST'].comments = [comment] - get_rose_vars_from_config_node(ret, node, {}) + tree.node = node + process_config(tree, {}) return node yield _inner diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 7557aa18..497b6ddd 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -23,7 +23,10 @@ import pytest from pytest import param -from cylc.rose.entry_points import get_rose_vars +from cylc.rose.entry_points import ( + process_config, + load_rose_config, +) from cylc.rose.utilities import ( MultipleTemplatingEnginesError, get_cli_opts_node, @@ -167,12 +170,14 @@ def test_get_rose_vars( f"[{section}:suite.rc]Another_Jinja2_var='Variable overridden'" ] suite_path = rose_config_template(section) - result = get_rose_vars( + config_tree = load_rose_config( suite_path, options - )['template_variables'] - - assert result['Another_Jinja2_var'] == exp_ANOTHER_JINJA2_ENV - assert result['JINJA2_VAR'] == exp_JINJA2_VAR + ) + template_variables = ( + process_config(config_tree)['template_variables'] + ) + assert template_variables['Another_Jinja2_var'] == exp_ANOTHER_JINJA2_ENV + assert template_variables['JINJA2_VAR'] == exp_JINJA2_VAR def test_get_rose_vars_env_section(tmp_path): @@ -182,9 +187,9 @@ def test_get_rose_vars_env_section(tmp_path): "DOG_TYPE = Spaniel \n" ) - assert ( - get_rose_vars(tmp_path)['env']['DOG_TYPE'] - ) == 'Spaniel' + config_tree = load_rose_config(tmp_path) + environment_variables = process_config(config_tree)['env'] + assert environment_variables['DOG_TYPE'] == 'Spaniel' def test_get_rose_vars_expansions(monkeypatch, tmp_path): @@ -201,20 +206,26 @@ def test_get_rose_vars_expansions(monkeypatch, tmp_path): "BOOL=True\n" 'LIST=["a", 1, True]\n' ) - rose_vars = get_rose_vars(tmp_path) - assert rose_vars['template_variables']['LOCAL_ENV'] == 'xyz' - assert rose_vars['template_variables']['BAR'] == 'ab' - assert rose_vars['template_variables']['ESCAPED_ENV'] == '$HOME' - assert rose_vars['template_variables']['INT'] == 42 - assert rose_vars['template_variables']['BOOL'] is True - assert rose_vars['template_variables']['LIST'] == ["a", 1, True] + config_tree = load_rose_config(tmp_path) + template_variables = ( + process_config(config_tree)['template_variables'] + ) + assert template_variables['LOCAL_ENV'] == 'xyz' + assert template_variables['BAR'] == 'ab' + assert template_variables['ESCAPED_ENV'] == '$HOME' + assert template_variables['INT'] == 42 + assert template_variables['BOOL'] is True + assert template_variables['LIST'] == ["a", 1, True] def test_get_rose_vars_ROSE_VARS(tmp_path): """Test that rose variables are available in the environment section..""" (tmp_path / "rose-suite.conf").touch() - rose_vars = get_rose_vars(tmp_path) - assert sorted(rose_vars['env']) == sorted([ + config_tree = load_rose_config(tmp_path) + environment_variables = ( + process_config(config_tree)['env'] + ) + assert sorted(environment_variables) == sorted([ 'ROSE_ORIG_HOST', 'ROSE_VERSION', ]) @@ -225,9 +236,12 @@ def test_get_rose_vars_jinja2_ROSE_VARS(tmp_path): (tmp_path / "rose-suite.conf").write_text( "[jinja2:suite.rc]" ) - rose_vars = get_rose_vars(tmp_path) + config_tree = load_rose_config(tmp_path) + template_variables = ( + process_config(config_tree)['template_variables'] + ) assert sorted( - rose_vars['template_variables']['ROSE_SUITE_VARIABLES'] + template_variables['ROSE_SUITE_VARIABLES'] ) == sorted([ 'ROSE_VERSION', 'ROSE_ORIG_HOST', @@ -241,8 +255,9 @@ def test_get_rose_vars_fail_if_empy_AND_jinja2(tmp_path): "[jinja2:suite.rc]\n" "[empy:suite.rc]\n" ) + config_tree = load_rose_config(tmp_path) with pytest.raises(MultipleTemplatingEnginesError): - get_rose_vars(tmp_path) + process_config(config_tree) @pytest.mark.parametrize( diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index f58dd47e..72b7fbbd 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -19,6 +19,7 @@ from types import SimpleNamespace from typing import Any, Tuple +from metomi.rose.config_tree import ConfigTree from metomi.rose.fs_util import FileSystemUtil from metomi.rose.popen import RosePopener from metomi.rose.reporter import Reporter @@ -427,10 +428,13 @@ def test_process_template_engine_set_correctly(monkeypatch, language, expect): https://github.com/cylc/cylc-rose/issues/246 """ - # Mimic expected result from get_rose_vars method: monkeypatch.setattr( - 'cylc.rose.stem.get_rose_vars', - lambda _: {'templating_detected': language} + 'cylc.rose.stem.load_rose_config', + lambda _: ConfigTree() + ) + monkeypatch.setattr( + 'cylc.rose.stem.process_config', + lambda _: {'templating_detected': language, 'env': {}} ) monkeypatch.setattr( 'sys.argv',