6033N/AFrom 8ddd9180967fae11803b59e92cab0e51faa99cf6 Mon Sep 17 00:00:00 2001
6033N/AFrom: Ben Nemec <bnemec@redhat.com>
6033N/ADate: Tue, 31 Mar 2015 15:37:25 +0000
6033N/ASubject: Fix logging of deprecated opts with dest override
6033N/A
6033N/AWhen a deprecated opt is used and the new opt name has the dest
6033N/Aproperty overridden, we were logging the dest property instead of
6033N/Athe actual opt name in the deprecation warning. This is confusing
6033N/Abecause it leads to log messages such as:
6033N/A
6033N/AOption "username" from group "nova" is deprecated. Use option
6033N/A"username" from group "nova".
6033N/A
6033N/AThis is nonsense. In this case the proper new name was "user-name"
6033N/Abut because it had a dest override of "username" that's what was
6033N/Alogged.
6033N/A
6033N/ABecause the deprecation check code does not have access to the
6033N/Aactual opt itself, we need to pass the undeprecated name down from
6033N/Athe opt into the namespace code that does the check.
6033N/A
6033N/AI also tried simply adding a (group, name) tuple to the beginning
6033N/Aof the names list, but this subtly broke the CLI opt tests because
6033N/Athere are specific combinations of - and _ variants expected, and
6033N/Athis seems to violate those assumptions. I did not pursue it any
6033N/Afurther because I felt it was better to simply be explicit about
6033N/Athe current name and not mess around with adding non-existent opt
6033N/Anames to the lookup list solely for logging purposes.
6033N/A
6033N/AChange-Id: Ib1ea8ae2d60a9c508935bad27d7ddc2cdb5ab505
6033N/ACloses-Bug: #1438314
6033N/A---
6033N/A oslo_config/cfg.py | 14 +++++++++-----
6033N/A oslo_config/tests/test_cfg.py | 16 ++++++++++++++++
6033N/A 2 files changed, 25 insertions(+), 5 deletions(-)
6033N/A
6033N/Adiff --git a/oslo_config/cfg.py b/oslo_config/cfg.py
6033N/Aindex 77fd44d..f472139 100644
6033N/A--- a/oslo_config/cfg.py
6033N/A+++ b/oslo_config/cfg.py
6033N/A@@ -717,6 +717,7 @@ class Opt(object):
6033N/A :param group_name: a group name
6033N/A """
6033N/A names = [(group_name, self.dest)]
6033N/A+ current_name = (group_name, self.name)
6033N/A
6033N/A for opt in self.deprecated_opts:
6033N/A dname, dgroup = opt.name, opt.group
6033N/A@@ -724,7 +725,8 @@ class Opt(object):
6033N/A names.append((dgroup if dgroup else group_name,
6033N/A dname if dname else self.dest))
6033N/A
6033N/A- value = namespace._get_value(names, self.multi, self.positional)
6033N/A+ value = namespace._get_value(names, self.multi, self.positional,
6033N/A+ current_name)
6033N/A # The previous line will raise a KeyError if no value is set in the
6033N/A # config file, so we'll only log deprecations for set options.
6033N/A if self.deprecated_for_removal and not self._logged_deprecation:
6033N/A@@ -1469,7 +1471,7 @@ class MultiConfigParser(object):
6033N/A def get(self, names, multi=False):
6033N/A return self._get(names, multi=multi)
6033N/A
6033N/A- def _get(self, names, multi=False, normalized=False):
6033N/A+ def _get(self, names, multi=False, normalized=False, current_name=None):
6033N/A """Fetch a config file value from the parsed files.
6033N/A
6033N/A :param names: a list of (section, name) tuples
6033N/A@@ -1488,7 +1490,8 @@ class MultiConfigParser(object):
6033N/A if section not in sections:
6033N/A continue
6033N/A if name in sections[section]:
6033N/A- self._check_deprecated((section, name), names[0],
6033N/A+ current_name = current_name or names[0]
6033N/A+ self._check_deprecated((section, name), current_name,
6033N/A names[1:])
6033N/A val = sections[section][name]
6033N/A if multi:
6033N/A@@ -1641,7 +1644,7 @@ class _Namespace(argparse.Namespace):
6033N/A
6033N/A raise KeyError
6033N/A
6033N/A- def _get_value(self, names, multi, positional):
6033N/A+ def _get_value(self, names, multi, positional, current_name):
6033N/A """Fetch a value from config files.
6033N/A
6033N/A Multiple names for a given configuration option may be supplied so
6033N/A@@ -1658,7 +1661,8 @@ class _Namespace(argparse.Namespace):
6033N/A pass
6033N/A
6033N/A names = [(g if g is not None else 'DEFAULT', n) for g, n in names]
6033N/A- values = self._parser._get(names, multi=multi, normalized=True)
6033N/A+ values = self._parser._get(names, multi=multi, normalized=True,
6033N/A+ current_name=current_name)
6033N/A return values if multi else values[-1]
6033N/A
6033N/A
6033N/Adiff --git a/oslo_config/tests/test_cfg.py b/oslo_config/tests/test_cfg.py
6033N/Aindex 5518f53..e007494 100644
6033N/A--- a/oslo_config/tests/test_cfg.py
6033N/A+++ b/oslo_config/tests/test_cfg.py
6033N/A@@ -3724,3 +3724,19 @@ class DeprecationWarningTests(DeprecationWarningTestBase):
6033N/A 'removal. Its value may be silently ignored in the '
6033N/A 'future.\n')
6033N/A self.assertEqual(expected, self.log_fixture.output)
6033N/A+
6033N/A+ def test_deprecated_with_dest(self):
6033N/A+ self.conf.register_group(cfg.OptGroup('other'))
6033N/A+ self.conf.register_opt(cfg.StrOpt('foo-bar', deprecated_name='bar',
6033N/A+ dest='foo'),
6033N/A+ group='other')
6033N/A+ content = 'bar=baz'
6033N/A+ paths = self.create_tempfiles([('test',
6033N/A+ '[other]\n' +
6033N/A+ content + '\n')])
6033N/A+
6033N/A+ self.conf(['--config-file', paths[0]])
6033N/A+ self.assertEqual('baz', self.conf.other.foo)
6033N/A+ expected = (self._parser_class._deprecated_opt_message %
6033N/A+ ('bar', 'other', 'foo-bar', 'other') + '\n')
6033N/A+ self.assertEqual(expected, self.log_fixture.output)
6033N/A--
6033N/Acgit v0.11.2
6033N/A