Skip to content

Commit 8558019

Browse files
committed
Config: check all required params recursively
Instead of checking for required in params in sub dictionnaries at load time, check them recursively after everything is loaded. This change notably fixes bug in case required parameter in a sub dictionnary is not defined in a configuration file but defined in another file and in merged options eventually.
1 parent e0af0a5 commit 8558019

File tree

2 files changed

+61
-15
lines changed

2 files changed

+61
-15
lines changed

lib/rift/Config.py

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -522,8 +522,8 @@ def _dict_value(self, syntax, key, value):
522522
raise DeclError(f"Unknown {key} keys: {', '.join(unknown_keys)}")
523523

524524
# Iterate over the keys defined in syntax. If the subvalue or default
525-
# value is defined, set it or raise error if required.
526-
for subkey, subkey_format in syntax.items():
525+
# value is defined, set it.
526+
for subkey in syntax.keys():
527527
subkey_value = value.get(subkey,
528528
Config._syntax_default(syntax, subkey))
529529
if subkey_value is not None:
@@ -533,12 +533,7 @@ def _dict_value(self, syntax, key, value):
533533
subkey_value,
534534
syntax[subkey].get('check', 'string')
535535
)
536-
elif subkey_format.get('required', False):
537-
raise DeclError(
538-
f"Key {subkey} is required in dict parameter {key}"
539-
)
540536

541-
# Set the key in options dict eventually.
542537
return result
543538

544539
def _record_value(self, syntax, value):
@@ -585,22 +580,45 @@ def update(self, data):
585580
self.set(key, value, arch=arch)
586581

587582
def _check(self):
588-
"""Checks for mandatory options."""
589-
for key, value in self.SYNTAX.items():
583+
"""Checks for required options in main syntax recursively."""
584+
self._check_syntax(self.SYNTAX, self.options)
585+
586+
def _check_syntax(self, syntax, options, param='__main__'):
587+
"""Checks for mandatory options regarding the provided syntax recursively."""
588+
for key in syntax:
590589
if (
591-
value.get('required', False) and
592-
'default' not in value
590+
syntax[key].get('required', False) and
591+
'default' not in syntax[key]
593592
):
594593
# Check key is in options or defined in all supported arch
595594
# specific options.
596595
if (
597-
key not in self.options and
596+
key not in options and
598597
not all(
599-
arch in self.options and key in self.options[arch]
598+
arch in options and key in options[arch]
600599
for arch in self.get('arch')
601600
)
602601
):
603-
raise DeclError(f"'{key}' is not defined")
602+
if param == '__main__':
603+
raise DeclError(f"'{key}' is not defined")
604+
raise DeclError(
605+
f"Key {key} is required in dict parameter {param}"
606+
)
607+
# If the parameter is a dict with a syntax, check the value.
608+
if (
609+
syntax[key].get('check') == 'dict' and
610+
syntax[key].get('syntax') is not None and key in options
611+
):
612+
self._check_syntax(syntax[key]['syntax'], options[key], key)
613+
# If the parameter is a record with dict values and a syntax, check
614+
# all values.
615+
if (
616+
syntax[key].get('check') == 'record' and
617+
syntax[key].get('content') == 'dict' and
618+
syntax[key].get('syntax') is not None and key in options
619+
):
620+
for value in options[key].values():
621+
self._check_syntax(syntax[key]['syntax'], value, key)
604622

605623

606624
class Staff():

tests/Config.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,6 @@ def test_load_gpg_missing_keyring_or_key(self):
501501
'^Key (key|keyring) is required in dict parameter gpg$'
502502
):
503503
config.load(cfgfile.name)
504-
self.assertEqual(config.get('gpg'), None)
505504

506505
def test_load_gpg_unknown_key(self):
507506
"""Load gpg parameters raise DeclError if unknown key"""
@@ -1058,6 +1057,35 @@ def test_load_dict_merged_syntax(self):
10581057
self.assertEqual(param0['key1'], 'value2')
10591058
self.assertEqual(param0['key2'], 1)
10601059

1060+
def test_load_dict_merged_syntax_missing_required(self):
1061+
"""load() merges dict from multiple files with syntax and required param missing in one file"""
1062+
self._add_fake_params()
1063+
conf_files = [
1064+
make_temp_file(
1065+
textwrap.dedent(
1066+
"""
1067+
param0:
1068+
key1: value1
1069+
"""
1070+
)
1071+
),
1072+
make_temp_file(
1073+
textwrap.dedent(
1074+
"""
1075+
param0:
1076+
key2: 1
1077+
"""
1078+
)
1079+
),
1080+
]
1081+
config = Config()
1082+
config.load([conf_file.name for conf_file in conf_files])
1083+
param0 = config.get('param0')
1084+
self.assertTrue('key1' in param0)
1085+
self.assertTrue('key2' in param0)
1086+
self.assertEqual(param0['key1'], 'value1')
1087+
self.assertEqual(param0['key2'], 1)
1088+
10611089
def test_load_record(self):
10621090
"""Load record without content"""
10631091
Config.SYNTAX.update({

0 commit comments

Comments
 (0)