Skip to content

Commit 57c0017

Browse files
rezibrezib
committed
Config: add possibility to deprecate parameters
This is now possible to mark configuration parameters as deprecated, by specifying the name of their replacement. Arbitrary levels of dict keys can be specified in the name using dot separator (ex: hash1.hash2.key3). A FutureWarning is emitted when deprecated parameter is loaded, to alert user of the change and inform of the new parameter name. When both the deprecated and replacement parameters are defined, the deprecated parameter is ignored. Unit tests are introduced to cover all cases of this feature.
1 parent 8558019 commit 57c0017

File tree

2 files changed

+295
-1
lines changed

2 files changed

+295
-1
lines changed

‎lib/rift/Config.py‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
"""
3636
import errno
3737
import os
38+
import warnings
39+
import logging
40+
3841
import yaml
3942

4043
from rift import DeclError
@@ -56,6 +59,9 @@ def _construct_mapping(loader, node):
5659
loader.flatten_mapping(node)
5760
return OrderedDict(loader.construct_pairs(node))
5861

62+
class RiftDeprecatedConfWarning(FutureWarning):
63+
"""Warning emitted when deprecated configuration parameter is loaded."""
64+
5965
OrderedLoader.add_constructor(
6066
yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG,
6167
_construct_mapping)
@@ -444,6 +450,12 @@ def set(self, key, value, arch=None):
444450
if key not in self.SYNTAX:
445451
raise DeclError(f"Unknown '{key}' key")
446452

453+
# Check not deprecated
454+
replacement = self.SYNTAX[key].get('deprecated')
455+
if replacement:
456+
raise DeclError(f"Parameter {key} is deprecated, use "
457+
f"{' > '.join(replacement.split('.'))} instead")
458+
447459
options = self._arch_options(arch)
448460
value = self._key_value(
449461
self.SYNTAX[key],
@@ -551,6 +563,73 @@ def _record_value(self, syntax, value):
551563
)
552564
return result
553565

566+
@staticmethod
567+
def _get_replacement_dict_key(data, replacement):
568+
"""
569+
Return a 2-tuple with the dict that contains the replacement parameter
570+
and the key of this parameter in this dict.
571+
"""
572+
sub = data
573+
replacement_items = replacement.split('.')
574+
# Browse in data dict depth until last replacement item.
575+
for index, item in enumerate(replacement_items, start=1):
576+
if index < len(replacement_items):
577+
if item not in sub:
578+
sub[item] = {}
579+
sub = sub[item]
580+
else:
581+
return sub, item
582+
return None
583+
584+
def _move_deprecated_param(self, data, param, value):
585+
"""
586+
If the given parameter is deprecated, move its value to its replacement
587+
parameter.
588+
"""
589+
# Leave if parameter not found in syntax, the error is managed in set()
590+
# method eventually.
591+
if param not in self.SYNTAX:
592+
return
593+
# Check presence of deprecated attribute and leave if not defined.
594+
replacement = self.SYNTAX[param].get("deprecated")
595+
if replacement is None:
596+
return
597+
# Warn user with FutureWarning.
598+
warnings.warn(f"Configuration parameter {param} is deprecated, use "
599+
f"{' > '.join(replacement.split('.'))} instead",
600+
RiftDeprecatedConfWarning)
601+
# Get position of replacement parameter.
602+
sub, item = Config._get_replacement_dict_key(data, replacement)
603+
# If both new and deprecated parameter are defined, emit warning log to
604+
# explain deprecated parameter is ignored. Else, move deprecated
605+
# parameter to its new place.
606+
if item in sub:
607+
logging.warning("Both deprecated parameter %s and new parameter "
608+
"%s are declared in configuration, deprecated "
609+
"parameter %s is ignored",
610+
param, replacement, param)
611+
else:
612+
sub[item] = value
613+
del data[param]
614+
615+
def _move_deprecated(self, data):
616+
"""
617+
Iterate over data dict to check for deprecated parameters and move them
618+
to their replacements.
619+
"""
620+
# Load generic options (ie. not architecture specific)
621+
for param, value in data.copy().items():
622+
# Skip architecture specific options
623+
if param in self.get('arch'):
624+
continue
625+
self._move_deprecated_param(data, param, value)
626+
627+
# Load architecture specific options
628+
for arch in self.get('arch'):
629+
if arch in data and isinstance(data[arch], dict):
630+
for param, value in data.copy()[arch].items():
631+
self._move_deprecated_param(data[arch], param, value)
632+
554633
def update(self, data):
555634
"""
556635
Update config content with data dict, checking data content respect
@@ -561,6 +640,9 @@ def update(self, data):
561640
self.set('arch', data['arch'])
562641
del data['arch']
563642

