Log a warning when deprecated opts are used
This change logs the warning at lookup time instead of parse time because we have no way of knowing at parse time which options are deprecated. It does use a set to make sure it only logs a given deprecation warning once per option so it won't spam the logs. I could not find a way to log deprecations for cli args. argparse seems to only write the value to the undeprecated name, so there's no way to tell whether it came from a deprecated name originally. Since most of our opts are set through config files I don't see this as a huge issue though. I'm also not using the warnings module because its deprecation mechanisms seem more developer focused, and for config opts we need to notify the user. This is not possible with the warnings module in Python 2.6 (there's no way to send warnings to the log by default), and since we still need to support that for oslo libs I went with a simple logged warning instead. Change-Id: I4580831f34c48882fb07288fa8d55272b58e2674 Closes-Bug: 1390408
This commit is contained in:
Ben Nemec
parent
f9dbf86379
commit
dc1d2340d5
|
|
@ -1416,9 +1416,13 @@ class ConfigParser(iniparser.BaseParser):
|
|||
|
||||
|
||||
class MultiConfigParser(object):
|
||||
_deprecated_opt_message = ('Option "%s" from group "%s" is deprecated. '
|
||||
'Use option "%s" from group "%s".')
|
||||
|
||||
def __init__(self):
|
||||
self.parsed = []
|
||||
self._normalized = []
|
||||
self._emitted_deprecations = set()
|
||||
|
||||
def read(self, config_files):
|
||||
read_ok = []
|
||||
|
|
@ -1470,6 +1474,8 @@ class MultiConfigParser(object):
|
|||
if section not in sections:
|
||||
continue
|
||||
if name in sections[section]:
|
||||
self._check_deprecated((section, name), names[0],
|
||||
names[1:])
|
||||
val = sections[section][name]
|
||||
if multi:
|
||||
rvalue = val + rvalue
|
||||
|
|
@ -1479,6 +1485,32 @@ class MultiConfigParser(object):
|
|||
return rvalue
|
||||
raise KeyError
|
||||
|
||||
def _check_deprecated(self, name, current, deprecated):
|
||||
"""Check for usage of deprecated names.
|
||||
|
||||
:param name: A tuple of the form (group, name) representing the group
|
||||
and name where an opt value was found.
|
||||
:param current: A tuple of the form (group, name) representing the
|
||||
current name for an option.
|
||||
:param deprecated: A list of tuples with the same format as the name
|
||||
param which represent any deprecated names for an option.
|
||||
If the name param matches any entries in this list a
|
||||
deprecation warning will be logged.
|
||||
"""
|
||||
# Opts in the DEFAULT group may come in with a group name of either
|
||||
# 'DEFAULT' or None. Force them all to 'DEFAULT' since that's a more
|
||||
# user-friendly form.
|
||||
deprecated_names = set((g or 'DEFAULT', n) for (g, n) in deprecated)
|
||||
name = (name[0] or 'DEFAULT', name[1])
|
||||
if name in deprecated_names and name not in self._emitted_deprecations:
|
||||
self._emitted_deprecations.add(name)
|
||||
current = (current[0] or 'DEFAULT', current[1])
|
||||
# NOTE(bnemec): Not using versionutils for this to avoid a
|
||||
# circular dependency between oslo.config and whatever library
|
||||
# versionutils ends up in.
|
||||
LOG.warning(self._deprecated_opt_message, name[1],
|
||||
name[0], current[1], current[0])
|
||||
|
||||
|
||||
class _Namespace(argparse.Namespace):
|
||||
|
||||
|
|
|
|||
|
|
@ -3563,3 +3563,100 @@ class SectionsTestCase(base.BaseTestCase):
|
|||
config(args=[], default_config_files=[paths[0]])
|
||||
sections = set(config.list_all_sections())
|
||||
self.assertEqual(sections, set(['DEFAULT', 'BLAA']))
|
||||
|
||||
|
||||
class DeprecationWarningTestBase(BaseTestCase):
|
||||
def setUp(self):
|
||||
super(DeprecationWarningTestBase, self).setUp()
|
||||
self.log_fixture = self.useFixture(fixtures.FakeLogger())
|
||||
self._parser_class = cfg.MultiConfigParser
|
||||
|
||||
|
||||
class DeprecationWarningTestScenarios(DeprecationWarningTestBase):
|
||||
scenarios = [('default-deprecated', dict(deprecated=True,
|
||||
group='DEFAULT')),
|
||||
('default-not-deprecated', dict(deprecated=False,
|
||||
group='DEFAULT')),
|
||||
('other-deprecated', dict(deprecated=True,
|
||||
group='other')),
|
||||
('other-not-deprecated', dict(deprecated=False,
|
||||
group='other')),
|
||||
]
|
||||
|
||||
def test_deprecated_logging(self):
|
||||
self.conf.register_opt(cfg.StrOpt('foo', deprecated_name='bar'))
|
||||
self.conf.register_group(cfg.OptGroup('other'))
|
||||
self.conf.register_opt(cfg.StrOpt('foo', deprecated_name='bar'),
|
||||
group='other')
|
||||
if self.deprecated:
|
||||
content = 'bar=baz'
|
||||
else:
|
||||
content = 'foo=baz'
|
||||
paths = self.create_tempfiles([('test',
|
||||
'[' + self.group + ']\n' +
|
||||
content + '\n')])
|
||||
|
||||
self.conf(['--config-file', paths[0]])
|
||||
# Reference these twice to verify they only get logged once
|
||||
if self.group == 'DEFAULT':
|
||||
self.assertEqual('baz', self.conf.foo)
|
||||
self.assertEqual('baz', self.conf.foo)
|
||||
else:
|
||||
self.assertEqual('baz', self.conf.other.foo)
|
||||
self.assertEqual('baz', self.conf.other.foo)
|
||||
if self.deprecated:
|
||||
expected = (self._parser_class._deprecated_opt_message %
|
||||
('bar', self.group, 'foo', self.group) + '\n')
|
||||
else:
|
||||
expected = ''
|
||||
self.assertEqual(expected, self.log_fixture.output)
|
||||
|
||||
|
||||
class DeprecationWarningTests(DeprecationWarningTestBase):
|
||||
def test_DeprecatedOpt(self):
|
||||
default_deprecated = [cfg.DeprecatedOpt('bar')]
|
||||
other_deprecated = [cfg.DeprecatedOpt('baz', group='other')]
|
||||
self.conf.register_group(cfg.OptGroup('other'))
|
||||
self.conf.register_opt(cfg.StrOpt('foo',
|
||||
deprecated_opts=default_deprecated))
|
||||
self.conf.register_opt(cfg.StrOpt('foo',
|
||||
deprecated_opts=other_deprecated),
|
||||
group='other')
|
||||
paths = self.create_tempfiles([('test',
|
||||
'[DEFAULT]\n' +
|
||||
'bar=baz\n' +
|
||||
'[other]\n' +
|
||||
'baz=baz\n')])
|
||||
self.conf(['--config-file', paths[0]])
|
||||
self.assertEqual('baz', self.conf.foo)
|
||||
self.assertEqual('baz', self.conf.other.foo)
|
||||
self.assertIn('Option "bar" from group "DEFAULT"',
|
||||
self.log_fixture.output)
|
||||
self.assertIn('Option "baz" from group "other"',
|
||||
self.log_fixture.output)
|
||||
|
||||
def test_check_deprecated_default_none(self):
|
||||
parser = self._parser_class()
|
||||
deprecated_list = [(None, 'bar')]
|
||||
parser._check_deprecated(('DEFAULT', 'bar'), (None, 'foo'),
|
||||
deprecated_list)
|
||||
self.assert_message_logged('bar', 'DEFAULT', 'foo', 'DEFAULT')
|
||||
# Make sure check_deprecated didn't modify the list passed in
|
||||
self.assertEqual([(None, 'bar')], deprecated_list)
|
||||
|
||||
def test_check_deprecated_none_default(self):
|
||||
parser = self._parser_class()
|
||||
deprecated_list = [('DEFAULT', 'bar')]
|
||||
parser._check_deprecated((None, 'bar'), ('DEFAULT', 'foo'),
|
||||
deprecated_list)
|
||||
self.assert_message_logged('bar', 'DEFAULT', 'foo', 'DEFAULT')
|
||||
# Make sure check_deprecated didn't modify the list passed in
|
||||
self.assertEqual([('DEFAULT', 'bar')], deprecated_list)
|
||||
|
||||
def assert_message_logged(self, deprecated_name, deprecated_group,
|
||||
current_name, current_group):
|
||||
expected = (self._parser_class._deprecated_opt_message %
|
||||
(deprecated_name, deprecated_group,
|
||||
current_name, current_group)
|
||||
)
|
||||
self.assertEqual(expected + '\n', self.log_fixture.output)
|
||||
|
|
|
|||
Loading…
Reference in New Issue