From a50ea6f2827bedf95188442cfb50d2348cbb3194 Mon Sep 17 00:00:00 2001 From: Mehdi Abaakouk Date: Wed, 10 Sep 2014 15:29:08 +0200 Subject: Looks for variable subtitution in the same group When we do a variable subtitution we must try to look if the option is avialable into the same group and then into the default one for backward compatibilty. Otherwise variable subtitution doesn't works when we change the group of an option and deprecated the DEFAULT group. Change-Id: Ie5750ee028a58fba6da225a8c4b4221e23db829f Closes-bug: #1367790 --- oslo/config/cfg.py | 25 +++++++++++++++++-------- tests/test_cfg.py | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/oslo/config/cfg.py b/oslo/config/cfg.py index f41264a..fa3295f 100644 --- a/oslo/config/cfg.py +++ b/oslo/config/cfg.py @@ -2071,7 +2071,8 @@ class ConfigOpts(collections.Mapping): namespace = self._namespace def convert(value): - return self._convert_value(self._substitute(value, namespace), opt) + return self._convert_value( + self._substitute(value, group, namespace), opt) if namespace is not None: group_name = group.name if group else None @@ -2099,19 +2100,21 @@ class ConfigOpts(collections.Mapping): return None - def _substitute(self, value, namespace=None): + def _substitute(self, value, group=None, namespace=None): """Perform string template substitution. Substitute any template variables (for example $foo, ${bar}) in the supplied string value(s) with opt values. :param value: the string value, or list of string values + :param group: the group that retrieves the option value from :param namespace: the namespace object that retrieves the option value from :returns: the substituted string(s) """ if isinstance(value, list): - return [self._substitute(i, namespace=namespace) for i in value] + return [self._substitute(i, group=group, namespace=namespace) + for i in value] elif isinstance(value, str): # Treat a backslash followed by the dollar sign "\$" # the same as the string template escape "$$" as it is @@ -2119,8 +2122,9 @@ class ConfigOpts(collections.Mapping): if '\$' in value: value = value.replace('\$', '$$') tmpl = string.Template(value) - return tmpl.safe_substitute( - self.StrSubWrapper(self, namespace=namespace)) + ret = tmpl.safe_substitute( + self.StrSubWrapper(self, group=group, namespace=namespace)) + return ret else: return value @@ -2251,7 +2255,7 @@ class ConfigOpts(collections.Mapping): except KeyError: continue - value = self._substitute(value, namespace=namespace) + value = self._substitute(value, group=group, namespace=namespace) try: self._convert_value(value, opt) @@ -2362,13 +2366,14 @@ class ConfigOpts(collections.Mapping): Exposes opt values as a dict for string substitution. """ - def __init__(self, conf, namespace=None): + def __init__(self, conf, group=None, namespace=None): """Construct a StrSubWrapper object. :param conf: a ConfigOpts object """ self.conf = conf self.namespace = namespace + self.group = group def __getitem__(self, key): """Look up an opt value from the ConfigOpts object. @@ -2377,7 +2382,11 @@ class ConfigOpts(collections.Mapping): :returns: an opt value :raises: TemplateSubstitutionError if attribute is a group """ - value = self.conf._get(key, namespace=self.namespace) + try: + value = self.conf._get(key, group=self.group, + namespace=self.namespace) + except NoSuchOptError: + value = self.conf._get(key, namespace=self.namespace) if isinstance(value, self.conf.GroupAttr): raise TemplateSubstitutionError( 'substituting group %s not supported' % key) diff --git a/tests/test_cfg.py b/tests/test_cfg.py index 7b6d282..dab7542 100644 --- a/tests/test_cfg.py +++ b/tests/test_cfg.py @@ -2101,8 +2101,55 @@ class TemplateSubstitutionTestCase(BaseTestCase): self.assertTrue(hasattr(self.conf, 'ba')) self.assertTrue(hasattr(self.conf.ba, 'r')) + self.assertEqual(self.conf.foo, '123') self.assertEqual(self.conf.ba.r, 123) + def test_sub_group_from_default_deprecated(self): + self.conf.register_group(cfg.OptGroup('ba')) + self.conf.register_cli_opt(cfg.StrOpt( + 'foo', default='123', deprecated_group='DEFAULT'), group='ba') + self.conf.register_cli_opt(cfg.IntOpt('r', default='$foo'), group='ba') + + self.conf([]) + + self.assertTrue(hasattr(self.conf, 'ba')) + self.assertTrue(hasattr(self.conf.ba, 'foo')) + self.assertEqual(self.conf.ba.foo, '123') + self.assertTrue(hasattr(self.conf.ba, 'r')) + self.assertEqual(self.conf.ba.r, 123) + + def test_sub_group_from_args_deprecated(self): + self.conf.register_group(cfg.OptGroup('ba')) + self.conf.register_cli_opt(cfg.StrOpt( + 'foo', default='123', deprecated_group='DEFAULT'), group='ba') + self.conf.register_cli_opt(cfg.IntOpt('r', default='$foo'), group='ba') + + self.conf(['--ba-foo=4242']) + + self.assertTrue(hasattr(self.conf, 'ba')) + self.assertTrue(hasattr(self.conf.ba, 'foo')) + self.assertTrue(hasattr(self.conf.ba, 'r')) + self.assertEqual(self.conf.ba.foo, '4242') + self.assertEqual(self.conf.ba.r, 4242) + + def test_sub_group_from_configfile_deprecated(self): + self.conf.register_group(cfg.OptGroup('ba')) + self.conf.register_cli_opt(cfg.StrOpt( + 'foo', default='123', deprecated_group='DEFAULT'), group='ba') + self.conf.register_cli_opt(cfg.IntOpt('r', default='$foo'), group='ba') + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo=4242\n')]) + + self.conf(['--config-file', paths[0]]) + + self.assertTrue(hasattr(self.conf, 'ba')) + self.assertTrue(hasattr(self.conf.ba, 'foo')) + self.assertTrue(hasattr(self.conf.ba, 'r')) + self.assertEqual(self.conf.ba.foo, '4242') + self.assertEqual(self.conf.ba.r, 4242) + class ConfigDirTestCase(BaseTestCase): -- cgit v1.2.1