643+
# Look for deprecated parameters, and update dict with new parameters.
644+
self._move_deprecated(data)
645+
564646
# Load generic options (ie. not architecture specific)
565647
for key, value in data.items():
566648
# Skip architecture specific options

‎tests/Config.py‎

Lines changed: 213 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
_DEFAULT_QEMU_CMD, _DEFAULT_REPO_CMD, \
1818
_DEFAULT_SHARED_FS_TYPE, _DEFAULT_VIRTIOFSD, \
1919
_DEFAULT_SYNC_METHOD, _DEFAULT_SYNC_INCLUDE, \
20-
_DEFAULT_SYNC_EXCLUDE
20+
_DEFAULT_SYNC_EXCLUDE, \
21+
RiftDeprecatedConfWarning
2122

2223
class ConfigTest(RiftTestCase):
2324

@@ -1273,6 +1274,217 @@ def test_load_record_merged(self):
12731274
self.assertEqual(record0['value2'], 20)
12741275
self.assertEqual(record0['value3'], 3)
12751276

1277+
def test_deprecated_param(self):
1278+
"""Load deprecated parameter"""
1279+
Config.SYNTAX.update({
1280+
'new_parameter': {},
1281+
'old_parameter': {
1282+
'deprecated': 'new_parameter',
1283+
},
1284+
})
1285+
cfgfile = make_temp_file(
1286+
textwrap.dedent(
1287+
"""
1288+
old_parameter: test_value
1289+
"""
1290+
)
1291+
)
1292+
config = Config()
1293+
with self.assertWarns(RiftDeprecatedConfWarning) as cm:
1294+
config.load(cfgfile.name)
1295+
self.assertEqual(
1296+
config.get('new_parameter'), 'test_value'
1297+
)
1298+
self.assertIsNone(config.get('old_parameter'))
1299+
self.assertEqual(
1300+
str(cm.warning),
1301+
"Configuration parameter old_parameter is deprecated, use "
1302+
"new_parameter instead"
1303+
)
1304+
# Check set() on deprecated parameter raise declaration error.
1305+
with self.assertRaisesRegex(
1306+
DeclError,
1307+
"^Parameter old_parameter is deprecated, use new_parameter instead$"
1308+
):
1309+
config.set('old_parameter', 'another value')
1310+
1311+
1312+
def test_deprecated_param_with_arch(self):
1313+
"""Load deprecated parameter with arch override"""
1314+
Config.SYNTAX.update({
1315+
'new_parameter_1': {},
1316+
'old_parameter_1': {
1317+
'deprecated': 'new_parameter_1',
1318+
},
1319+
'new_parameter_2': {},
1320+
'old_parameter_2': {
1321+
'deprecated': 'new_parameter_2',
1322+
},
1323+
})
1324+
cfgfile = make_temp_file(
1325+
textwrap.dedent(
1326+
"""
1327+
arch:
1328+
- x86_64
1329+
- aarch64
1330+
old_parameter_1: generic_value
1331+
aarch64:
1332+
old_parameter_1: aarch64_value
1333+
old_parameter_2: generic_value_$arch
1334+
"""
1335+
)
1336+
)
1337+
config = Config()
1338+
with self.assertWarns(RiftDeprecatedConfWarning):
1339+
config.load(cfgfile.name)
1340+
self.assertEqual(config.get('new_parameter_1'), 'generic_value')
1341+
self.assertEqual(
1342+
config.get('new_parameter_1', arch='x86_64'), 'generic_value'
1343+
)
1344+
self.assertEqual(
1345+
config.get('new_parameter_1', arch='aarch64'), 'aarch64_value'
1346+
)
1347+
self.assertEqual(
1348+
config.get('new_parameter_2', arch='x86_64'), 'generic_value_x86_64'
1349+
)
1350+
self.assertEqual(
1351+
config.get('new_parameter_2', arch='aarch64'),
1352+
'generic_value_aarch64'
1353+
)
1354+
self.assertIsNone(config.get('old_parameter_1'))
1355+
self.assertIsNone(config.get('old_parameter_2'))
1356+
1357+
def test_deprecated_param_subdict(self):
1358+
"""Load deprecated parameter moved in subdict"""
1359+
Config.SYNTAX.update({
1360+
'new_parameter': {
1361+
'check': 'dict',
1362+
'syntax': {
1363+
'sub_dict1': {
1364+
'check': 'dict',
1365+
'syntax': {
1366+
'new_key1': {}
1367+
},
1368+
},
1369+
},
1370+
},
1371+
'old_parameter': {
1372+
'deprecated': 'new_parameter.sub_dict1.new_key1',
1373+
},
1374+
})
1375+
cfgfile = make_temp_file(
1376+
textwrap.dedent(
1377+
"""
1378+
old_parameter: test_value
1379+
"""
1380+
)
1381+
)
1382+
config = Config()
1383+
with self.assertWarns(RiftDeprecatedConfWarning) as cm:
1384+
config.load(cfgfile.name)
1385+
self.assertEqual(
1386+
config.get('new_parameter').get('sub_dict1').get('new_key1'), 'test_value'
1387+
)
1388+
self.assertIsNone(config.get('old_parameter'))
1389+
self.assertEqual(
1390+
str(cm.warning),
1391+
"Configuration parameter old_parameter is deprecated, use "
1392+
"new_parameter > sub_dict1 > new_key1 instead"
1393+
)
1394+
1395+
def test_deprecated_param_invalid_type(self):
1396+
"""Deprecated parameter with invalid type error"""
1397+
Config.SYNTAX.update({
1398+
'new_parameter': {
1399+
'check': 'digit',
1400+
},
1401+
'old_parameter': {
1402+
'deprecated': 'new_parameter',
1403+
},
1404+
})
1405+
cfgfile = make_temp_file(
1406+
textwrap.dedent(
1407+
"""
1408+
old_parameter: test_value
1409+
"""
1410+
)
1411+
)
1412+
config = Config()
1413+
# In this case, Config.set() should emit a declaration error on
1414+
# new_parameter. Also check warning is emited for old_parameter.
1415+
with self.assertWarns(RiftDeprecatedConfWarning) as aw:
1416+
with self.assertRaisesRegex(
1417+
DeclError,
1418+
"Bad data type str for 'new_parameter'"
1419+
):
1420+
config.load(cfgfile.name)
1421+
self.assertEqual(
1422+
str(aw.warning),
1423+
"Configuration parameter old_parameter is deprecated, use "
1424+
"new_parameter instead"
1425+
)
1426+
1427+
def test_deprecated_param_unexisting_replacement(self):
1428+
"""Deprecated parameter without replacement error"""
1429+
Config.SYNTAX.update({
1430+
'old_parameter': {
1431+
'deprecated': 'new_parameter',
1432+
},
1433+
})
1434+
cfgfile = make_temp_file(
1435+
textwrap.dedent(
1436+
"""
1437+
old_parameter: test_value
1438+
"""
1439+
)
1440+
)
1441+
config = Config()
1442+
# In this case, Config.set() should emit a declaration error.
1443+
with self.assertWarns(RiftDeprecatedConfWarning) as aw:
1444+
with self.assertRaisesRegex(
1445+
DeclError, "Unknown 'new_parameter' key"):
1446+
config.load(cfgfile.name)
1447+
self.assertEqual(
1448+
str(aw.warning),
1449+
"Configuration parameter old_parameter is deprecated, use "
1450+
"new_parameter instead"
1451+
)
1452+
1453+
def test_deprecated_param_conflict(self):
1454+
"""Load deprecated parameter conflict with replacement parameter"""
1455+
Config.SYNTAX.update({
1456+
'new_parameter': {},
1457+
'old_parameter': {
1458+
'deprecated': 'new_parameter',
1459+
},
1460+
})
1461+
cfgfile = make_temp_file(
1462+
textwrap.dedent(
1463+
"""
1464+
new_parameter: test_new_value
1465+
old_parameter: test_old_value
1466+
"""
1467+
)
1468+
)
1469+
config = Config()
1470+
with self.assertWarns(RiftDeprecatedConfWarning) as aw:
1471+
with self.assertLogs(level='WARNING') as al:
1472+
config.load(cfgfile.name)
1473+
self.assertEqual(
1474+
config.get('new_parameter'), 'test_new_value'
1475+
)
1476+
self.assertIsNone(config.get('old_parameter'))
1477+
self.assertEqual(
1478+
str(aw.warning),
1479+
"Configuration parameter old_parameter is deprecated, use "
1480+
"new_parameter instead"
1481+
)
1482+
self.assertEqual(
1483+
al.output,
1484+
['WARNING:root:Both deprecated parameter old_parameter and new '
1485+
'parameter new_parameter are declared in configuration, '
1486+
'deprecated parameter old_parameter is ignored']
1487+
)
12761488

12771489
class ProjectConfigTest(RiftTestCase):
12781490

0 commit comments

Comments
 (0)