Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permitted warnings about root-dir for all versions of Cylc. #231

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ 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 (<span actions:bind='release-date'>Upcoming</span>)__

### 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 (<span actions:bind='release-date'>Released 2023-01-16</span>)__

### Fixes
Expand Down
68 changes: 45 additions & 23 deletions cylc/rose/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -42,6 +42,8 @@
ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING = (
' ROSE_ORIG_HOST set by cylc install.'
)
MESSAGE = 'message'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, if you find yourself coding dictionary keys as constants, then named tuples are likely a better fit.

ALL_MODES = 'all modes'


class MultipleTemplatingEnginesError(Exception):
Expand Down Expand Up @@ -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])
6 changes: 1 addition & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,7 @@ 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 = Options(parser, args)()

options = Options(validate_gop(), args)()
MetRonnie marked this conversation as resolved.
Show resolved Hide resolved
output = SimpleNamespace()

try:
Expand Down Expand Up @@ -130,7 +128,6 @@ def _inner(srcpath, args=None):
args: Dictionary of arguments.
"""
options = Options(install_gop(), args)()

output = SimpleNamespace()

try:
Expand All @@ -156,7 +153,6 @@ def _inner(workflow_id, opts=None):
args: Dictionary of arguments.
"""
options = Options(reinstall_gop(), opts)()

output = SimpleNamespace()

try:
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/test_pre_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
43 changes: 43 additions & 0 deletions tests/unit/test_config_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Loading