From 3d3e3f4727777b345cd95630fc9d05e2bf42e74e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 13:51:08 +0000 Subject: [PATCH 01/12] refactor: move utilities out of the entry_points module * Move utils out of the entry points module and move the rose-stem entry point in. * Move fileinstall into its own module. * Sort imports. --- cylc/rose/entry_points.py | 299 +----------------- cylc/rose/fileinstall.py | 80 +++++ cylc/rose/jinja2_parser.py | 11 +- cylc/rose/platform_utils.py | 4 +- cylc/rose/stem.py | 23 +- cylc/rose/utilities.py | 227 ++++++++++++- setup.cfg | 2 +- tests/conftest.py | 23 +- tests/functional/test_ROSE_ORIG_HOST.py | 6 +- tests/functional/test_copy_config_file.py | 4 +- tests/functional/test_pre_configure.py | 15 +- tests/functional/test_reinstall.py | 12 +- tests/functional/test_reinstall_clean.py | 10 +- .../functional/test_reinstall_fileinstall.py | 6 +- tests/functional/test_rose_fileinstall.py | 5 +- tests/functional/test_rose_stem.py | 26 +- tests/unit/test_config_node.py | 12 +- tests/unit/test_config_tree.py | 17 +- tests/unit/test_functional_post_install.py | 26 +- tests/unit/test_generic_utils.py | 4 +- tests/unit/test_platform_utils.py | 16 +- tests/unit/test_rose_opts.py | 5 +- tests/unit/test_rose_stem_units.py | 19 +- 23 files changed, 423 insertions(+), 429 deletions(-) create mode 100644 cylc/rose/fileinstall.py diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 0517bde4..f169be90 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -13,53 +13,28 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Cylc support for reading and interpreting ``rose-suite.conf`` workflow -configuration files. +"""Top level module providing entry point functions.""" -Top level module providing entry point functions. -""" - -import os -import shutil - -from pathlib import Path - -from metomi.rose.config import ConfigLoader, ConfigDumper from cylc.rose.utilities import ( - ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, - deprecation_warnings, + copy_config_file, dump_rose_log, - get_rose_vars_from_config_node, - identify_templating_section, - invalid_defines_check, - rose_config_exists, - rose_config_tree_loader, - merge_rose_cylc_suite_install_conf, + get_rose_vars, paths_to_pathlib, - get_cli_opts_node, - add_cylc_install_to_rose_conf_node_opts, + record_cylc_install_options, + rose_config_exists, ) -from cylc.flow.hostuserutil import get_host - - -class NotARoseSuiteException(Exception): - def __str__(self): - msg = ( - 'Cylc-Rose CLI arguments only used ' - 'if a rose-suite.conf file is present:' - '\n * "--opt-conf-key" or "-O"' - '\n * "--define" or "-D"' - '\n * "--rose-template-variable" or "-S"' - ) - return msg def pre_configure(srcdir=None, opts=None, rundir=None): + """Run before the Cylc configuration is read.""" srcdir, rundir = paths_to_pathlib([srcdir, rundir]) return get_rose_vars(srcdir=srcdir, opts=opts) def post_install(srcdir=None, opts=None, rundir=None): + """Run after Cylc file installation has completed.""" + from cylc.rose.fileinstall import rose_fileinstall + if not rose_config_exists(srcdir, opts): return False srcdir, rundir = paths_to_pathlib([srcdir, rundir]) @@ -78,255 +53,9 @@ def post_install(srcdir=None, opts=None, rundir=None): return results -def get_rose_vars(srcdir=None, opts=None): - """Load template variables from Rose suite configuration. - - Loads the Rose suite configuration tree from the filesystem - using the shell environment. - - Args: - srcdir(pathlib.Path): - 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. - - Returns: - dict - A dictionary of sections of rose-suite.conf. - For each section either a dictionary or None is returned. - E.g. - { - 'env': {'MYVAR': 42}, - 'empy:suite.rc': None, - 'jinja2:suite.rc': { - 'myJinja2Var': {'yes': 'it is a dictionary!'} - } - } - """ - # 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, opts): - if ( - getattr(opts, "opt_conf_keys", None) - or getattr(opts, "defines", None) - or getattr(opts, "rose_template_vars", None) - ): - raise NotARoseSuiteException() - return config - - # Check for definitely invalid defines - if opts and hasattr(opts, 'defines'): - invalid_defines_check(opts.defines) - - # Load the raw config tree - config_tree = rose_config_tree_loader(srcdir, opts) - deprecation_warnings(config_tree) - - # Extract templatevars from the configuration - get_rose_vars_from_config_node( - config, - config_tree.node, - os.environ - ) - - # Export environment vars - for key, val in config['env'].items(): - os.environ[key] = val - - return config - - -def record_cylc_install_options( - rundir=None, - opts=None, - srcdir=None, -): - """Create/modify files recording Cylc install config options. - - Creates a new config based on CLI options and writes it to the workflow - install location as ``rose-suite-cylc-install.conf``. - - If ``rose-suite-cylc-install.conf`` already exists over-writes changed - items, except for ``!opts=`` which is merged and simplified. - - If ``!opts=`` have been changed these are appended to those that have - been written in the installed ``rose-suite.conf``. - - Args: - srcdir (pathlib.Path): - Used to check whether the source directory contains a rose config. - opts: - Cylc option parser object - we want to extract the following - values: - - opt_conf_keys (list or str): - Equivalent of ``rose suite-run --option KEY`` - - defines (list of str): - Equivalent of ``rose suite-run --define KEY=VAL`` - - rose_template_vars (list of str): - Equivalent of ``rose suite-run --define-suite KEY=VAL`` - rundir (pathlib.Path): - Path to dump the rose-suite-cylc-conf - - Returns: - cli_config - Config Node which has been dumped to - ``rose-suite-cylc-install.conf``. - rose_suite_conf['opts'] - Opts section of the config node dumped to - installed ``rose-suite.conf``. - """ - # Create a config based on command line options: - cli_config = get_cli_opts_node(opts, srcdir) - - # raise error if CLI config has multiple templating sections - identify_templating_section(cli_config) - - # Construct path objects representing our target files. - (Path(rundir) / 'opt').mkdir(exist_ok=True) - conf_filepath = Path(rundir) / 'opt/rose-suite-cylc-install.conf' - rose_conf_filepath = Path(rundir) / 'rose-suite.conf' - dumper = ConfigDumper() - loader = ConfigLoader() - - # If file exists we need to merge with our new config, over-writing with - # new items where there are duplicates. - if conf_filepath.is_file(): - if opts.clear_rose_install_opts: - conf_filepath.unlink() - else: - oldconfig = loader.load(str(conf_filepath)) - # Check old config for clashing template variables sections. - identify_templating_section(oldconfig) - cli_config = merge_rose_cylc_suite_install_conf( - oldconfig, cli_config - ) - - # Get Values for standard ROSE variable ROSE_ORIG_HOST. - rose_orig_host = get_host() - for section in [ - 'env', 'jinja2:suite.rc', 'empy:suite.rc', 'template variables' - ]: - if section in cli_config: - cli_config[section].set(['ROSE_ORIG_HOST'], rose_orig_host) - cli_config[section]['ROSE_ORIG_HOST'].comments = [ - ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING - ] - - cli_config.comments = [' This file records CLI Options.'] - dumper.dump(cli_config, str(conf_filepath)) - - # Merge the opts section of the rose-suite.conf with those set by CLI: - rose_conf_filepath.touch() - rose_suite_conf = loader.load(str(rose_conf_filepath)) - rose_suite_conf = add_cylc_install_to_rose_conf_node_opts( - rose_suite_conf, cli_config - ) - identify_templating_section(rose_suite_conf) - - dumper(rose_suite_conf, rose_conf_filepath) - - return cli_config, rose_suite_conf - - -def rose_fileinstall(srcdir=None, opts=None, rundir=None): - """Call Rose Fileinstall. - - Args: - srcdir(pathlib.Path): - Search for a ``rose-suite.conf`` file in this location. - rundir (pathlib.Path) - - """ - if not rose_config_exists(rundir, opts): - return False - - # Load the config tree - config_tree = rose_config_tree_loader(rundir, opts) - - if any(i.startswith('file') for i in config_tree.node.value): - try: - startpoint = os.getcwd() - os.chdir(rundir) - except FileNotFoundError as exc: - raise exc - else: - # Carry out imports. - import asyncio - from metomi.rose.config_processor import ConfigProcessorsManager - from metomi.rose.popen import RosePopener - from metomi.rose.reporter import Reporter - from metomi.rose.fs_util import FileSystemUtil - - # Update config tree with install location - # NOTE-TO-SELF: value=os.environ["CYLC_WORKFLOW_RUN_DIR"] - config_tree.node = config_tree.node.set( - keys=["file-install-root"], value=str(rundir) - ) - - # Artificially set rose to verbose. - event_handler = Reporter(3) - fs_util = FileSystemUtil(event_handler) - popen = RosePopener(event_handler) - - # Get an Asyncio loop if one doesn't exist: - # Rose may need an event loop to invoke async interfaces, - # doing this here incase we want to go async in cylc-rose. - # See https://github.com/cylc/cylc-rose/pull/130/files - try: - asyncio.get_event_loop() - except RuntimeError: - asyncio.set_event_loop(asyncio.new_event_loop()) - - # Process fileinstall. - config_pm = ConfigProcessorsManager(event_handler, popen, fs_util) - config_pm(config_tree, "file") - finally: - os.chdir(startpoint) - - return config_tree.node - - -def copy_config_file( - srcdir=None, - opts=None, - rundir=None, -): - """Copy the ``rose-suite.conf`` from a workflow source to run directory. - - Args: - srcdir (pathlib.Path | or str): - Source Path of Cylc install. - opts: - Not used in this function, but requried for consistent entry point. - rundir (pathlib.Path | or str): - Destination path of Cylc install - the workflow rundir. - - Return: - True if ``rose-suite.conf`` has been installed. - False if insufficiant information to install file given. - """ - if ( - rundir is None or - srcdir is None - ): - raise FileNotFoundError( - "This plugin requires both source and rundir to exist." - ) - - rundir = Path(rundir) - srcdir = Path(srcdir) - srcdir_rose_conf = srcdir / 'rose-suite.conf' - rundir_rose_conf = rundir / 'rose-suite.conf' - - if not srcdir_rose_conf.is_file(): - return False - elif rundir_rose_conf.is_file(): - rundir_rose_conf.unlink() - shutil.copy2(srcdir_rose_conf, rundir_rose_conf) +def rose_stem(): + """Implements the "rose stem" command.""" + from cylc.rose.stem import get_rose_stem_opts - return True + parser, opts = get_rose_stem_opts() + rose_stem(parser, opts) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py new file mode 100644 index 00000000..4c9c83b0 --- /dev/null +++ b/cylc/rose/fileinstall.py @@ -0,0 +1,80 @@ +# THIS FILE IS PART OF THE ROSE-CYLC PLUGIN FOR THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Utilities related to performing Rose file installation.""" + +import os + +from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader + + +def rose_fileinstall(srcdir=None, opts=None, rundir=None): + """Call Rose Fileinstall. + + Args: + srcdir(pathlib.Path): + Search for a ``rose-suite.conf`` file in this location. + rundir (pathlib.Path) + + """ + if not rose_config_exists(rundir, opts): + return False + + # Load the config tree + config_tree = rose_config_tree_loader(rundir, opts) + + if any(i.startswith('file') for i in config_tree.node.value): + try: + startpoint = os.getcwd() + os.chdir(rundir) + except FileNotFoundError as exc: + raise exc + else: + # Carry out imports. + import asyncio + + from metomi.rose.config_processor import ConfigProcessorsManager + from metomi.rose.fs_util import FileSystemUtil + from metomi.rose.popen import RosePopener + from metomi.rose.reporter import Reporter + + # Update config tree with install location + # NOTE-TO-SELF: value=os.environ["CYLC_WORKFLOW_RUN_DIR"] + config_tree.node = config_tree.node.set( + keys=["file-install-root"], value=str(rundir) + ) + + # Artificially set rose to verbose. + event_handler = Reporter(3) + fs_util = FileSystemUtil(event_handler) + popen = RosePopener(event_handler) + + # Get an Asyncio loop if one doesn't exist: + # Rose may need an event loop to invoke async interfaces, + # doing this here incase we want to go async in cylc-rose. + # See https://github.com/cylc/cylc-rose/pull/130/files + try: + asyncio.get_event_loop() + except RuntimeError: + asyncio.set_event_loop(asyncio.new_event_loop()) + + # Process fileinstall. + config_pm = ConfigProcessorsManager(event_handler, popen, fs_util) + config_pm(config_tree, "file") + finally: + os.chdir(startpoint) + + return config_tree.node diff --git a/cylc/rose/jinja2_parser.py b/cylc/rose/jinja2_parser.py index 47387d72..aa0528e7 100644 --- a/cylc/rose/jinja2_parser.py +++ b/cylc/rose/jinja2_parser.py @@ -16,22 +16,21 @@ """Utility for parsing Jinja2 expressions.""" from ast import literal_eval as python_literal_eval -from copy import deepcopy from contextlib import contextmanager +from copy import deepcopy import re +from cylc.flow import LOG +import jinja2.lexer from jinja2.nativetypes import NativeEnvironment # type: ignore from jinja2.nodes import ( # type: ignore Literal, + Neg, Output, Pair, + Pos, Template, - Neg, - Pos ) -import jinja2.lexer - -from cylc.flow import LOG def _strip_leading_zeros(string): diff --git a/cylc/rose/platform_utils.py b/cylc/rose/platform_utils.py index 88363954..5c3d6740 100644 --- a/cylc/rose/platform_utils.py +++ b/cylc/rose/platform_utils.py @@ -17,9 +17,9 @@ # ----------------------------------------------------------------------------- """Interfaces for Cylc Platforms for use by rose apps. """ -import subprocess from optparse import Values import sqlite3 +import subprocess from time import sleep from typing import Any, Dict @@ -29,7 +29,7 @@ from cylc.flow.platforms import ( HOST_REC_COMMAND, get_platform, - is_platform_definition_subshell + is_platform_definition_subshell, ) from cylc.flow.rundb import CylcWorkflowDAO diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index ba585775..47e816a1 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -60,7 +60,6 @@ is intended to specify the revision of `fcm-make` config files. """ -from ansimarkup import parse as cparse from contextlib import suppress from optparse import OptionGroup import os @@ -68,22 +67,19 @@ import re import sys +from ansimarkup import parse as cparse from cylc.flow.exceptions import CylcError -from cylc.flow.scripts.install import ( - get_option_parser, - install as cylc_install -) - -from cylc.rose.entry_points import get_rose_vars -from cylc.rose.utilities import id_templating_section - +from cylc.flow.scripts.install import get_option_parser +from cylc.flow.scripts.install import install as cylc_install import metomi.rose.config from metomi.rose.fs_util import FileSystemUtil from metomi.rose.host_select import HostSelector from metomi.rose.popen import RosePopener -from metomi.rose.reporter import Reporter, Event +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 EXC_EXIT = cparse('{name}: {exc}') DEFAULT_TEST_DIR = 'rose-stem' @@ -552,7 +548,7 @@ def get_source_opt_from_args(opts, args): return opts -def _get_rose_stem_opts(): +def get_rose_stem_opts(): """Implement rose stem.""" # use the cylc install option parser parser = get_option_parser() @@ -603,11 +599,6 @@ def _get_rose_stem_opts(): return parser, opts -def main(): - parser, opts = _get_rose_stem_opts() - rose_stem(parser, opts) - - def rose_stem(parser, opts): try: # modify the CLI options to add whatever rose stem would like to add diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index c570d4ed..853d3fef 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -13,26 +13,35 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Cylc support for reading and interpreting ``rose-suite.conf`` workflow configuration files. """ + import itertools import os from pathlib import Path import re import shlex +import shutil from typing import TYPE_CHECKING, Any, List, Optional, Tuple, Union -from cylc.flow.hostuserutil import get_host from cylc.flow import LOG from cylc.flow.exceptions import CylcError from cylc.flow.flags import cylc7_back_compat -from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros -from metomi.rose import __version__ as ROSE_VERSION +from cylc.flow.hostuserutil import get_host from metomi.isodatetime.datetimeoper import DateTimeOperator -from metomi.rose.config import ConfigNodeDiff, ConfigNode, ConfigDumper +from metomi.rose import __version__ as ROSE_VERSION +from metomi.rose.config import ( + ConfigDumper, + ConfigLoader, + ConfigNode, + ConfigNodeDiff, +) from metomi.rose.config_processor import ConfigProcessError -from metomi.rose.env import env_var_process, UnboundEnvironmentVariableError +from metomi.rose.env import UnboundEnvironmentVariableError, env_var_process + +from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros if TYPE_CHECKING: from optparse import Values @@ -47,6 +56,18 @@ ALL_MODES = 'all modes' +class NotARoseSuiteException(Exception): + def __str__(self): + msg = ( + 'Cylc-Rose CLI arguments only used ' + 'if a rose-suite.conf file is present:' + '\n * "--opt-conf-key" or "-O"' + '\n * "--define" or "-D"' + '\n * "--rose-template-variable" or "-S"' + ) + return msg + + class MultipleTemplatingEnginesError(CylcError): ... @@ -787,3 +808,199 @@ def deprecation_warnings(config_tree): and name in string.lower() ): LOG.warning(info[MESSAGE]) + + +def get_rose_vars(srcdir=None, opts=None): + """Load template variables from Rose suite configuration. + + Loads the Rose suite configuration tree from the filesystem + using the shell environment. + + Args: + srcdir(pathlib.Path): + 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. + + Returns: + dict - A dictionary of sections of rose-suite.conf. + For each section either a dictionary or None is returned. + E.g. + { + 'env': {'MYVAR': 42}, + 'empy:suite.rc': None, + 'jinja2:suite.rc': { + 'myJinja2Var': {'yes': 'it is a dictionary!'} + } + } + """ + # 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, opts): + if ( + getattr(opts, "opt_conf_keys", None) + or getattr(opts, "defines", None) + or getattr(opts, "rose_template_vars", None) + ): + raise NotARoseSuiteException() + return config + + # Check for definitely invalid defines + if opts and hasattr(opts, 'defines'): + invalid_defines_check(opts.defines) + + # Load the raw config tree + config_tree = rose_config_tree_loader(srcdir, opts) + deprecation_warnings(config_tree) + + # Extract templatevars from the configuration + get_rose_vars_from_config_node( + config, + config_tree.node, + os.environ + ) + + # Export environment vars + for key, val in config['env'].items(): + os.environ[key] = val + + return config + + +def record_cylc_install_options( + rundir=None, + opts=None, + srcdir=None, +): + """Create/modify files recording Cylc install config options. + + Creates a new config based on CLI options and writes it to the workflow + install location as ``rose-suite-cylc-install.conf``. + + If ``rose-suite-cylc-install.conf`` already exists over-writes changed + items, except for ``!opts=`` which is merged and simplified. + + If ``!opts=`` have been changed these are appended to those that have + been written in the installed ``rose-suite.conf``. + + Args: + srcdir (pathlib.Path): + Used to check whether the source directory contains a rose config. + opts: + Cylc option parser object - we want to extract the following + values: + - opt_conf_keys (list or str): + Equivalent of ``rose suite-run --option KEY`` + - defines (list of str): + Equivalent of ``rose suite-run --define KEY=VAL`` + - rose_template_vars (list of str): + Equivalent of ``rose suite-run --define-suite KEY=VAL`` + rundir (pathlib.Path): + Path to dump the rose-suite-cylc-conf + + Returns: + cli_config - Config Node which has been dumped to + ``rose-suite-cylc-install.conf``. + rose_suite_conf['opts'] - Opts section of the config node dumped to + installed ``rose-suite.conf``. + """ + # Create a config based on command line options: + cli_config = get_cli_opts_node(opts, srcdir) + + # raise error if CLI config has multiple templating sections + identify_templating_section(cli_config) + + # Construct path objects representing our target files. + (Path(rundir) / 'opt').mkdir(exist_ok=True) + conf_filepath = Path(rundir) / 'opt/rose-suite-cylc-install.conf' + rose_conf_filepath = Path(rundir) / 'rose-suite.conf' + dumper = ConfigDumper() + loader = ConfigLoader() + + # If file exists we need to merge with our new config, over-writing with + # new items where there are duplicates. + if conf_filepath.is_file(): + if opts.clear_rose_install_opts: + conf_filepath.unlink() + else: + oldconfig = loader.load(str(conf_filepath)) + # Check old config for clashing template variables sections. + identify_templating_section(oldconfig) + cli_config = merge_rose_cylc_suite_install_conf( + oldconfig, cli_config + ) + + # Get Values for standard ROSE variable ROSE_ORIG_HOST. + rose_orig_host = get_host() + for section in [ + 'env', 'jinja2:suite.rc', 'empy:suite.rc', 'template variables' + ]: + if section in cli_config: + cli_config[section].set(['ROSE_ORIG_HOST'], rose_orig_host) + cli_config[section]['ROSE_ORIG_HOST'].comments = [ + ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING + ] + + cli_config.comments = [' This file records CLI Options.'] + dumper.dump(cli_config, str(conf_filepath)) + + # Merge the opts section of the rose-suite.conf with those set by CLI: + rose_conf_filepath.touch() + rose_suite_conf = loader.load(str(rose_conf_filepath)) + rose_suite_conf = add_cylc_install_to_rose_conf_node_opts( + rose_suite_conf, cli_config + ) + identify_templating_section(rose_suite_conf) + + dumper(rose_suite_conf, rose_conf_filepath) + + return cli_config, rose_suite_conf + + +def copy_config_file( + srcdir=None, + opts=None, + rundir=None, +): + """Copy the ``rose-suite.conf`` from a workflow source to run directory. + + Args: + srcdir (pathlib.Path | or str): + Source Path of Cylc install. + opts: + Not used in this function, but requried for consistent entry point. + rundir (pathlib.Path | or str): + Destination path of Cylc install - the workflow rundir. + + Return: + True if ``rose-suite.conf`` has been installed. + False if insufficiant information to install file given. + """ + if ( + rundir is None or + srcdir is None + ): + raise FileNotFoundError( + "This plugin requires both source and rundir to exist." + ) + + rundir = Path(rundir) + srcdir = Path(srcdir) + srcdir_rose_conf = srcdir / 'rose-suite.conf' + rundir_rose_conf = rundir / 'rose-suite.conf' + + if not srcdir_rose_conf.is_file(): + return False + elif rundir_rose_conf.is_file(): + rundir_rose_conf.unlink() + shutil.copy2(srcdir_rose_conf, rundir_rose_conf) + + return True diff --git a/setup.cfg b/setup.cfg index 3fa012bd..136eeaea 100644 --- a/setup.cfg +++ b/setup.cfg @@ -89,4 +89,4 @@ cylc.pre_configure = cylc.post_install = rose_opts = cylc.rose.entry_points:post_install rose.commands = - stem = cylc.rose.stem:main + stem = cylc.rose.entry_points:rose_stem diff --git a/tests/conftest.py b/tests/conftest.py index 4031397e..93647bf2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -16,26 +16,17 @@ """Functional tests for top-level function record_cylc_install_options and """ -import pytest from types import SimpleNamespace from cylc.flow import __version__ as CYLC_VERSION from cylc.flow.option_parsers import Options - -from cylc.flow.scripts.validate import ( - _main as cylc_validate, - get_option_parser as validate_gop -) - -from cylc.flow.scripts.install import ( - install_cli as cylc_install, - get_option_parser as install_gop -) - -from cylc.flow.scripts.reinstall import ( - reinstall_cli as cylc_reinstall, - get_option_parser as reinstall_gop -) +from cylc.flow.scripts.install import get_option_parser as install_gop +from cylc.flow.scripts.install import install_cli as cylc_install +from cylc.flow.scripts.reinstall import get_option_parser as reinstall_gop +from cylc.flow.scripts.reinstall import reinstall_cli as cylc_reinstall +from cylc.flow.scripts.validate import _main as cylc_validate +from cylc.flow.scripts.validate import get_option_parser as validate_gop +import pytest @pytest.fixture(scope='module') diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py index b5037da8..df1db67e 100644 --- a/tests/functional/test_ROSE_ORIG_HOST.py +++ b/tests/functional/test_ROSE_ORIG_HOST.py @@ -59,16 +59,14 @@ """ -import pytest +from pathlib import Path import re import shutil - -from pathlib import Path from uuid import uuid4 from cylc.flow.hostuserutil import get_host from cylc.flow.pathutil import get_workflow_run_dir - +import pytest HOST = get_host() diff --git a/tests/functional/test_copy_config_file.py b/tests/functional/test_copy_config_file.py index ae02f35f..76ec3cd6 100644 --- a/tests/functional/test_copy_config_file.py +++ b/tests/functional/test_copy_config_file.py @@ -17,10 +17,10 @@ copy_config_file. """ -import pytest - from pathlib import Path +import pytest + from cylc.rose.entry_points import copy_config_file diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index ddefff41..79f3c9a6 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -16,19 +16,18 @@ """Functional tests for top-level function record_cylc_install_options and """ -import os -import pytest -import re - from itertools import product +import os from pathlib import Path -from pytest import param +import re from shlex import split from subprocess import run from types import SimpleNamespace -import cylc -from cylc.rose.entry_points import get_rose_vars, NotARoseSuiteException +import pytest +from pytest import param + +from cylc.rose.utilities import NotARoseSuiteException, get_rose_vars def envar_exporter(dict_): @@ -157,7 +156,7 @@ def test_warn_if_old_templating_set( ): """Test using unsupported root-dir config raises error.""" monkeypatch.setattr( - cylc.rose.utilities, 'cylc7_back_compat', compat_mode + 'cylc.rose.utilities.cylc7_back_compat', compat_mode ) (tmp_path / 'rose-suite.conf').write_text(f'[{rose_config}]') get_rose_vars(srcdir=tmp_path) diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py index db4c5e46..4190a092 100644 --- a/tests/functional/test_reinstall.py +++ b/tests/functional/test_reinstall.py @@ -27,19 +27,19 @@ - ~/cylc-run/temporary-id/opt/rose-suite-cylc-install.conf """ -import pytest -import shutil - from itertools import product from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.hostuserutil import get_host -from cylc.flow.pathutil import get_workflow_run_dir -from cylc.rose.utilities import ( - ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING as ROHIOS) from cylc.flow.install import reinstall_workflow +from cylc.flow.pathutil import get_workflow_run_dir +import pytest +from cylc.rose.utilities import ( + ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING as ROHIOS, +) HOST = get_host() diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py index 2883afb5..d6a332f0 100644 --- a/tests/functional/test_reinstall_clean.py +++ b/tests/functional/test_reinstall_clean.py @@ -27,17 +27,17 @@ - ~/cylc-run/temporary-id/opt/rose-suite-cylc-install.conf """ -import pytest -import shutil - from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.hostuserutil import get_host from cylc.flow.pathutil import get_workflow_run_dir -from cylc.rose.utilities import ( - ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING as ROHIOS) +import pytest +from cylc.rose.utilities import ( + ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING as ROHIOS, +) HOST = get_host() diff --git a/tests/functional/test_reinstall_fileinstall.py b/tests/functional/test_reinstall_fileinstall.py index 53f0968f..70ca41e8 100644 --- a/tests/functional/test_reinstall_fileinstall.py +++ b/tests/functional/test_reinstall_fileinstall.py @@ -17,14 +17,12 @@ trouble. """ -import pytest -import shutil - from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.pathutil import get_workflow_run_dir - +import pytest WORKFLOW_SRC = Path(__file__).parent / '14_reinstall_fileinstall' diff --git a/tests/functional/test_rose_fileinstall.py b/tests/functional/test_rose_fileinstall.py index e11a496f..5998b778 100644 --- a/tests/functional/test_rose_fileinstall.py +++ b/tests/functional/test_rose_fileinstall.py @@ -16,13 +16,12 @@ """Functional tests for top-level function record_cylc_install_options and """ -import pytest -import shutil - from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.pathutil import get_workflow_run_dir +import pytest @pytest.fixture diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 01caeb72..743455e2 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -63,24 +63,24 @@ """ import os -import pytest +from pathlib import Path import re +from shlex import split import shutil import subprocess - -from pathlib import Path -from shlex import split from types import SimpleNamespace from uuid import uuid4 -from cylc.flow.pathutil import get_workflow_run_dir from cylc.flow.hostuserutil import get_host - -from cylc.rose.stem import ( - RoseStemVersionException, rose_stem, _get_rose_stem_opts) - +from cylc.flow.pathutil import get_workflow_run_dir from metomi.rose.resource import ResourceLocator +import pytest +from cylc.rose.stem import ( + RoseStemVersionException, + get_rose_stem_opts, + rose_stem, +) HOST = get_host().split('.')[0] @@ -225,7 +225,7 @@ def rose_stem_run_template(setup_stem_repo, pytestconfig, monkeymodule): def _inner_fn(rose_stem_opts, verbosity=verbosity): monkeymodule.setattr('sys.argv', ['stem']) monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = _get_rose_stem_opts() + parser, opts = get_rose_stem_opts() [setattr(opts, key, val) for key, val in rose_stem_opts.items()] run_stem = SimpleNamespace() @@ -593,7 +593,7 @@ def test_incompatible_versions(setup_stem_repo, monkeymodule, caplog, capsys): monkeymodule.setattr('sys.argv', ['stem']) monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = _get_rose_stem_opts() + parser, opts = get_rose_stem_opts() [setattr(opts, key, val) for key, val in rose_stem_opts.items()] with pytest.raises( @@ -618,7 +618,7 @@ def test_project_not_in_keywords(setup_stem_repo, monkeymodule, capsys): monkeymodule.setattr('sys.argv', ['stem']) monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = _get_rose_stem_opts() + parser, opts = get_rose_stem_opts() [setattr(opts, key, val) for key, val in rose_stem_opts.items()] rose_stem(parser, opts) @@ -636,7 +636,7 @@ def test_picks_template_section(setup_stem_repo, monkeymodule, capsys): 'ROSE_STEM_VERSION=1\n' '[template_variables]\n' ) - parser, opts = _get_rose_stem_opts() + parser, opts = get_rose_stem_opts() rose_stem(parser, opts) _, err = capsys.readouterr() assert "[jinja2:suite.rc]' is deprecated" not in err diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py index bcd8d422..848815e3 100644 --- a/tests/unit/test_config_node.py +++ b/tests/unit/test_config_node.py @@ -15,25 +15,23 @@ # along with this program. If not, see . """Tests the plugin with Rose suite configurations via the Python API.""" -import pytest from types import SimpleNamespace from metomi.isodatetime.datetimeoper import DateTimeOperator from metomi.rose import __version__ as ROSE_VERSION -from metomi.rose.config import ( - ConfigNode, -) +from metomi.rose.config import ConfigNode from metomi.rose.config_processor import ConfigProcessError +import pytest from cylc.rose.utilities import ( ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, - get_rose_vars_from_config_node, + MultipleTemplatingEnginesError, add_cylc_install_to_rose_conf_node_opts, deprecation_warnings, dump_rose_log, - identify_templating_section, + get_rose_vars_from_config_node, id_templating_section, - MultipleTemplatingEnginesError + identify_templating_section, ) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 3718785b..768ee584 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -20,27 +20,24 @@ """ +from io import StringIO import os -import pytest -from pytest import param - from types import SimpleNamespace -from io import StringIO from cylc.flow.hostuserutil import get_host +from metomi.rose.config import ConfigLoader +import pytest +from pytest import param + +from cylc.rose.entry_points import get_rose_vars from cylc.rose.utilities import ( + MultipleTemplatingEnginesError, get_cli_opts_node, merge_opts, merge_rose_cylc_suite_install_conf, rose_config_exists, rose_config_tree_loader, - MultipleTemplatingEnginesError ) -from cylc.rose.entry_points import ( - get_rose_vars, -) -from metomi.rose.config import ConfigLoader - HOST = get_host() diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index eb352672..967eeb35 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -20,26 +20,26 @@ ``cylc install -D [fileinstall:myfile]example`` will lead to the correct file installation. """ -import pytest from pathlib import Path from types import SimpleNamespace +from cylc.flow.hostuserutil import get_host from metomi.isodatetime.datetimeoper import DateTimeOperator +from metomi.rose.config import ConfigLoader +from metomi.rose.config_tree import ConfigTree +import pytest -import cylc -from cylc.flow.hostuserutil import get_host from cylc.rose.entry_points import ( - record_cylc_install_options, rose_fileinstall, post_install, - copy_config_file + copy_config_file, + post_install, + record_cylc_install_options, ) +from cylc.rose.fileinstall import rose_fileinstall from cylc.rose.utilities import ( ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, - MultipleTemplatingEnginesError + MultipleTemplatingEnginesError, ) -from metomi.rose.config import ConfigLoader -from metomi.rose.config_tree import ConfigTree - HOST = get_host() @@ -360,11 +360,13 @@ def fakenode(_, __): return tree monkeypatch.setattr( - cylc.rose.entry_points, 'rose_config_tree_loader', - fakenode + 'cylc.rose.utilities.rose_config_tree_loader', + fakenode, ) monkeypatch.setattr( - cylc.rose.entry_points, "rose_config_exists", lambda x, y: True) + 'cylc.rose.fileinstall.rose_config_exists', + lambda x, y: True, + ) with pytest.raises(FileNotFoundError): rose_fileinstall(srcdir=tmp_path, rundir='/oiruhgaqhnaigujhj') diff --git a/tests/unit/test_generic_utils.py b/tests/unit/test_generic_utils.py index 82607da3..3005769c 100644 --- a/tests/unit/test_generic_utils.py +++ b/tests/unit/test_generic_utils.py @@ -16,10 +16,10 @@ """Test generic ultilities """ -import pytest - from pathlib import Path +import pytest + from cylc.rose.utilities import paths_to_pathlib diff --git a/tests/unit/test_platform_utils.py b/tests/unit/test_platform_utils.py index 8ad68d78..04433d03 100644 --- a/tests/unit/test_platform_utils.py +++ b/tests/unit/test_platform_utils.py @@ -19,24 +19,22 @@ """ import os -import pytest from pathlib import Path from shutil import rmtree import sqlite3 from uuid import uuid4 -from cylc.rose.platform_utils import ( - get_platform_from_task_def, - get_platforms_from_task_jobs -) - from cylc.flow import __version__ as cylc_version from cylc.flow.cfgspec.globalcfg import SPEC from cylc.flow.parsec.config import ParsecConfig -from cylc.flow.pathutil import ( - get_workflow_run_pub_db_path -) +from cylc.flow.pathutil import get_workflow_run_pub_db_path from cylc.flow.workflow_db_mgr import CylcWorkflowDAO +import pytest + +from cylc.rose.platform_utils import ( + get_platform_from_task_def, + get_platforms_from_task_jobs, +) MOCK_GLBL_CFG = ( 'cylc.flow.platforms.glbl_cfg', diff --git a/tests/unit/test_rose_opts.py b/tests/unit/test_rose_opts.py index 14bb9884..a05658dd 100644 --- a/tests/unit/test_rose_opts.py +++ b/tests/unit/test_rose_opts.py @@ -16,14 +16,13 @@ """Functional tests for top-level function record_cylc_install_options and """ -import pytest -import shutil - from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.hostuserutil import get_host from cylc.flow.pathutil import get_workflow_run_dir +import pytest from cylc.rose.utilities import ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index 8a73015b..f58dd47e 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -16,26 +16,25 @@ """Functional tests for top-level function record_cylc_install_options and """ -import cylc.rose -import pytest -from pytest import param from types import SimpleNamespace from typing import Any, Tuple +from metomi.rose.fs_util import FileSystemUtil +from metomi.rose.popen import RosePopener +from metomi.rose.reporter import Reporter +import pytest +from pytest import param + +import cylc.rose from cylc.rose.stem import ( - _get_rose_stem_opts, ProjectNotFoundException, RoseStemVersionException, RoseSuiteConfNotFoundException, StemRunner, + get_rose_stem_opts, get_source_opt_from_args, ) -from metomi.rose.reporter import Reporter -from metomi.rose.popen import RosePopener -from metomi.rose.fs_util import FileSystemUtil - - Fixture = Any @@ -440,7 +439,7 @@ def test_process_template_engine_set_correctly(monkeypatch, language, expect): # We are not interested in these checks, just in the defines # created by the process method. - stemrunner = StemRunner(_get_rose_stem_opts()[1]) + stemrunner = StemRunner(get_rose_stem_opts()[1]) stemrunner._ascertain_project = lambda _: ['', '', '', '', ''] stemrunner._this_suite = lambda: '.' stemrunner._check_suite_version = lambda _: '1' From e4f1cbbcab9b8cb67d67480e1f9ca90364b056d0 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:02:18 +0000 Subject: [PATCH 02/12] tests: fix os.environ test interaction --- tests/functional/test_pre_configure.py | 27 +++++++++----------------- tests/unit/test_config_tree.py | 22 +++++++++------------ 2 files changed, 18 insertions(+), 31 deletions(-) diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index 79f3c9a6..baecc6df 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -30,11 +30,6 @@ from cylc.rose.utilities import NotARoseSuiteException, get_rose_vars -def envar_exporter(dict_): - for key, val in dict_.items(): - os.environ[key] = val - - @pytest.mark.parametrize( 'srcdir, expect', [ @@ -82,40 +77,36 @@ def test_validate_fail(srcdir, expect, cylc_validate_cli): ('09_template_vars_vanilla', {'XYZ': 'xyz'}, None), ], ) -def test_validate(tmp_path, srcdir, envvars, args, cylc_validate_cli): - if envvars is not None: - envvars = os.environ.update(envvars) +def test_validate(monkeypatch, srcdir, envvars, args, cylc_validate_cli): + for key, value in (envvars or {}).items(): + monkeypatch.setenv(key, value) srcdir = Path(__file__).parent / srcdir validate = cylc_validate_cli(str(srcdir), args) assert validate.ret == 0 @pytest.mark.parametrize( - 'srcdir, envvars, args', + 'srcdir, envvars', [ - ('00_jinja2_basic', None, None), - ('01_empy', None, None), + ('00_jinja2_basic', None), + ('01_empy', None), ( '04_opts_set_from_env', {'ROSE_SUITE_OPT_CONF_KEYS': 'Gaelige'}, - None ), ( '05_opts_set_from_rose_suite_conf', {'ROSE_SUITE_OPT_CONF_KEYS': ''}, - None ), - ('06_jinja2_thorough', {'XYZ': 'xyz'}, None), + ('06_jinja2_thorough', {'XYZ': 'xyz'}), ], ) -def test_process(tmp_path, srcdir, envvars, args): - if envvars is not None: - envvars = os.environ.update(envvars) +def test_process(srcdir, envvars): srcdir = Path(__file__).parent / srcdir result = run( ['cylc', 'view', '-p', str(srcdir)], capture_output=True, - env=envvars + env={**os.environ, **(envvars or {})} ).stdout.decode() expect = (srcdir / 'processed.conf.control').read_text() assert expect == result diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 768ee584..fae84e5d 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -13,15 +13,9 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Tests the plugin with Rose suite configurations on the filesystem. - -Warning: - These tests share the same os.environ so may interact. - -""" +"""Tests the plugin with Rose suite configurations on the filesystem.""" from io import StringIO -import os from types import SimpleNamespace from cylc.flow.hostuserutil import get_host @@ -155,6 +149,7 @@ def wrapped_function(section): ] ) def test_get_rose_vars( + monkeypatch, rose_config_template, override, section, @@ -174,10 +169,10 @@ def test_get_rose_vars( opt_conf_keys=[], defines=[], rose_template_vars=[] ) if override == 'environment': - os.environ['ROSE_SUITE_OPT_CONF_KEYS'] = "gravy" + monkeypatch.setenv('ROSE_SUITE_OPT_CONF_KEYS', 'gravy') else: # Prevent externally set environment var breaking tests. - os.environ['ROSE_SUITE_OPT_CONF_KEYS'] = "" + monkeypatch.setenv('ROSE_SUITE_OPT_CONF_KEYS', '') if override == 'CLI': options.opt_conf_keys = ["chips"] elif override == 'override': @@ -206,9 +201,9 @@ def test_get_rose_vars_env_section(tmp_path): ) == 'Spaniel' -def test_get_rose_vars_expansions(tmp_path): +def test_get_rose_vars_expansions(monkeypatch, tmp_path): """Check that variables are expanded correctly.""" - os.environ['XYZ'] = "xyz" + monkeypatch.setenv('XYZ', 'xyz') (tmp_path / "rose-suite.conf").write_text( "[env]\n" "FOO=a\n" @@ -298,6 +293,7 @@ def test_get_rose_vars_fail_if_empy_AND_jinja2(tmp_path): ] ) def test_rose_config_tree_loader( + monkeypatch, rose_config_template, override, section, @@ -316,10 +312,10 @@ def test_rose_config_tree_loader( """ options = None if override == 'environment': - os.environ['ROSE_SUITE_OPT_CONF_KEYS'] = "gravy" + monkeypatch.setenv('ROSE_SUITE_OPT_CONF_KEYS', 'gravy') else: # Prevent externally set environment var breaking tests. - os.environ['ROSE_SUITE_OPT_CONF_KEYS'] = "" + monkeypatch.setenv('ROSE_SUITE_OPT_CONF_KEYS', '') if opts_format == 'list': conf_keys = ['chips'] else: From 9bd514374d4fe8c38adab55b02f1596eda5e88f3 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:05:53 +0000 Subject: [PATCH 03/12] tests: fix accidentally skipped tests * `['a'].sort()` returns `None` not `['a']` * So `assert a.sort() == b.sort()` will always be `True`. --- tests/unit/test_config_tree.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index fae84e5d..393fb363 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -228,10 +228,10 @@ 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 list(rose_vars['env'].keys()).sort() == [ + assert sorted(rose_vars['env']) == sorted([ 'ROSE_ORIG_HOST', 'ROSE_VERSION', - ].sort() + ]) def test_get_rose_vars_jinja2_ROSE_VARS(tmp_path): @@ -240,13 +240,13 @@ def test_get_rose_vars_jinja2_ROSE_VARS(tmp_path): "[jinja2:suite.rc]" ) rose_vars = get_rose_vars(tmp_path) - assert list(rose_vars['template_variables'][ - 'ROSE_SUITE_VARIABLES' - ].keys()).sort() == [ + assert sorted( + rose_vars['template_variables']['ROSE_SUITE_VARIABLES'] + ) == sorted([ 'ROSE_VERSION', 'ROSE_ORIG_HOST', 'ROSE_SUITE_VARIABLES' - ].sort() + ]) def test_get_rose_vars_fail_if_empy_AND_jinja2(tmp_path): From 2f33ba1e1ae78f299e3d82050a74e3bbf043efad Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:13:49 +0000 Subject: [PATCH 04/12] utils: make environ arg optional --- cylc/rose/utilities.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 853d3fef..e56f854f 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -76,7 +76,7 @@ class InvalidDefineError(CylcError): ... -def get_rose_vars_from_config_node(config, config_node, environ): +def get_rose_vars_from_config_node(config, config_node, environ=os.environ): """Load template variables from a Rose config node. This uses only the provided config node and environment variables @@ -88,7 +88,7 @@ def get_rose_vars_from_config_node(config, config_node, environ): config_node (metomi.rose.config.ConfigNode): Configuration node representing the Rose suite configuration. environ (dict): - Dictionary of environment variables + Dictionary of environment variables (for testing). """ # Don't allow multiple templating sections. @@ -865,7 +865,6 @@ def get_rose_vars(srcdir=None, opts=None): get_rose_vars_from_config_node( config, config_tree.node, - os.environ ) # Export environment vars From 1ee64621ef0eeef7fcb9ff5acd5d8b57ab4f723f Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:28:19 +0000 Subject: [PATCH 05/12] utils: remove unused "opts" arg in "rose_config_exists" --- cylc/rose/entry_points.py | 2 +- cylc/rose/fileinstall.py | 2 +- cylc/rose/utilities.py | 10 ++-------- tests/unit/test_config_tree.py | 20 +++++--------------- tests/unit/test_functional_post_install.py | 2 +- 5 files changed, 10 insertions(+), 26 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index f169be90..7cf88a7b 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -35,7 +35,7 @@ def post_install(srcdir=None, opts=None, rundir=None): """Run after Cylc file installation has completed.""" from cylc.rose.fileinstall import rose_fileinstall - if not rose_config_exists(srcdir, opts): + if not rose_config_exists(srcdir): return False srcdir, rundir = paths_to_pathlib([srcdir, rundir]) results = {} diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 4c9c83b0..1e9dc0ea 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -30,7 +30,7 @@ def rose_fileinstall(srcdir=None, opts=None, rundir=None): rundir (pathlib.Path) """ - if not rose_config_exists(rundir, opts): + if not rose_config_exists(rundir): return False # Load the config tree diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index e56f854f..69fc9370 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -43,9 +43,6 @@ from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros -if TYPE_CHECKING: - from optparse import Values - SECTIONS = {'jinja2:suite.rc', 'empy:suite.rc', 'template variables'} SET_BY_CYLC = 'set by Cylc' @@ -229,14 +226,11 @@ def id_templating_section( return templating -def rose_config_exists( - srcdir: Union[Path, str, None], opts: 'Values' -) -> bool: +def rose_config_exists(srcdir: Union[Path, str, None]) -> bool: """Do opts or srcdir contain a rose config? Args: srcdir: location to test. - opts: Cylc Rose options, which might contain config items. Returns: True if a ``rose-suite.conf`` exists, or option config items have @@ -844,7 +838,7 @@ def get_rose_vars(srcdir=None, opts=None): } # Return a blank config dict if srcdir does not exist - if not rose_config_exists(srcdir, opts): + if not rose_config_exists(srcdir): if ( getattr(opts, "opt_conf_keys", None) or getattr(opts, "defines", None) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 393fb363..89eb608c 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -62,31 +62,21 @@ def test_node_stripper(): assert result == {'foo': 'bar'} -def test_rose_config_exists_no_dir(tmp_path): - assert rose_config_exists(None, SimpleNamespace( - opt_conf_keys=None, defines=[], rose_template_vars=[]) - ) is False +def test_rose_config_exists_no_dir(): + assert not rose_config_exists(None) def test_rose_config_exists_no_rose_suite_conf(tmp_path): - assert rose_config_exists( - tmp_path, SimpleNamespace( - opt_conf_keys=None, defines=[], rose_template_vars=[] - ) - ) is False + assert not rose_config_exists(tmp_path) def test_rose_config_exists_nonexistant_dir(tmp_path): - assert rose_config_exists( - tmp_path / "non-existant-folder", SimpleNamespace( - opt_conf_keys='', defines=[], rose_template_vars=[] - ) - ) is False + assert not rose_config_exists(tmp_path / "non-existant-folder") def test_rose_config_exists_true(tmp_path): (tmp_path / "rose-suite.conf").touch() - assert rose_config_exists(tmp_path, SimpleNamespace()) is True + assert rose_config_exists(tmp_path) @pytest.fixture diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 967eeb35..0e293b3b 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -365,7 +365,7 @@ def fakenode(_, __): ) monkeypatch.setattr( 'cylc.rose.fileinstall.rose_config_exists', - lambda x, y: True, + lambda x: True, ) with pytest.raises(FileNotFoundError): rose_fileinstall(srcdir=tmp_path, rundir='/oiruhgaqhnaigujhj') From ba070631909b813695488f5a54e9c57f98a7361e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:31:55 +0000 Subject: [PATCH 06/12] fileinstall: remove unused "srcdir" argument --- cylc/rose/entry_points.py | 4 +--- cylc/rose/fileinstall.py | 2 +- tests/unit/test_functional_post_install.py | 17 +++++------------ 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 7cf88a7b..3639cf48 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -43,9 +43,7 @@ def post_install(srcdir=None, opts=None, rundir=None): results['record_install'] = record_cylc_install_options( srcdir=srcdir, opts=opts, rundir=rundir ) - results['fileinstall'] = rose_fileinstall( - srcdir=srcdir, opts=opts, rundir=rundir - ) + results['fileinstall'] = rose_fileinstall(opts=opts, rundir=rundir) # Finally dump a log of the rose-conf in its final state. if results['fileinstall']: dump_rose_log(rundir=rundir, node=results['fileinstall']) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 1e9dc0ea..c0eb2819 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -21,7 +21,7 @@ from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader -def rose_fileinstall(srcdir=None, opts=None, rundir=None): +def rose_fileinstall(opts=None, rundir=None): """Call Rose Fileinstall. Args: diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 0e293b3b..dfcb57cf 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -65,11 +65,6 @@ def test_no_rose_suite_conf_in_devdir(tmp_path): assert result is False -def test_rose_fileinstall_no_config_in_folder(): - # It returns false if no rose-suite.conf - assert rose_fileinstall(Path('/dev/null')) is False - - def test_rose_fileinstall_uses_rose_template_vars(tmp_path): # Setup source and destination dirs, including the file ``installme``: srcdir = tmp_path / 'source' @@ -89,7 +84,7 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path): # Run both record_cylc_install options and fileinstall. record_cylc_install_options(opts=opts, rundir=destdir) - rose_fileinstall(srcdir, opts, destdir) + rose_fileinstall(opts, destdir) assert ((destdir / 'installedme').read_text() == 'Galileo No! We will not let you go.' ) @@ -259,9 +254,7 @@ def fake(*arg, **kwargs): rundir=testdir, opts=opts, srcdir=testdir ) ) - rose_fileinstall( - rundir=testdir, opts=opts, srcdir=testdir - ) + rose_fileinstall(rundir=testdir, opts=opts) ritems = sorted([i.relative_to(refdir) for i in refdir.rglob('*')]) titems = sorted([i.relative_to(testdir) for i in testdir.rglob('*')]) assert titems == ritems @@ -287,7 +280,7 @@ def test_functional_rose_database_dumped_correctly(tmp_path): "[file:Gnu]\nsrc=nicest_work_of.nature\n" ) (rundir / 'cylc.flow').touch() - rose_fileinstall(srcdir=srcdir, rundir=rundir) + rose_fileinstall(rundir=rundir) assert (rundir / '.rose-config_processors-file.db').is_file() @@ -300,7 +293,7 @@ def test_functional_rose_database_dumped_errors(tmp_path): "[file:Gnu]\nsrc=nicest_work_of.nature\n" ) (srcdir / 'cylc.flow').touch() - assert rose_fileinstall(srcdir=Path('/this/path/goes/nowhere')) is False + assert rose_fileinstall() is False @pytest.mark.parametrize( @@ -368,7 +361,7 @@ def fakenode(_, __): lambda x: True, ) with pytest.raises(FileNotFoundError): - rose_fileinstall(srcdir=tmp_path, rundir='/oiruhgaqhnaigujhj') + rose_fileinstall(rundir='/oiruhgaqhnaigujhj') def test_cylc_no_rose(tmp_path): From aa79c43dd073069cbe8fa48bc4aee98b11c482ba Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:50:38 +0000 Subject: [PATCH 07/12] entry_points: catch None values earlier * Avoid having to handle None values for srcdir/rundir throughout the codebase by catching them earlier. --- cylc/rose/entry_points.py | 30 ++++++++++++---- cylc/rose/fileinstall.py | 7 +++- cylc/rose/utilities.py | 42 ++++------------------ tests/functional/test_copy_config_file.py | 40 +++++---------------- tests/unit/test_config_tree.py | 4 --- tests/unit/test_functional_post_install.py | 30 ++++------------ tests/unit/test_generic_utils.py | 42 ---------------------- 7 files changed, 51 insertions(+), 144 deletions(-) delete mode 100644 tests/unit/test_generic_utils.py diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 3639cf48..1f8680bd 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -15,35 +15,51 @@ # along with this program. If not, see . """Top level module providing entry point functions.""" +from pathlib import Path +from typing import Union + from cylc.rose.utilities import ( copy_config_file, dump_rose_log, get_rose_vars, - paths_to_pathlib, record_cylc_install_options, rose_config_exists, ) -def pre_configure(srcdir=None, opts=None, rundir=None): +def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: """Run before the Cylc configuration is read.""" - srcdir, rundir = paths_to_pathlib([srcdir, rundir]) + if not srcdir: + # not sure how this could happen + return { + 'env': {}, + 'template_variables': {}, + 'templating_detected': None + } + srcdir: Path = Path(srcdir) return get_rose_vars(srcdir=srcdir, opts=opts) -def post_install(srcdir=None, opts=None, rundir=None): +def post_install(srcdir=None, opts=None, rundir=None) -> Union[dict, bool]: """Run after Cylc file installation has completed.""" from cylc.rose.fileinstall import rose_fileinstall - if not rose_config_exists(srcdir): + if ( + not srcdir + or not rundir + or not rose_config_exists(srcdir) + ): + # nothing to do here return False - srcdir, rundir = paths_to_pathlib([srcdir, rundir]) + srcdir: Path = Path(srcdir) + rundir: Path = Path(rundir) + results = {} copy_config_file(srcdir=srcdir, rundir=rundir) results['record_install'] = record_cylc_install_options( srcdir=srcdir, opts=opts, rundir=rundir ) - results['fileinstall'] = rose_fileinstall(opts=opts, rundir=rundir) + results['fileinstall'] = rose_fileinstall(rundir, opts) # Finally dump a log of the rose-conf in its final state. if results['fileinstall']: dump_rose_log(rundir=rundir, node=results['fileinstall']) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index c0eb2819..514d06d0 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -17,11 +17,16 @@ """Utilities related to performing Rose file installation.""" import os +from typing import TYPE_CHECKING from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader +if TYPE_CHECKING: + from pathlib import Path + from cylc.flow.option_parser import Values -def rose_fileinstall(opts=None, rundir=None): + +def rose_fileinstall(rundir: 'Path', opts: 'Values'): """Call Rose Fileinstall. Args: diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 69fc9370..c4019cc3 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -24,7 +24,7 @@ import re import shlex import shutil -from typing import TYPE_CHECKING, Any, List, Optional, Tuple, Union +from typing import Any, List, Optional, Tuple, Union from cylc.flow import LOG from cylc.flow.exceptions import CylcError @@ -226,22 +226,17 @@ def id_templating_section( return templating -def rose_config_exists(srcdir: Union[Path, str, None]) -> bool: - """Do opts or srcdir contain a rose config? +def rose_config_exists(dir_: Path) -> bool: + """Does dir_ a rose config? Args: - srcdir: location to test. + dir_: location to test. Returns: True if a ``rose-suite.conf`` exists, or option config items have been set. """ - # Return false if source dir doesn't exist. - if srcdir is None: - return False - - # Return true if and only if the rose suite.conf exists. - return Path(srcdir, 'rose-suite.conf').is_file() + return (dir_ / 'rose-suite.conf').is_file() def rose_config_tree_loader(srcdir=None, opts=None): @@ -662,16 +657,6 @@ def dump_rose_log(rundir, node): return rel_path -def paths_to_pathlib(paths): - """Convert paths to pathlib - """ - return [ - Path(path) if path is not None - else None - for path in paths - ] - - def override_this_variable(node, section, variable): """Variable exists in this section of the config and should be replaced because it is a standard variable. @@ -959,17 +944,14 @@ def record_cylc_install_options( def copy_config_file( - srcdir=None, - opts=None, - rundir=None, + srcdir: Path, + rundir: Path, ): """Copy the ``rose-suite.conf`` from a workflow source to run directory. Args: srcdir (pathlib.Path | or str): Source Path of Cylc install. - opts: - Not used in this function, but requried for consistent entry point. rundir (pathlib.Path | or str): Destination path of Cylc install - the workflow rundir. @@ -977,16 +959,6 @@ def copy_config_file( True if ``rose-suite.conf`` has been installed. False if insufficiant information to install file given. """ - if ( - rundir is None or - srcdir is None - ): - raise FileNotFoundError( - "This plugin requires both source and rundir to exist." - ) - - rundir = Path(rundir) - srcdir = Path(srcdir) srcdir_rose_conf = srcdir / 'rose-suite.conf' rundir_rose_conf = rundir / 'rose-suite.conf' diff --git a/tests/functional/test_copy_config_file.py b/tests/functional/test_copy_config_file.py index 76ec3cd6..caed1a13 100644 --- a/tests/functional/test_copy_config_file.py +++ b/tests/functional/test_copy_config_file.py @@ -19,43 +19,21 @@ from pathlib import Path -import pytest - from cylc.rose.entry_points import copy_config_file -@pytest.mark.parametrize( - 'sources, inputs, expect', - [ - ( - # Valid sourcedir with rose file, rose file at dest: - { - 'src/rose-suite.conf': '[env]\nFOO=2', - 'dest/rose-suite.conf': '[env]\nFOO=1' - }, - {'srcdir': 'src', 'rundir': 'dest'}, - True - ) - ] -) -def test_basic(tmp_path, sources, inputs, expect): +def test_basic(tmp_path): # Create files - for fname, content in sources.items(): + for fname, content in ( + ('src/rose-suite.conf', '[env]\nFOO=2'), + ('dest/rose-suite.conf', '[env]\nFOO=1'), + ): fname = Path(tmp_path / fname) fname.parent.mkdir(parents=True, exist_ok=True) fname.write_text(content) - # Flesh-out filepaths. - inputs = { - kwarg: Path(tmp_path / path) for kwarg, path in inputs.items() - if path is not None - } - # Test - if expect: - assert copy_config_file(**inputs) == expect - assert (Path(tmp_path / 'src/rose-suite.conf').read_text() == - Path(tmp_path / 'dest/rose-suite.conf').read_text() - ) - else: - assert copy_config_file(**inputs) == expect + assert copy_config_file(tmp_path / 'src', tmp_path / 'dest') + assert Path(tmp_path / 'src/rose-suite.conf').read_text() == ( + Path(tmp_path / 'dest/rose-suite.conf').read_text() + ) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 89eb608c..7557aa18 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -62,10 +62,6 @@ def test_node_stripper(): assert result == {'foo': 'bar'} -def test_rose_config_exists_no_dir(): - assert not rose_config_exists(None) - - def test_rose_config_exists_no_rose_suite_conf(tmp_path): assert not rose_config_exists(tmp_path) diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index dfcb57cf..9371714a 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -21,7 +21,6 @@ installation. """ -from pathlib import Path from types import SimpleNamespace from cylc.flow.hostuserutil import get_host @@ -84,7 +83,7 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path): # Run both record_cylc_install options and fileinstall. record_cylc_install_options(opts=opts, rundir=destdir) - rose_fileinstall(opts, destdir) + rose_fileinstall(destdir, opts) assert ((destdir / 'installedme').read_text() == 'Galileo No! We will not let you go.' ) @@ -254,7 +253,7 @@ def fake(*arg, **kwargs): rundir=testdir, opts=opts, srcdir=testdir ) ) - rose_fileinstall(rundir=testdir, opts=opts) + rose_fileinstall(testdir, opts) ritems = sorted([i.relative_to(refdir) for i in refdir.rglob('*')]) titems = sorted([i.relative_to(testdir) for i in testdir.rglob('*')]) assert titems == ritems @@ -280,22 +279,11 @@ def test_functional_rose_database_dumped_correctly(tmp_path): "[file:Gnu]\nsrc=nicest_work_of.nature\n" ) (rundir / 'cylc.flow').touch() - rose_fileinstall(rundir=rundir) + rose_fileinstall(rundir, SimpleNamespace()) assert (rundir / '.rose-config_processors-file.db').is_file() -def test_functional_rose_database_dumped_errors(tmp_path): - srcdir = (tmp_path / 'srcdir') - srcdir.mkdir() - (srcdir / 'nicest_work_of.nature').touch() - (srcdir / 'rose-suite.conf').write_text( - "[file:Gnu]\nsrc=nicest_work_of.nature\n" - ) - (srcdir / 'cylc.flow').touch() - assert rose_fileinstall() is False - - @pytest.mark.parametrize( ( 'opts, files, expect' @@ -361,7 +349,7 @@ def fakenode(_, __): lambda x: True, ) with pytest.raises(FileNotFoundError): - rose_fileinstall(rundir='/oiruhgaqhnaigujhj') + rose_fileinstall('/oiruhgaqhnaigujhj', SimpleNamespace()) def test_cylc_no_rose(tmp_path): @@ -371,12 +359,6 @@ def test_cylc_no_rose(tmp_path): assert post_install(srcdir=tmp_path, rundir=tmp_path) is False -def test_copy_config_file_fails(): - """It fails when source or rundir not specified.""" - with pytest.raises(FileNotFoundError, match='both source and rundir'): - copy_config_file() - - -def test_copy_config_file_fails2(): +def test_copy_config_file_fails(tmp_path): """It fails if source not a rose suite.""" - copy_config_file(srcdir='/foo', rundir='/bar') + copy_config_file(tmp_path, tmp_path) diff --git a/tests/unit/test_generic_utils.py b/tests/unit/test_generic_utils.py deleted file mode 100644 index 3005769c..00000000 --- a/tests/unit/test_generic_utils.py +++ /dev/null @@ -1,42 +0,0 @@ -# THIS FILE IS PART OF THE ROSE-CYLC PLUGIN FOR THE CYLC WORKFLOW ENGINE. -# Copyright (C) NIWA & British Crown (Met Office) & Contributors. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -"""Test generic ultilities -""" - -from pathlib import Path - -import pytest - -from cylc.rose.utilities import paths_to_pathlib - - -@pytest.mark.parametrize( - 'paths, expect', - [ - # None is passed through: - ([None, None], [None, None]), - # Path as string is made Path: - (['/foot/path/', None], [Path('/foot/path'), None]), - # Path as Path left alone: - ([Path('/cycle/path/'), None], [Path('/cycle/path'), None]), - # Different starting types re-typed independently: - ( - [Path('/cycle/path/'), '/bridle/path'], - [Path('/cycle/path'), Path('/bridle/path')]), - ] -) -def test_paths_to_pathlib(paths, expect): - assert paths_to_pathlib(paths) == expect From d1ff77dac28807ff928604a0cebaf404bdad2007 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 17:50:12 +0000 Subject: [PATCH 08/12] utils: split the loading and processing of the config * A future rose stem interface will need to inject itself between config loading and processing. * So colocate the plugin processing logic into one function. --- cylc/rose/entry_points.py | 18 ++++- cylc/rose/fileinstall.py | 1 + cylc/rose/stem.py | 18 ++++- cylc/rose/utilities.py | 103 ++++++++++++++----------- tests/functional/test_pre_configure.py | 8 +- tests/unit/test_config_node.py | 31 ++++---- tests/unit/test_config_tree.py | 57 +++++++++----- tests/unit/test_rose_stem_units.py | 10 ++- 8 files changed, 154 insertions(+), 92 deletions(-) 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', From 0f376d111e8616ec47c575f3be7f1493526bbc3d Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 1 Nov 2023 11:10:19 +0000 Subject: [PATCH 09/12] entry_points: remove unused results return value --- cylc/rose/entry_points.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 2389a139..644e311e 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -16,7 +16,6 @@ """Top level module providing entry point functions.""" from pathlib import Path -from typing import Union from cylc.rose.utilities import ( copy_config_file, @@ -52,7 +51,7 @@ def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: return plugin_result -def post_install(srcdir=None, opts=None, rundir=None) -> Union[dict, bool]: +def post_install(srcdir=None, opts=None, rundir=None) -> bool: """Run after Cylc file installation has completed.""" from cylc.rose.fileinstall import rose_fileinstall @@ -66,17 +65,20 @@ def post_install(srcdir=None, opts=None, rundir=None) -> Union[dict, bool]: srcdir: Path = Path(srcdir) rundir: Path = Path(rundir) - results = {} + # transfer the rose-suite.conf file copy_config_file(srcdir=srcdir, rundir=rundir) - results['record_install'] = record_cylc_install_options( + + # write cylc-install CLI options to an optional config + record_cylc_install_options( srcdir=srcdir, opts=opts, rundir=rundir ) - results['fileinstall'] = rose_fileinstall(rundir, opts) - # Finally dump a log of the rose-conf in its final state. - if results['fileinstall']: - dump_rose_log(rundir=rundir, node=results['fileinstall']) - return results + # perform file installation + config_node = rose_fileinstall(rundir, opts) + if config_node: + dump_rose_log(rundir=rundir, node=config_node) + + return True def rose_stem(): From 9bef3c8ed3fe90d9262b3a3c55c799ed85007513 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 1 Nov 2023 11:42:42 +0000 Subject: [PATCH 10/12] entry_points: remove type uncertainty * Entry points accept missing or None values for arguments, however, this can never happen in practice. * Set the appropriate type hints and remove code paths which could not get executed. --- cylc/rose/entry_points.py | 27 +++--- cylc/rose/fileinstall.py | 11 ++- cylc/rose/utilities.py | 102 +++++++++++---------- tests/unit/test_config_tree.py | 3 +- tests/unit/test_functional_post_install.py | 18 +--- 5 files changed, 80 insertions(+), 81 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 644e311e..98b6de37 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -16,6 +16,7 @@ """Top level module providing entry point functions.""" from pathlib import Path +from typing import TYPE_CHECKING from cylc.rose.utilities import ( copy_config_file, @@ -27,8 +28,11 @@ rose_config_exists, ) +if TYPE_CHECKING: + from cylc.flow.option_parsers import Values -def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: + +def pre_configure(srcdir: Path, opts: 'Values') -> dict: """Run before the Cylc configuration is read.""" if not srcdir: # not sure how this could happen @@ -51,32 +55,25 @@ def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: return plugin_result -def post_install(srcdir=None, opts=None, rundir=None) -> bool: +def post_install(srcdir: Path, rundir: str, opts: 'Values') -> bool: """Run after Cylc file installation has completed.""" from cylc.rose.fileinstall import rose_fileinstall - if ( - not srcdir - or not rundir - or not rose_config_exists(srcdir) - ): + if not rose_config_exists(srcdir): # nothing to do here return False - srcdir: Path = Path(srcdir) - rundir: Path = Path(rundir) + _rundir: Path = Path(rundir) # transfer the rose-suite.conf file - copy_config_file(srcdir=srcdir, rundir=rundir) + copy_config_file(srcdir=srcdir, rundir=_rundir) # write cylc-install CLI options to an optional config - record_cylc_install_options( - srcdir=srcdir, opts=opts, rundir=rundir - ) + record_cylc_install_options(srcdir, _rundir, opts) # perform file installation - config_node = rose_fileinstall(rundir, opts) + config_node = rose_fileinstall(_rundir, opts) if config_node: - dump_rose_log(rundir=rundir, node=config_node) + dump_rose_log(rundir=_rundir, node=config_node) return True diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 1be4cfdd..986ab05f 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -17,16 +17,20 @@ """Utilities related to performing Rose file installation.""" import os -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Union from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader if TYPE_CHECKING: from pathlib import Path - from cylc.flow.option_parser import Values + from cylc.flow.option_parsers import Values + from metomi.rose.config import ConfigNode -def rose_fileinstall(rundir: 'Path', opts: 'Values'): +def rose_fileinstall( + rundir: 'Path', + opts: 'Values', +) -> 'Union[ConfigNode, bool]': """Call Rose Fileinstall. Args: @@ -39,7 +43,6 @@ 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/utilities.py b/cylc/rose/utilities.py index b2ea9905..15540b21 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, Dict, List, Optional, Tuple, Union +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union from cylc.flow import LOG from cylc.flow.exceptions import CylcError @@ -44,6 +44,9 @@ from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros +if TYPE_CHECKING: + from cylc.flow.option_parsers import Values + SECTIONS = {'jinja2:suite.rc', 'empy:suite.rc', 'template variables'} SET_BY_CYLC = 'set by Cylc' @@ -255,7 +258,10 @@ def rose_config_exists(dir_: Path) -> bool: return (dir_ / 'rose-suite.conf').is_file() -def rose_config_tree_loader(srcdir=None, opts=None): +def rose_config_tree_loader( + srcdir: Path, + opts: 'Optional[Values]', +) -> ConfigTree: """Get a rose config tree from srcdir. Args: @@ -298,7 +304,7 @@ def rose_config_tree_loader(srcdir=None, opts=None): # Reload the Config using the suite_ variables. # (we can't do this first time around because we have no idea what the # templating section is.) - if getattr(opts, 'rose_template_vars', None): + if opts and getattr(opts, 'rose_template_vars', None): template_section = identify_templating_section(config_tree.node) for template_var in opts.rose_template_vars or []: redefinitions.append(f'[{template_section}]{template_var}') @@ -451,7 +457,7 @@ def parse_cli_defines(define: str) -> Union[ return (keys, match['value'], match['state']) -def get_cli_opts_node(opts=None, srcdir=None): +def get_cli_opts_node(srcdir: Path, opts: 'Values'): """Create a ConfigNode representing options set on the command line. Args: @@ -463,12 +469,13 @@ def get_cli_opts_node(opts=None, srcdir=None): Example: >>> from types import SimpleNamespace + >>> from pathlib import Path >>> opts = SimpleNamespace( ... opt_conf_keys='A B', ... defines=["[env]FOO=BAR"], ... rose_template_vars=["QUX=BAZ"] ... ) - >>> node = get_cli_opts_node(opts) + >>> node = get_cli_opts_node(Path('no/such/dir'), opts) >>> node['opts'] {'value': 'A B', 'state': '!', 'comments': []} >>> node['env']['FOO'] @@ -477,9 +484,9 @@ def get_cli_opts_node(opts=None, srcdir=None): {'value': 'BAZ', 'state': '', 'comments': []} """ # Unpack info we want from opts: - opt_conf_keys = [] - defines = [] - rose_template_vars = [] + opt_conf_keys: list = [] + defines: list = [] + rose_template_vars: list = [] if opts and 'opt_conf_keys' in dir(opts): opt_conf_keys = opts.opt_conf_keys or [] if opts and 'defines' in dir(opts): @@ -502,21 +509,26 @@ def get_cli_opts_node(opts=None, srcdir=None): newconfig.set(*parsed_define) # For each __suite define__ add define. - if srcdir is not None: - config_node = rose_config_tree_loader(srcdir, opts).node - templating = identify_templating_section(config_node) - else: + templating: str + if not rose_config_exists(srcdir): templating = 'template variables' + else: + templating = identify_templating_section( + rose_config_tree_loader(srcdir, opts).node + ) for define in rose_template_vars: - match = re.match( + _match = re.match( r'(?P!{0,2})(?P.*)\s*=\s*(?P.*)', define - ).groupdict() + ) + if not _match: + raise ValueError(f'Invalid define: {define}') + _match_groups = _match.groupdict() # Guess templating type? newconfig.set( - keys=[templating, match['key']], - value=match['value'], - state=match['state'] + keys=[templating, _match_groups['key']], + value=_match_groups['value'], + state=_match_groups['state'] ) # Specialised treatement of optional configs. @@ -650,7 +662,7 @@ def simplify_opts_strings(opts): return ' '.join(reversed(seen_once)) -def dump_rose_log(rundir, node): +def dump_rose_log(rundir: Path, node: ConfigNode): """Dump a config node to a timestamped file in the ``log`` sub-directory. Args: @@ -807,8 +819,7 @@ def deprecation_warnings(config_tree): def load_rose_config( srcdir: Path, - opts=None, - warn: bool = True, + opts: 'Optional[Values]' = None, ) -> 'ConfigTree': """Load rose configuration from srcdir. @@ -824,20 +835,12 @@ def load_rose_config( opts: Options object containing specification of optional configuarations set by the CLI. - warn: - Log deprecation warnings if True. + + Note: this is None for "rose stem" usage. Returns: - dict - A dictionary of sections of rose-suite.conf. - For each section either a dictionary or None is returned. - E.g. - { - 'env': {'MYVAR': 42}, - 'empy:suite.rc': None, - 'jinja2:suite.rc': { - 'myJinja2Var': {'yes': 'it is a dictionary!'} - } - } + The Rose configuration tree for "srcdir". + """ # Return a blank config dict if srcdir does not exist if not rose_config_exists(srcdir): @@ -858,23 +861,22 @@ def load_rose_config( # Load the raw config tree config_tree = rose_config_tree_loader(srcdir, opts) - if warn: - deprecation_warnings(config_tree) + deprecation_warnings(config_tree) return config_tree -def export_environment(environment): +def export_environment(environment: Dict[str, str]) -> None: # Export environment vars for key, val in environment.items(): os.environ[key] = val def record_cylc_install_options( - rundir=None, - opts=None, - srcdir=None, -): + srcdir: Path, + rundir: Path, + opts: 'Values', +) -> Tuple[ConfigNode, ConfigNode]: """Create/modify files recording Cylc install config options. Creates a new config based on CLI options and writes it to the workflow @@ -887,28 +889,32 @@ def record_cylc_install_options( been written in the installed ``rose-suite.conf``. Args: - srcdir (pathlib.Path): + srcdir: Used to check whether the source directory contains a rose config. + rundir: + Path to dump the rose-suite-cylc-conf opts: Cylc option parser object - we want to extract the following values: - - opt_conf_keys (list or str): + - opt_conf_keys (list of str): Equivalent of ``rose suite-run --option KEY`` - defines (list of str): Equivalent of ``rose suite-run --define KEY=VAL`` - rose_template_vars (list of str): Equivalent of ``rose suite-run --define-suite KEY=VAL`` - rundir (pathlib.Path): - Path to dump the rose-suite-cylc-conf Returns: - cli_config - Config Node which has been dumped to - ``rose-suite-cylc-install.conf``. - rose_suite_conf['opts'] - Opts section of the config node dumped to - installed ``rose-suite.conf``. + Tuple - (cli_config, rose_suite_conf) + + cli_config: + The Cylc install config aka "rose-suite-cylc-install.conf". + rose_suite_conf: + The "opts" section of the config node dumped to + installed ``rose-suite.conf``. + """ # Create a config based on command line options: - cli_config = get_cli_opts_node(opts, srcdir) + cli_config = get_cli_opts_node(srcdir, opts) # raise error if CLI config has multiple templating sections identify_templating_section(cli_config) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 497b6ddd..86c1d464 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -16,6 +16,7 @@ """Tests the plugin with Rose suite configurations on the filesystem.""" from io import StringIO +from pathlib import Path from types import SimpleNamespace from cylc.flow.hostuserutil import get_host @@ -501,7 +502,7 @@ def test_get_cli_opts_node(opt_confs, defines, rose_template_vars, expect): ) loader = ConfigLoader() expect = loader.load(StringIO(expect)) - result = get_cli_opts_node(opts) + result = get_cli_opts_node(Path('no/such/dir'), opts) for item in ['env', 'template variables', 'opts']: assert result[item] == expect[item] diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 9371714a..fd8f6fba 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -60,7 +60,7 @@ def assert_rose_conf_full_equal(left, right, no_ignore=True): def test_no_rose_suite_conf_in_devdir(tmp_path): - result = post_install(srcdir=tmp_path) + result = post_install(tmp_path, tmp_path, SimpleNamespace()) assert result is False @@ -82,7 +82,7 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path): ) # Run both record_cylc_install options and fileinstall. - record_cylc_install_options(opts=opts, rundir=destdir) + record_cylc_install_options(srcdir, destdir, opts) rose_fileinstall(destdir, opts) assert ((destdir / 'installedme').read_text() == 'Galileo No! We will not let you go.' @@ -248,11 +248,7 @@ def fake(*arg, **kwargs): loader = ConfigLoader() # Run the entry point top-level function: - rose_suite_cylc_install_node, rose_suite_opts_node = ( - record_cylc_install_options( - rundir=testdir, opts=opts, srcdir=testdir - ) - ) + record_cylc_install_options(testdir, testdir, opts=opts) rose_fileinstall(testdir, opts) ritems = sorted([i.relative_to(refdir) for i in refdir.rglob('*')]) titems = sorted([i.relative_to(testdir) for i in testdir.rglob('*')]) @@ -321,11 +317,7 @@ def test_template_section_conflict( with pytest.raises(MultipleTemplatingEnginesError) as exc_info: # Run the entry point top-level function: - rose_suite_cylc_install_node, rose_suite_opts_node = ( - record_cylc_install_options( - rundir=testdir, opts=opts, srcdir=testdir - ) - ) + record_cylc_install_options(testdir, testdir, opts) assert exc_info.match(expect) @@ -356,7 +348,7 @@ def test_cylc_no_rose(tmp_path): """A Cylc workflow that contains no ``rose-suite.conf`` installs OK. """ from cylc.rose.entry_points import post_install - assert post_install(srcdir=tmp_path, rundir=tmp_path) is False + assert post_install(tmp_path, tmp_path, SimpleNamespace()) is False def test_copy_config_file_fails(tmp_path): From b993900ed8cc537baeb5df5c3c68349e34c0c7a5 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 1 Nov 2023 12:08:19 +0000 Subject: [PATCH 11/12] tests: amend docstrings * Amend false docstrings. * Rename a couple of modules. --- cylc/rose/platform_utils.py | 5 +++-- cylc/rose/utilities.py | 4 +--- tests/conftest.py | 2 -- tests/functional/test_ROSE_ORIG_HOST.py | 1 + tests/functional/test_pre_configure.py | 4 ++-- tests/functional/test_reinstall.py | 1 + tests/functional/test_reinstall_clean.py | 1 + tests/functional/test_reinstall_fileinstall.py | 4 +++- tests/functional/test_rose_fileinstall.py | 4 ++-- tests/functional/test_rose_stem.py | 1 + tests/functional/{test_copy_config_file.py => test_utils.py} | 5 ++--- tests/functional/test_validate_no_rose_ignore_items.py | 1 + tests/unit/test_config_node.py | 1 + tests/unit/test_config_tree.py | 1 + tests/unit/{test_rose_opts.py => test_fileinstall.py} | 4 ++-- tests/unit/test_functional_post_install.py | 1 + tests/unit/test_platform_utils.py | 4 ++-- tests/unit/test_rose_stem_units.py | 4 ++-- 18 files changed, 27 insertions(+), 21 deletions(-) rename tests/functional/{test_copy_config_file.py => test_utils.py} (93%) rename tests/unit/{test_rose_opts.py => test_fileinstall.py} (97%) diff --git a/cylc/rose/platform_utils.py b/cylc/rose/platform_utils.py index 5c3d6740..91dd11a7 100644 --- a/cylc/rose/platform_utils.py +++ b/cylc/rose/platform_utils.py @@ -15,8 +15,9 @@ # You should have received a copy of the GNU General Public License # along with Rose. If not, see . # ----------------------------------------------------------------------------- -"""Interfaces for Cylc Platforms for use by rose apps. -""" + +"""Interfaces for Cylc Platforms for use by rose apps.""" + from optparse import Values import sqlite3 import subprocess diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 15540b21..e23f6e54 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -14,9 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Cylc support for reading and interpreting ``rose-suite.conf`` workflow -configuration files. -""" +"""Cylc support for reading and interpreting ``rose-suite.conf`` files.""" import itertools import os diff --git a/tests/conftest.py b/tests/conftest.py index 93647bf2..624d7c6a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,8 +13,6 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" from types import SimpleNamespace diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py index df1db67e..73d6fa1a 100644 --- a/tests/functional/test_ROSE_ORIG_HOST.py +++ b/tests/functional/test_ROSE_ORIG_HOST.py @@ -15,6 +15,7 @@ # You should have received a copy of the GNU General Public License # along with Rose. If not, see . # ----------------------------------------------------------------------------- + """ Tests for cylc.rose.stem ======================== diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index 7785735b..00d423b7 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -13,8 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" + +"""Test pre_configure entry point.""" from itertools import product import os diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py index 4190a092..65002f9a 100644 --- a/tests/functional/test_reinstall.py +++ b/tests/functional/test_reinstall.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Functional tests for reinstalling of config files. This test does the following: diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py index d6a332f0..749778c4 100644 --- a/tests/functional/test_reinstall_clean.py +++ b/tests/functional/test_reinstall_clean.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Functional tests for reinstalling of config files. This test does the following: diff --git a/tests/functional/test_reinstall_fileinstall.py b/tests/functional/test_reinstall_fileinstall.py index 70ca41e8..817500c6 100644 --- a/tests/functional/test_reinstall_fileinstall.py +++ b/tests/functional/test_reinstall_fileinstall.py @@ -13,7 +13,9 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Cylc reinstall is able to use the async fileinstall from rose without + +""" +Ensure Cylc reinstall is able to use the async fileinstall from rose without trouble. """ diff --git a/tests/functional/test_rose_fileinstall.py b/tests/functional/test_rose_fileinstall.py index 5998b778..d0bca3b3 100644 --- a/tests/functional/test_rose_fileinstall.py +++ b/tests/functional/test_rose_fileinstall.py @@ -13,8 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" + +"""Functional tests for Rose file installation.""" from pathlib import Path import shutil diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 743455e2..4d203196 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -17,6 +17,7 @@ # You should have received a copy of the GNU General Public License # along with Rose. If not, see . # ----------------------------------------------------------------------------- + """ Tests for cylc.rose.stem ======================== diff --git a/tests/functional/test_copy_config_file.py b/tests/functional/test_utils.py similarity index 93% rename from tests/functional/test_copy_config_file.py rename to tests/functional/test_utils.py index caed1a13..f2b9ef07 100644 --- a/tests/functional/test_copy_config_file.py +++ b/tests/functional/test_utils.py @@ -13,9 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function cylc.rose.entry points -copy_config_file. -""" + +"""Unit tests for utilities.""" from pathlib import Path diff --git a/tests/functional/test_validate_no_rose_ignore_items.py b/tests/functional/test_validate_no_rose_ignore_items.py index 5f79b194..99f48318 100644 --- a/tests/functional/test_validate_no_rose_ignore_items.py +++ b/tests/functional/test_validate_no_rose_ignore_items.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Functional Test: It ignores commented items in rose-suite.conf See https://github.com/cylc/cylc-rose/pull/171 diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py index 8c697dad..8254dc15 100644 --- a/tests/unit/test_config_node.py +++ b/tests/unit/test_config_node.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Tests the plugin with Rose suite configurations via the Python API.""" from types import SimpleNamespace diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 86c1d464..a622aa5d 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Tests the plugin with Rose suite configurations on the filesystem.""" from io import StringIO diff --git a/tests/unit/test_rose_opts.py b/tests/unit/test_fileinstall.py similarity index 97% rename from tests/unit/test_rose_opts.py rename to tests/unit/test_fileinstall.py index a05658dd..a4f70abf 100644 --- a/tests/unit/test_rose_opts.py +++ b/tests/unit/test_fileinstall.py @@ -13,8 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" + +"""Unit tests for Rose file installation.""" from pathlib import Path import shutil diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index fd8f6fba..7ff194ed 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Functional tests for top-level function record_cylc_install_options and rose_fileinstall diff --git a/tests/unit/test_platform_utils.py b/tests/unit/test_platform_utils.py index 04433d03..7ac65594 100644 --- a/tests/unit/test_platform_utils.py +++ b/tests/unit/test_platform_utils.py @@ -15,8 +15,8 @@ # You should have received a copy of the GNU General Public License # along with Rose. If not, see . # ----------------------------------------------------------------------------- -"""Tests for platform utils module: -""" + +"""Tests for platform utils module.""" import os from pathlib import Path diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index 72b7fbbd..5128030f 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -13,8 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" + +"""Unit tests for Rose Stem.""" from types import SimpleNamespace from typing import Any, Tuple From 8169941e06eb994a84904f37bd5863f040f6bdac Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 8 Nov 2023 09:59:27 +0000 Subject: [PATCH 12/12] feedback --- cylc/rose/entry_points.py | 9 --------- cylc/rose/fileinstall.py | 9 +-------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 98b6de37..9f9b5d54 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -34,15 +34,6 @@ def pre_configure(srcdir: Path, opts: 'Values') -> dict: """Run before the Cylc configuration is read.""" - if not srcdir: - # not sure how this could happen - return { - # default return value - 'env': {}, - 'template_variables': {}, - 'templating_detected': None - } - # load the Rose config config_tree = load_rose_config(Path(srcdir), opts=opts) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 986ab05f..d91d32a3 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -31,14 +31,7 @@ def rose_fileinstall( rundir: 'Path', opts: 'Values', ) -> 'Union[ConfigNode, bool]': - """Call Rose Fileinstall. - - Args: - srcdir(pathlib.Path): - Search for a ``rose-suite.conf`` file in this location. - rundir (pathlib.Path) - - """ + """Call Rose Fileinstall.""" if not rose_config_exists(rundir): return False