Fix traceback in cylc config -i (cylc#6062)

Fix traceback in `cylc config -i`
Fixes bug when trying to get a setting in a `__MANY__` section when the setting is valid but not set in the config.
Better handle `ItemNotFoundError` when printing platforms config

Author: MetRonnie

cylc/flow/cfgspec/globalcfg.py

@@ -1994,26 +1994,29 @@ def platform_dump(
         """Print informations about platforms currently defined.
         """
         if print_platform_names:
-            with suppress(ItemNotFoundError):
-                self.dump_platform_names(self)
+            self.dump_platform_names(self)
         if print_platforms:
-            with suppress(ItemNotFoundError):
-                self.dump_platform_details(self)
+            self.dump_platform_details(self)
 
     @staticmethod
     def dump_platform_names(cfg) -> None:
         """Print a list of defined platforms and groups.
         """
+        # [platforms] is always defined with at least localhost
         platforms = '\n'.join(cfg.get(['platforms']).keys())
-        platform_groups = '\n'.join(cfg.get(['platform groups']).keys())
         print(f'{PLATFORM_REGEX_TEXT}\n\nPlatforms\n---------', file=stderr)
         print(platforms)
-        print('\n\nPlatform Groups\n--------------', file=stderr)
+        try:
+            platform_groups = '\n'.join(cfg.get(['platform groups']).keys())
+        except ItemNotFoundError:
+            return
+        print('\nPlatform Groups\n--------------', file=stderr)
         print(platform_groups)
 
     @staticmethod
     def dump_platform_details(cfg) -> None:
         """Print platform and platform group configs.
         """
         for config in ['platforms', 'platform groups']:
-            printcfg({config: cfg.get([config], sparse=True)})
+            with suppress(ItemNotFoundError):
+                printcfg({config: cfg.get([config], sparse=True)})

cylc/flow/config.py

@@ -73,7 +73,7 @@
 from cylc.flow.param_expand import NameExpander
 from cylc.flow.parsec.exceptions import ItemNotFoundError
 from cylc.flow.parsec.OrderedDict import OrderedDictWithDefaults
-from cylc.flow.parsec.util import replicate
+from cylc.flow.parsec.util import dequote, replicate
 from cylc.flow.pathutil import (
     get_workflow_name_from_id,
     get_cylc_run_dir,
@@ -198,29 +198,6 @@ def interpolate_template(tmpl, params_dict):
         raise ParamExpandError('bad template syntax')
 
 
-def dequote(string):
-    """Strip quotes off a string.
-
-    Examples:
-        >>> dequote('"foo"')
-        'foo'
-        >>> dequote("'foo'")
-        'foo'
-        >>> dequote('foo')
-        'foo'
-        >>> dequote('"f')
-        '"f'
-        >>> dequote('f')
-        'f'
-
-    """
-    if len(string) < 2:
-        return string
-    if (string[0] == string[-1]) and string.startswith(("'", '"')):
-        return string[1:-1]
-    return string
-
-
 class WorkflowConfig:
     """Class for workflow configuration items and derived quantities."""
 

cylc/flow/context_node.py

@@ -15,7 +15,13 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-from typing import Optional
+from typing import TYPE_CHECKING, Optional
+
+if TYPE_CHECKING:
+    # BACK COMPAT: typing_extensions.Self
+    # FROM: Python 3.7
+    # TO: Python 3.11
+    from typing_extensions import Self
 
 
 class ContextNode():
@@ -93,7 +99,7 @@ def __init__(self, name: str):
         self._children = None
         self.DATA[self] = set(self.DATA)
 
-    def __enter__(self):
+    def __enter__(self) -> 'Self':
         return self
 
     def __exit__(self, *args):
@@ -129,24 +135,36 @@ def __iter__(self):
     def __contains__(self, name: str) -> bool:
         return name in self._children  # type: ignore[operator]  # TODO
 
-    def __getitem__(self, name: str):
+    def __getitem__(self, name: str) -> 'Self':
         if self._children:
             return self._children.__getitem__(name)
         raise TypeError('This is not a leaf node')
 
-    def get(self, *names: str):
+    def get(self, *names: str) -> 'Self':
         """Retrieve the node given by the list of names.
 
         Example:
             >>> with ContextNode('a') as a:
-            ...     with ContextNode('b') as b:
+            ...     with ContextNode('b'):
             ...          c = ContextNode('c')
             >>> a.get('b', 'c')
             a/b/c
+
+            >>> with ContextNode('a') as a:
+            ...     with ContextNode('b'):
+            ...         with ContextNode('__MANY__'):
+            ...             c = ContextNode('c')
+            >>> a.get('b', 'foo', 'c')
+            a/b/__MANY__/c
         """
         node = self
         for name in names:
-            node = node[name]
+            try:
+                node = node[name]
+            except KeyError as exc:
+                if '__MANY__' not in node:
+                    raise exc
+                node = node['__MANY__']
         return node
 
     def __str__(self) -> str:

cylc/flow/parsec/config.py

@@ -145,7 +145,9 @@ def get(self, keys: Optional[Iterable[str]] = None, sparse: bool = False):
                     cfg = cfg[key]
                 except (KeyError, TypeError):
                     if (
+                        # __MANY__ setting not present:
                         parents in self.manyparents or
+                        # setting not present in __MANY__ section:
                         key in self.spec.get(*parents)
                     ):
                         raise ItemNotFoundError(itemstr(parents, key))

cylc/flow/parsec/util.py

@@ -406,16 +406,7 @@ def expand_many_section(config):
     """
     ret = {}
     for section_name, section in config.items():
-        expanded_names = [
-            dequote(name.strip()).strip()
-            for name in SECTION_EXPAND_PATTERN.findall(section_name)
-        ]
-        for name in expanded_names:
-            if name in ret:
-                # already defined -> merge
-                replicate(ret[name], section)
-
-            else:
-                ret[name] = {}
-                replicate(ret[name], section)
+        for name in SECTION_EXPAND_PATTERN.findall(section_name):
+            name = dequote(name.strip()).strip()
+            replicate(ret.setdefault(name, {}), section)
     return ret

tests/functional/cylc-config/08-item-not-found.t

@@ -18,7 +18,7 @@
 # Test cylc config
 . "$(dirname "$0")/test_header"
 #-------------------------------------------------------------------------------
-set_test_number 7
+set_test_number 9
 #-------------------------------------------------------------------------------
 cat >>'global.cylc' <<__HERE__
 [platforms]
@@ -29,21 +29,29 @@ OLD="$CYLC_CONF_PATH"
 export CYLC_CONF_PATH="${PWD}"
 
 # Control Run
-run_ok "${TEST_NAME_BASE}-ok" cylc config -i "[platforms]foo"
+run_ok "${TEST_NAME_BASE}-ok" cylc config -i "[platforms][foo]"
 
 # If item not settable in config (platforms is mis-spelled):
-run_fail "${TEST_NAME_BASE}-not-in-config-spec" cylc config -i "[platfroms]foo"
-grep_ok "InvalidConfigError" "${TEST_NAME_BASE}-not-in-config-spec.stderr"
+run_fail "${TEST_NAME_BASE}-not-in-config-spec" cylc config -i "[platfroms][foo]"
+cmp_ok "${TEST_NAME_BASE}-not-in-config-spec.stderr" << __HERE__
+InvalidConfigError: "platfroms" is not a valid configuration for global.cylc.
+__HERE__
 
-# If item not defined, item not found.
+# If item settable in config but not set.
 run_fail "${TEST_NAME_BASE}-not-defined" cylc config -i "[scheduler]"
-grep_ok "ItemNotFoundError" "${TEST_NAME_BASE}-not-defined.stderr"
+cmp_ok "${TEST_NAME_BASE}-not-defined.stderr" << __HERE__
+ItemNotFoundError: You have not set "scheduler" in this config.
+__HERE__
+
+run_fail "${TEST_NAME_BASE}-not-defined-2" cylc config -i "[platforms][bar]"
+cmp_ok "${TEST_NAME_BASE}-not-defined-2.stderr" << __HERE__
+ItemNotFoundError: You have not set "[platforms]bar" in this config.
+__HERE__
 
-# If item settable in config, item not found.
-run_fail "${TEST_NAME_BASE}-not-defined__MULTI__" cylc config -i "[platforms]bar"
-grep_ok "ItemNotFoundError" "${TEST_NAME_BASE}-not-defined__MULTI__.stderr"
+run_fail "${TEST_NAME_BASE}-not-defined-3" cylc config -i "[platforms][foo]hosts"
+cmp_ok "${TEST_NAME_BASE}-not-defined-3.stderr" << __HERE__
+ItemNotFoundError: You have not set "[platforms][foo]hosts" in this config.
+__HERE__
 
 rm global.cylc
 export CYLC_CONF_PATH="$OLD"
-
-exit

tests/functional/cylc-config/09-platforms.t

@@ -54,7 +54,6 @@ They are searched from the bottom up, until the first match is found.
 Platforms
 ---------
 
-
 Platform Groups
 --------------
 __HEREDOC__

tests/unit/parsec/test_config.py

@@ -156,7 +156,7 @@ def test_validate():
     :return:
     """
 
-    with Conf('myconf') as spec:  # noqa: SIM117
+    with Conf('myconf') as spec:
         with Conf('section'):
             Conf('name', VDR.V_STRING)
             Conf('address', VDR.V_STRING)
@@ -192,13 +192,20 @@ def parsec_config_2(tmp_path: Path):
             Conf('address', VDR.V_INTEGER_LIST)
         with Conf('allow_many'):
             Conf('<user defined>', VDR.V_STRING, '')
+        with Conf('so_many'):
+            with Conf('<thing>'):
+                Conf('color', VDR.V_STRING)
+                Conf('horsepower', VDR.V_INTEGER)
     parsec_config = ParsecConfig(spec, validator=cylc_config_validate)
     conf_file = tmp_path / 'myconf'
     conf_file.write_text("""
     [section]
     name = test
     [allow_many]
     anything = yup
+    [so_many]
+    [[legs]]
+    horsepower = 123
     """)
     parsec_config.loadcfg(conf_file, "1.0")
     return parsec_config
@@ -213,25 +220,32 @@ def test_expand(parsec_config_2: ParsecConfig):
 
 def test_get(parsec_config_2: ParsecConfig):
     cfg = parsec_config_2.get(keys=None, sparse=False)
-    assert parsec_config_2.dense == cfg
+    assert cfg == parsec_config_2.dense
 
     cfg = parsec_config_2.get(keys=None, sparse=True)
-    assert parsec_config_2.sparse == cfg
+    assert cfg == parsec_config_2.sparse
 
     cfg = parsec_config_2.get(keys=['section'], sparse=True)
-    assert parsec_config_2.sparse['section'] == cfg
-
-    with pytest.raises(InvalidConfigError):
-        parsec_config_2.get(keys=['alloy_many', 'a'], sparse=True)
-
-    cfg = parsec_config_2.get(keys=['section', 'name'], sparse=True)
-    assert cfg == 'test'
-
-    with pytest.raises(InvalidConfigError):
-        parsec_config_2.get(keys=['section', 'a'], sparse=True)
-
-    with pytest.raises(ItemNotFoundError):
-        parsec_config_2.get(keys=['allow_many', 'a'], sparse=True)
+    assert cfg == parsec_config_2.sparse['section']
+
+
+@pytest.mark.parametrize('keys, expected', [
+    (['section', 'name'], 'test'),
+    (['section', 'a'], InvalidConfigError),
+    (['alloy_many', 'anything'], InvalidConfigError),
+    (['allow_many', 'anything'], 'yup'),
+    (['allow_many', 'a'], ItemNotFoundError),
+    (['so_many', 'legs', 'horsepower'], 123),
+    (['so_many', 'legs', 'color'], ItemNotFoundError),
+    (['so_many', 'legs', 'a'], InvalidConfigError),
+    (['so_many', 'teeth', 'horsepower'], ItemNotFoundError),
+])
+def test_get__sparse(parsec_config_2: ParsecConfig, keys, expected):
+    if isinstance(expected, type) and issubclass(expected, Exception):
+        with pytest.raises(expected):
+            parsec_config_2.get(keys, sparse=True)
+    else:
+        assert parsec_config_2.get(keys, sparse=True) == expected
 
 
 def test_mdump_none(config, sample_spec, capsys):
@@ -288,12 +302,17 @@ def test_get_none(config, sample_spec):
 
 def test__get_namespace_parents():
     """It returns a list of parents and nothing else"""
-    with Conf('myconfig') as myconf:
-        with Conf('some_parent'):  # noqa: SIM117
-            with Conf('manythings'):
-                Conf('<thing>')
-        with Conf('other_parent'):
-            Conf('other_thing')
+    with Conf('myconfig.cylc') as myconf:
+        with Conf('a'):
+            with Conf('b'):
+                with Conf('<c>'):
+                    with Conf('d'):
+                        Conf('<e>')
+        with Conf('x'):
+            Conf('y')
 
     cfg = ParsecConfig(myconf)
-    assert cfg.manyparents == [['some_parent', 'manythings']]
+    assert cfg.manyparents == [
+        ['a', 'b'],
+        ['a', 'b', '__MANY__', 'd'],
+    ]

tests/unit/parsec/test_types.py

@@ -48,7 +48,7 @@ def _inner(typ, validator):
             cylc.flow.parsec.config.ConfigNode
 
         """
-        with Conf('/') as myconf:  # noqa: SIM117
+        with Conf('/') as myconf:
             with Conf(typ):
                 Conf('<item>', validator)
         return myconf

tox.ini

@@ -15,6 +15,8 @@ ignore=
 per-file-ignores=
     ; TYPE_CHECKING block suggestions
     tests/*: TC001
+    ; for clarity we don't merge 'with Conf():' context trees
+    tests/unit/parsec/*: SIM117
 
 exclude=
     build,