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

Fix traceback in cylc config -i #6062

Merged
merged 3 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
17 changes: 10 additions & 7 deletions cylc/flow/cfgspec/globalcfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

FYI, this extra newline was probably present for compliance with ReST code style.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this being used in the docs somewhere?

Copy link
Member

@wxtim wxtim Apr 23, 2024

Choose a reason for hiding this comment

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

It'd be pointless using it in the Cylc Docs, but I could imagine feeding it into metomi docs. I don't think that we do that at the moment, though.

This will have mostly been about what looked good to me.

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)})
25 changes: 1 addition & 24 deletions cylc/flow/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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."""

Expand Down
30 changes: 24 additions & 6 deletions cylc/flow/context_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions cylc/flow/parsec/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
15 changes: 3 additions & 12 deletions cylc/flow/parsec/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
30 changes: 19 additions & 11 deletions tests/functional/cylc-config/08-item-not-found.t
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# Test cylc config
. "$(dirname "$0")/test_header"
#-------------------------------------------------------------------------------
set_test_number 7
set_test_number 9
#-------------------------------------------------------------------------------
cat >>'global.cylc' <<__HERE__
[platforms]
Expand All @@ -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
1 change: 0 additions & 1 deletion tests/functional/cylc-config/09-platforms.t
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ They are searched from the bottom up, until the first match is found.
Platforms
---------


Platform Groups
--------------
__HEREDOC__
Expand Down
65 changes: 42 additions & 23 deletions tests/unit/parsec/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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'],
]
2 changes: 1 addition & 1 deletion tests/unit/parsec/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading