From c1859f43e60a6454e1a6e9d47e55a0c466507154 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 13 Jun 2023 16:12:36 +0100 Subject: [PATCH 1/4] Permitted warnings about root-dir for all versions of Cylc. --- CHANGES.md | 7 ++++ cylc/rose/utilities.py | 68 ++++++++++++++++++++++------------ tests/unit/test_config_node.py | 43 +++++++++++++++++++++ 3 files changed, 95 insertions(+), 23 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 3e58b3ee..1a4f990a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,13 @@ creating a new release entry be sure to copy & paste the span tag with the updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> +## __cylc-rose-1.2.1 (Upcoming)__ + +### Fixes + +[#231](https://github.com/cylc/cylc-rose/pull/231) - Show warning about +`root-dir` config setting in compatibility mode. + ## __cylc-rose-1.2.0 (Released 2023-01-16)__ ### Fixes diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 16fa5570..ed90511d 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -25,7 +25,7 @@ from cylc.flow.hostuserutil import get_host from cylc.flow import LOG -import cylc.flow.flags as flags +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 metomi.isodatetime.datetimeoper import DateTimeOperator @@ -42,6 +42,8 @@ ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING = ( ' ROSE_ORIG_HOST set by cylc install.' ) +MESSAGE = 'message' +ALL_MODES = 'all modes' class MultipleTemplatingEnginesError(Exception): @@ -656,30 +658,50 @@ def deprecation_warnings(config_tree): - "root-dir" - "jinja2:suite.rc" - "empy:suite.rc" + - root-dir + If ALL_MODES is True this deprecation will ignore whether there is a + flow.cylc or suite.rc in the workflow directory. """ deprecations = { - 'empy:suite.rc': ( - "'rose-suite.conf[empy:suite.rc]' is deprecated." - " Use [template variables] instead."), - 'jinja2:suite.rc': ( - "'rose-suite.conf[jinja2:suite.rc]' is deprecated." - " Use [template variables] instead."), - 'empy:flow.cylc': ( - "'rose-suite.conf[empy:flow.cylc]' is not used by Cylc." - " Use [template variables] instead."), - 'jinja2:flow.cylc': ( - "'rose-suite.conf[jinja2:flow.cylc]' is not used by Cylc." - " Use [template variables] instead."), - 'root-dir': ( - 'You have set "rose-suite.conf[root-dir]", ' - 'which is not supported at ' - 'Cylc 8. Use `[install] symlink dirs` in global.cylc ' - 'instead.') + 'empy:suite.rc': { + MESSAGE: ( + "'rose-suite.conf[empy:suite.rc]' is deprecated." + " Use [template variables] instead."), + ALL_MODES: False, + }, + 'jinja2:suite.rc': { + MESSAGE: ( + "'rose-suite.conf[jinja2:suite.rc]' is deprecated." + " Use [template variables] instead."), + ALL_MODES: False, + }, + 'empy:flow.cylc': { + MESSAGE: ( + "'rose-suite.conf[empy:flow.cylc]' is not used by Cylc." + " Use [template variables] instead."), + ALL_MODES: False, + }, + 'jinja2:flow.cylc': { + MESSAGE: ( + "'rose-suite.conf[jinja2:flow.cylc]' is not used by Cylc." + " Use [template variables] instead."), + ALL_MODES: False, + }, + 'root-dir': { + MESSAGE: ( + 'You have set "rose-suite.conf[root-dir]", ' + 'which is not supported at ' + 'Cylc 8. Use `[install] symlink dirs` in global.cylc ' + 'instead.'), + ALL_MODES: True, + }, } - if not flags.cylc7_back_compat: - for string in list(config_tree.node): - for deprecation in deprecations.keys(): - if deprecation in string.lower(): - LOG.warning(deprecations[deprecation]) + for string in list(config_tree.node): + for name, info in deprecations.items(): + if ( + (info[ALL_MODES] or not cylc7_back_compat) + and name in string.lower() + ): + LOG.warning(info[MESSAGE]) diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py index 8c7505b2..86bf8232 100644 --- a/tests/unit/test_config_node.py +++ b/tests/unit/test_config_node.py @@ -16,6 +16,7 @@ """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 @@ -28,6 +29,7 @@ ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, get_rose_vars_from_config_node, add_cylc_install_to_rose_conf_node_opts, + deprecation_warnings, dump_rose_log, identify_templating_section, MultipleTemplatingEnginesError @@ -285,3 +287,44 @@ def test_ROSE_ORIG_HOST_replacement_behaviour( ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING) assert not caplog.records assert node['env']['ROSE_ORIG_HOST'].value == 'IMPLAUSIBLE_HOST_NAME' + + +@pytest.mark.parametrize( + 'compat_mode, must_include, must_exclude', + ( + (True, None, 'Use [template variables]'), + (True, 'root-dir', None), + (False, 'Use [template variables]', None), + (False, 'root-dir', None), + ) +) +def test_deprecation_warnings( + caplog, monkeypatch, compat_mode, must_include, must_exclude +): + """Method logs warnings correctly. + + Two node items are set: + + * ``jinja2:suite.rc`` should not cause a warning in compatibility mode. + * ``root-dir=/somewhere`` should always lead to a warning being logged. + + Error messages about + """ + # Create a node to pass to the method + # (It's not a tree test because we can use a simpleNamespace in place of + # a tree object): + node = ConfigNode() + node.set(['jinja2:suite.rc']) + node.set(['root-dir', '~foo']) + tree = SimpleNamespace(node=node) + + # Patch compatibility mode flag and run the function under test: + monkeypatch.setattr('cylc.rose.utilities.cylc7_back_compat', compat_mode) + deprecation_warnings(tree) + + # Check that warnings have/not been logged: + records = '\n'.join([i.message for i in caplog.records]) + if must_include: + assert must_include in records + else: + assert must_exclude not in records From bdc417fc37f34da5c9c64598843cc77c24265dec Mon Sep 17 00:00:00 2001 From: WXTIM <26465611+wxtim@users.noreply.github.com> Date: Wed, 14 Jun 2023 09:08:07 +0100 Subject: [PATCH 2/4] fix inadvertently broken test --- tests/functional/test_pre_configure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index 9c715d2d..ddefff41 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -157,7 +157,7 @@ def test_warn_if_old_templating_set( ): """Test using unsupported root-dir config raises error.""" monkeypatch.setattr( - cylc.rose.utilities.flags, '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) From e134d9aa25652f4ce2a8ccfdca2b68447866b8d8 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 14 Jun 2023 13:03:19 +0100 Subject: [PATCH 3/4] Import Cylc option parser Options object to set up options for running Cylc installs. --- tests/conftest.py | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 9a6464f4..01523040 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,6 +20,7 @@ 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, @@ -99,11 +100,8 @@ def pytest_runtest_makereport(item, call): def _cylc_validate_cli(capsys, caplog): """Access the validate CLI""" def _inner(srcpath, args=None): - parser = validate_gop() - options = parser.get_default_values() - options.__dict__.update({ - 'templatevars': [], 'templatevars_file': [] - }) + parser = Options(validate_gop()) + options = parser() if args is not None: options.__dict__.update(args) @@ -134,13 +132,7 @@ def _inner(srcpath, args=None): srcpath: args: Dictionary of arguments. """ - parser = install_gop() - options = parser.get_default_values() - options.__dict__.update({ - 'profile_mode': None, 'templatevars': [], 'templatevars_file': [], - 'output': None - }) - + options = Options(install_gop())() if args is not None: options.__dict__.update(args) @@ -168,13 +160,7 @@ def _inner(workflow_id, opts=None): srcpath: args: Dictionary of arguments. """ - parser = reinstall_gop() - options = parser.get_default_values() - options.__dict__.update({ - 'profile_mode': None, 'templatevars': [], 'templatevars_file': [], - 'output': None - }) - + options = Options(reinstall_gop())() if opts is not None: options.__dict__.update(opts) From a8ada88fdac319046650981768bb3ade0b6c59b7 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Mon, 17 Jul 2023 10:48:26 +0100 Subject: [PATCH 4/4] Fix post-merge --- CHANGES.md | 6 +----- tests/conftest.py | 3 ++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 73bd3ea3..28f92b92 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,7 +6,7 @@ creating a new release entry be sure to copy & paste the span tag with the updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> -## __cylc-rose-1.3.0 (Awaiting Release)__ +## __cylc-rose-1.2.1 (Upcoming)__ ### Fixes @@ -14,10 +14,6 @@ ones in. --> Fix bug which stops rose-stem suites using the new `[template variables]` section in their `rose-suite.conf` files. -## __cylc-rose-1.2.1 (Upcoming)__ - -### Fixes - [#231](https://github.com/cylc/cylc-rose/pull/231) - Show warning about `root-dir` config setting in compatibility mode. diff --git a/tests/conftest.py b/tests/conftest.py index f48d3414..4031397e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -100,7 +100,8 @@ def pytest_runtest_makereport(item, call): def _cylc_validate_cli(capsys, caplog): """Access the validate CLI""" def _inner(srcpath, args=None): - options = Options(validate_gop(), args)() + parser = validate_gop() + options = Options(parser, args)() output = SimpleNamespace() try: