From 319c4a45b5f8a9fb9bbddf6050bc7033bff89007 Mon Sep 17 00:00:00 2001 From: Paul Madden <136389411+maddenp-noaa@users.noreply.github.com> Date: Wed, 11 Dec 2024 16:42:20 -0700 Subject: [PATCH] Double-tag fix for v2.5.1 (#672) --- recipe/meta.json | 2 +- src/uwtools/config/formats/yaml.py | 12 ++--- src/uwtools/config/jinja2.py | 68 +++++++++++++++---------- src/uwtools/config/support.py | 11 ++++ src/uwtools/resources/info.json | 2 +- src/uwtools/tests/config/test_jinja2.py | 30 +++++++++-- src/uwtools/tests/config/test_tools.py | 62 ++++++++++++++++++++++ 7 files changed, 145 insertions(+), 42 deletions(-) diff --git a/recipe/meta.json b/recipe/meta.json index 8cfa7a58f..4130c5e23 100644 --- a/recipe/meta.json +++ b/recipe/meta.json @@ -34,5 +34,5 @@ "pyyaml =6.0.*" ] }, - "version": "2.5.0" + "version": "2.5.1" } diff --git a/src/uwtools/config/formats/yaml.py b/src/uwtools/config/formats/yaml.py index 99c5b32a6..71bc870f8 100644 --- a/src/uwtools/config/formats/yaml.py +++ b/src/uwtools/config/formats/yaml.py @@ -10,9 +10,9 @@ from uwtools.config.support import ( INCLUDE_TAG, UWYAMLConvert, - UWYAMLRemove, from_od, log_and_error, + uw_yaml_loader, yaml_to_str, ) from uwtools.exceptions import UWConfigError @@ -94,10 +94,9 @@ def _load(self, config_file: Optional[Path]) -> dict: :param config_file: Path to config file to load. """ - loader = self._yaml_loader with readable(config_file) as f: try: - config = yaml.load(f.read(), Loader=loader) + config = yaml.load(f.read(), Loader=self._yaml_loader) if isinstance(config, dict): return config t = type(config).__name__ @@ -157,13 +156,10 @@ def _yaml_include(self, loader: yaml.Loader, node: yaml.SequenceNode) -> dict: @property def _yaml_loader(self) -> type[yaml.SafeLoader]: """ - The loader, with appropriate constructors added. + A loader with all UW constructors added. """ - loader = yaml.SafeLoader + loader = uw_yaml_loader() loader.add_constructor(INCLUDE_TAG, self._yaml_include) - for tag_class in (UWYAMLConvert, UWYAMLRemove): - for tag in getattr(tag_class, "TAGS"): - loader.add_constructor(tag, tag_class) return loader # Public methods diff --git a/src/uwtools/config/jinja2.py b/src/uwtools/config/jinja2.py index 8115e248b..0bce1dea2 100644 --- a/src/uwtools/config/jinja2.py +++ b/src/uwtools/config/jinja2.py @@ -8,10 +8,11 @@ from pathlib import Path from typing import Optional, Union +import yaml from jinja2 import Environment, FileSystemLoader, StrictUndefined, Undefined, meta from jinja2.exceptions import UndefinedError -from uwtools.config.support import UWYAMLConvert, UWYAMLRemove, format_to_config +from uwtools.config.support import UWYAMLConvert, UWYAMLRemove, format_to_config, uw_yaml_loader from uwtools.logging import INDENT, MSGWIDTH, log from uwtools.utils.file import get_file_format, readable, writable @@ -122,30 +123,41 @@ def dereference( :param keys: The dict keys leading to this value. :return: The input value, with Jinja2 syntax rendered. """ - rendered: _ConfigVal = val # fall-back value + rendered: _ConfigVal if isinstance(val, dict): keys = keys or [] - new = {} + rendered = {} for k, v in val.items(): if isinstance(v, UWYAMLRemove): - _deref_debug("Removing value at", " > ".join([*keys, k])) + deref_debug("Removing value at", ".".join([*keys, k])) else: - new[dereference(k, context)] = dereference(v, context, local=val, keys=[*keys, k]) - return new - if isinstance(val, list): - return [dereference(v, context) for v in val] - if isinstance(val, str): - _deref_debug("Rendering", val) + kd, vd = [dereference(x, context, val, [*keys, k]) for x in (k, v)] + rendered[kd] = vd + elif isinstance(val, list): + rendered = [dereference(v, context) for v in val] + elif isinstance(val, str): + deref_debug("Rendering", val) rendered = _deref_render(val, context, local) elif isinstance(val, UWYAMLConvert): - _deref_debug("Rendering", val.value) + deref_debug("Rendering", val.value) val.value = _deref_render(val.value, context, local) rendered = _deref_convert(val) else: - _deref_debug("Accepting", val) + deref_debug("Accepting", val) + rendered = val return rendered +def deref_debug(action: str, val: Optional[_ConfigVal] = "") -> None: + """ + Log a debug-level message related to dereferencing. + + :param action: The dereferencing activity being performed. + :param val: The value being dereferenced. + """ + log.debug("[dereference] %s: %s", action, val) + + def render( values_src: Optional[Union[dict, Path]] = None, values_format: Optional[str] = None, @@ -231,25 +243,15 @@ def _deref_convert(val: UWYAMLConvert) -> _ConfigVal: :return: The value translated to the specified type. """ converted: _ConfigVal = val # fall-back value - _deref_debug("Converting", val.value) + deref_debug("Converting", val.value) try: converted = val.convert() - _deref_debug("Converted", converted) + deref_debug("Converted", converted) except Exception as e: # pylint: disable=broad-exception-caught - _deref_debug("Conversion failed", str(e)) + deref_debug("Conversion failed", str(e)) return converted -def _deref_debug(action: str, val: _ConfigVal) -> None: - """ - Log a debug-level message related to dereferencing. - - :param action: The dereferencing activity being performed. - :param val: The value being dereferenced. - """ - log.debug("[dereference] %s: %s", action, val) - - def _deref_render(val: str, context: dict, local: Optional[dict] = None) -> str: """ Render a Jinja2 variable/expression as part of dereferencing. @@ -266,10 +268,22 @@ def _deref_render(val: str, context: dict, local: Optional[dict] = None) -> str: context = {**(local or {}), **context} try: rendered = _register_filters(env).from_string(val).render(context) - _deref_debug("Rendered", rendered) + deref_debug("Rendered", rendered) + except Exception as e: # pylint: disable=broad-exception-caught + rendered = val + deref_debug("Rendering failed", val) + for line in str(e).split("\n"): + deref_debug(line) + try: + loaded = yaml.load(rendered, Loader=uw_yaml_loader()) except Exception as e: # pylint: disable=broad-exception-caught + loaded = None + deref_debug("Loading rendered value as YAML", rendered) + for line in str(e).split("\n"): + deref_debug(line) + if isinstance(loaded, UWYAMLConvert): rendered = val - _deref_debug("Rendering failed", str(e)) + deref_debug("Held", rendered) return rendered diff --git a/src/uwtools/config/support.py b/src/uwtools/config/support.py index f6337853c..393ed1a44 100644 --- a/src/uwtools/config/support.py +++ b/src/uwtools/config/support.py @@ -68,6 +68,17 @@ def log_and_error(msg: str) -> Exception: return UWConfigError(msg) +def uw_yaml_loader() -> type[yaml.SafeLoader]: + """ + A loader with basic UW constructors added. + """ + loader = yaml.SafeLoader + for tag_class in (UWYAMLConvert, UWYAMLRemove): + for tag in getattr(tag_class, "TAGS"): + loader.add_constructor(tag, tag_class) + return loader + + def yaml_to_str(cfg: dict) -> str: """ Return a uwtools-conventional YAML representation of the given dict. diff --git a/src/uwtools/resources/info.json b/src/uwtools/resources/info.json index 147828162..8492c9639 100644 --- a/src/uwtools/resources/info.json +++ b/src/uwtools/resources/info.json @@ -1,4 +1,4 @@ { - "version": "2.5.0", + "version": "2.5.1", "buildnum": "0" } diff --git a/src/uwtools/tests/config/test_jinja2.py b/src/uwtools/tests/config/test_jinja2.py index 407e49f9c..10c3c0f7c 100644 --- a/src/uwtools/tests/config/test_jinja2.py +++ b/src/uwtools/tests/config/test_jinja2.py @@ -17,7 +17,7 @@ from uwtools.config import jinja2 from uwtools.config.jinja2 import J2Template -from uwtools.config.support import UWYAMLConvert, UWYAMLRemove +from uwtools.config.support import UWYAMLConvert, UWYAMLRemove, uw_yaml_loader from uwtools.logging import log from uwtools.tests.support import logged, regex_logged @@ -141,7 +141,7 @@ def test_dereference_remove(caplog): remove = UWYAMLRemove(yaml.SafeLoader(""), yaml.ScalarNode(tag="!remove", value="")) val = {"a": {"b": {"c": "cherry", "d": remove}}} assert jinja2.dereference(val=val, context={}) == {"a": {"b": {"c": "cherry"}}} - assert regex_logged(caplog, "Removing value at: a > b > d") + assert regex_logged(caplog, "Removing value at: a.b.d") def test_dereference_str_expression_rendered(): @@ -167,6 +167,12 @@ def test_dereference_str_variable_rendered_str(): assert jinja2.dereference(val=val, context={"greeting": "hello"}) == "hello" +def test_deref_debug(caplog): + log.setLevel(logging.DEBUG) + jinja2.deref_debug(action="Frobnicated", val="foo") + assert logged(caplog, "[dereference] Frobnicated: foo") + + def test_register_filters_env(): s = "hello {{ 'RECIPIENT' | env }}" template = jinja2._register_filters(Environment(undefined=DebugUndefined)).from_string(s) @@ -309,13 +315,16 @@ def test__deref_convert_ok(caplog, converted, tag, value): assert not regex_logged(caplog, "Conversion failed") -def test__deref_debug(caplog): +def test__deref_render_held(caplog): log.setLevel(logging.DEBUG) - jinja2._deref_debug(action="Frobnicated", val="foo") - assert logged(caplog, "[dereference] Frobnicated: foo") + val, context = "!int '{{ a }}'", yaml.load("a: !int '{{ 42 }}'", Loader=uw_yaml_loader()) + assert jinja2._deref_render(val=val, context=context) == val + assert regex_logged(caplog, "Rendered") + assert regex_logged(caplog, "Held") def test__deref_render_no(caplog, deref_render_assets): + log.setLevel(logging.DEBUG) val, context, _ = deref_render_assets assert jinja2._deref_render(val=val, context=context) == val assert not regex_logged(caplog, "Rendered") @@ -323,19 +332,30 @@ def test__deref_render_no(caplog, deref_render_assets): def test__deref_render_ok(caplog, deref_render_assets): + log.setLevel(logging.DEBUG) val, context, local = deref_render_assets assert jinja2._deref_render(val=val, context=context, local=local) == "hello world" assert regex_logged(caplog, "Rendered") assert not regex_logged(caplog, "Rendering failed") +def test__deref_render_unloadable_val(caplog): + log.setLevel(logging.DEBUG) + val = "&XMLENTITY;" + assert jinja2._deref_render(val='{{ "%s" if True }}' % val, context={}) == val + assert regex_logged(caplog, "Rendered") + assert not regex_logged(caplog, "Rendering failed") + + def test__dry_run_template(caplog): + log.setLevel(logging.DEBUG) jinja2._dry_run_template("roses are red\nviolets are blue") assert logged(caplog, "roses are red") assert logged(caplog, "violets are blue") def test__log_missing_values(caplog): + log.setLevel(logging.DEBUG) missing = ["roses_color", "violets_color"] jinja2._log_missing_values(missing) assert logged(caplog, "Value(s) required to render template not provided:") diff --git a/src/uwtools/tests/config/test_tools.py b/src/uwtools/tests/config/test_tools.py index 69e5c77e7..34e426caf 100644 --- a/src/uwtools/tests/config/test_tools.py +++ b/src/uwtools/tests/config/test_tools.py @@ -59,6 +59,16 @@ def realize_config_yaml_input(tmp_path): # Helpers +def help_realize_config_double_tag(config, expected, tmp_path): + path_in = tmp_path / "in.yaml" + path_out = tmp_path / "out.yaml" + with open(path_in, "w", encoding="utf-8") as f: + print(dedent(config).strip(), file=f) + tools.realize_config(input_config=path_in, output_file=path_out) + with open(path_out, "r", encoding="utf-8") as f: + assert f.read().strip() == dedent(expected).strip() + + def help_realize_config_fmt2fmt(input_file, input_format, update_file, update_format, tmpdir): input_file = fixture_path(input_file) update_file = fixture_path(update_file) @@ -235,6 +245,58 @@ def test_realize_config_depth_mismatch_to_sh(realize_config_yaml_input): ) +def test_realize_config_double_tag_flat(tmp_path): + config = """ + a: 1 + b: 2 + foo: !int "{{ a + b }}" + bar: !int "{{ foo }}" + """ + expected = """ + a: 1 + b: 2 + foo: 3 + bar: 3 + """ + help_realize_config_double_tag(config, expected, tmp_path) + + +def test_realize_config_double_tag_nest(tmp_path): + config = """ + a: 1.0 + b: 2.0 + qux: + foo: !float "{{ a + b }}" + bar: !float "{{ foo }}" + """ + expected = """ + a: 1.0 + b: 2.0 + qux: + foo: 3.0 + bar: 3.0 + """ + help_realize_config_double_tag(config, expected, tmp_path) + + +def test_realize_config_double_tag_nest_forwrad_reference(tmp_path): + config = """ + a: true + b: false + bar: !bool "{{ qux.foo }}" + qux: + foo: !bool "{{ a or b }}" + """ + expected = """ + a: true + b: false + bar: true + qux: + foo: true + """ + help_realize_config_double_tag(config, expected, tmp_path) + + def test_realize_config_dry_run(caplog): """ Test that providing a YAML base file with a dry-run flag will print an YAML config file.