From 430d1123b9d0b865ca1b674e22cfe2796735e87f Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 18 Oct 2021 16:01:32 +0100 Subject: [PATCH] Separate Parsec ItemNotFoundError into two ItemNotFoundError - Where an item could be defined, but isn't (in sparse mode). NotAConfigItemError - Where an item is not part of a config to be defined. move get_manyparents to init create unit test for get_manyparents --- CHANGES.md | 2 + cylc/flow/parsec/config.py | 45 ++++++++++++++--- cylc/flow/parsec/exceptions.py | 12 ++++- .../cylc-get-config/08-item-not-found.t | 49 +++++++++++++++++++ tests/unit/parsec/test_config.py | 21 +++++++- 5 files changed, 120 insertions(+), 9 deletions(-) create mode 100755 tests/functional/cylc-get-config/08-item-not-found.t diff --git a/CHANGES.md b/CHANGES.md index 1e6ceb275dc..bd1639caf31 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -104,6 +104,8 @@ now accepts ``&`` and ``|`` as valid line breaks in the same way as ``=>``. renamed to `CYLC_WORKFLOW_ID`. `CYLC_WORKFLOW_NAME` re-added as `CYLC_WORKFLOW_ID` shorn of any trailing `runX`. +[#4471](https://github.com/cylc/cylc-flow/pull/4471) - Users now get a different +error for a config item that doesn't exist to be set, to one that isn't set. ### Fixes diff --git a/cylc/flow/parsec/config.py b/cylc/flow/parsec/config.py index a31212516b8..1b7383bc760 100644 --- a/cylc/flow/parsec/config.py +++ b/cylc/flow/parsec/config.py @@ -17,11 +17,13 @@ from copy import deepcopy import re from textwrap import dedent +from typing import Union from cylc.flow.context_node import ContextNode from cylc.flow.parsec.exceptions import ( ItemNotFoundError, - NotSingleItemError + NotSingleItemError, + NotAConfigItemError, ) from cylc.flow.parsec.fileparse import parse from cylc.flow.parsec.util import printcfg @@ -44,6 +46,8 @@ def __init__(self, spec, upgrader=None, output_fname=None, tvars=None, if validator is None: validator = parsec_validate self.validator = validator + # Get a list of config items which have a private name ``__MANY__``: + self.manyparents = self._get_namespace_parents() def loadcfg(self, rcfile, title=""): """Parse a config file, upgrade or deprecate items if necessary, @@ -110,12 +114,18 @@ def get(self, keys=None, sparse=False): parents = [] if keys: for key in keys: - try: - cfg = cfg[key] - except (KeyError, TypeError): - raise ItemNotFoundError(itemstr(parents, key)) + if ( + key not in self.dense + and not parents or parents in self.manyparents + ): + raise NotAConfigItemError(key) else: - parents.append(key) + try: + cfg = cfg[key] + except (KeyError, TypeError): + raise ItemNotFoundError(itemstr(parents, key)) + else: + parents.append(key) return cfg @@ -162,6 +172,29 @@ def dump(self, keys=None, sparse=False, prefix='', none_str=''): cfg = self.get(keys, sparse) printcfg(cfg, prefix=prefix, level=len(keys), none_str=none_str) + def _get_namespace_parents(self) -> Union[list, None]: + """get a list of the parents of config items which can be user defined. + + For example, where + + .. code-block:: cylc + + [runtime] + [[my_task]] # Custom task names. + [[my_other_task]] + + this function will return ``[runtime]``. + """ + try: + manyparents = [ + list(key[1].parents())[0].name + for key in self.spec.walk() + if key[1].name == '__MANY__' + ] + except AttributeError: + manyparents = None + return manyparents + class ConfigNode(ContextNode): """A Cylc configuration schema, section, or setting. diff --git a/cylc/flow/parsec/exceptions.py b/cylc/flow/parsec/exceptions.py index 496ca0ff1e4..f1ff3bbc411 100644 --- a/cylc/flow/parsec/exceptions.py +++ b/cylc/flow/parsec/exceptions.py @@ -35,7 +35,17 @@ def __init__(self, item): self.item = item def __str__(self): - return f'item not found: {self.item}' + return f'You have not set \"{self.item}\" in this config.' + + +class NotAConfigItemError(ParsecError, KeyError): + """Error raised for missing configuration items.""" + + def __init__(self, item): + self.item = item + + def __str__(self): + return f'You cannot set \"{self.item}\" in this config.' class NotSingleItemError(ParsecError, TypeError): diff --git a/tests/functional/cylc-get-config/08-item-not-found.t b/tests/functional/cylc-get-config/08-item-not-found.t new file mode 100755 index 00000000000..73605041458 --- /dev/null +++ b/tests/functional/cylc-get-config/08-item-not-found.t @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# THIS FILE IS PART OF 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 cylc config +. "$(dirname "$0")/test_header" +#------------------------------------------------------------------------------- +set_test_number 7 +#------------------------------------------------------------------------------- +cat >>'global.cylc' <<__HERE__ +[platforms] + [[foo]] +__HERE__ + +OLD="$CYLC_CONF_PATH" +export CYLC_CONF_PATH="${PWD}" + +# Control Run +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 "NotAConfigItemError" "${TEST_NAME_BASE}-not-in-config-spec.stderr" + +# If item not defined, item not found. +run_fail "${TEST_NAME_BASE}-not-defined" cylc config -i "[scheduler]" +grep_ok "ItemNotFoundError" "${TEST_NAME_BASE}-not-defined.stderr" + +# 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" + +rm global.cylc +export CYLC_CONF_PATH="$OLD" + +exit diff --git a/tests/unit/parsec/test_config.py b/tests/unit/parsec/test_config.py index 5012ca5f8bf..cb37344f431 100644 --- a/tests/unit/parsec/test_config.py +++ b/tests/unit/parsec/test_config.py @@ -14,12 +14,17 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +from os import spawnl import tempfile +from _pytest.config import Config import pytest from cylc.flow.parsec import config -from cylc.flow.parsec.config import ConfigNode as Conf +from cylc.flow.parsec.config import ( + ConfigNode as Conf, + ParsecConfig +) from cylc.flow.parsec.OrderedDict import OrderedDictWithDefaults from cylc.flow.parsec.upgrade import upgrader from cylc.flow.parsec.validate import ( @@ -256,7 +261,7 @@ def test_get_item(sample_spec_2): def test_item_not_found_error(): error = config.ItemNotFoundError("internal error") - assert 'item not found: internal error' == str(error) + assert 'You have not set "internal error" in this config.' == str(error) def test_not_single_item_error(): @@ -314,3 +319,15 @@ def test_mdump_oneline(parse_config, sample_spec, capsys): def test_get_none(parse_config): cfg = parse_config(sample_spec, '') # blank config assert cfg.get(sparse=True) == {} + + +def test__get_namespace_parents(parse_config): + """It returns a list of parents and nothing else""" + def spec_(): + with Conf('myconfig') as myconf: + with Conf('manythings'): + Conf('') + + return myconf + cfg = ParsecConfig(spec_()) + assert cfg.manyparents == ['manythings']