diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py index 68e0a9f6..58c292e9 100644 --- a/oslo_config/cfg.py +++ b/oslo_config/cfg.py @@ -888,13 +888,20 @@ class Opt(object): self.deprecated_reason = deprecated_reason self.deprecated_since = deprecated_since self._logged_deprecation = False - if deprecated_name is not None: - deprecated_name = deprecated_name.replace('-', '_') self.deprecated_opts = copy.deepcopy(deprecated_opts) or [] + for o in self.deprecated_opts: + if '-' in o.name: + self.deprecated_opts.append(DeprecatedOpt( + o.name.replace('-', '_'), + group=o.group)) if deprecated_name is not None or deprecated_group is not None: self.deprecated_opts.append(DeprecatedOpt(deprecated_name, group=deprecated_group)) + if deprecated_name and '-' in deprecated_name: + self.deprecated_opts.append(DeprecatedOpt( + deprecated_name.replace('-', '_'), + group=deprecated_group)) self._check_default() self.mutable = mutable diff --git a/oslo_config/generator.py b/oslo_config/generator.py index 746faf62..3bfa7f80 100644 --- a/oslo_config/generator.py +++ b/oslo_config/generator.py @@ -270,8 +270,12 @@ class _OptFormatter(object): (opt.dest, err)) for d in opt.deprecated_opts: - lines.append('# Deprecated group/name - [%s]/%s\n' % - (d.group or group_name, d.name or opt.dest)) + # NOTE(bnemec): opt names with a - are not valid in a config file, + # but it is possible to add a DeprecatedOpt with a - name. We + # want to ignore those as they won't work anyway. + if d.name and '-' not in d.name: + lines.append('# Deprecated group/name - [%s]/%s\n' % + (d.group or group_name, d.name or opt.dest)) if opt.deprecated_for_removal: if opt.deprecated_since: diff --git a/oslo_config/tests/test_cfg.py b/oslo_config/tests/test_cfg.py index b40934bb..a7fff5b7 100644 --- a/oslo_config/tests/test_cfg.py +++ b/oslo_config/tests/test_cfg.py @@ -195,6 +195,20 @@ class HelpTestCase(BaseTestCase): list = [abc, ghi, zba] self.assertEqual(sorted(list), list) + def test_print_help_with_deprecated(self): + f = moves.StringIO() + abc = cfg.StrOpt('a-bc', + deprecated_opts=[cfg.DeprecatedOpt('d-ef')]) + uvw = cfg.StrOpt('u-vw', + deprecated_name='x-yz') + + self.conf.register_cli_opt(abc) + self.conf.register_cli_opt(uvw) + self.conf([]) + self.conf.print_help(file=f) + self.assertIn('--a-bc A_BC, --d-ef A_BC, --d_ef A_BC', f.getvalue()) + self.assertIn('--u-vw U_VW, --x-yz U_VW, --x_yz U_VW', f.getvalue()) + class FindConfigFilesTestCase(BaseTestCase): @@ -2381,8 +2395,12 @@ class MappingInterfaceTestCase(BaseTestCase): self.assertEqual(self.conf['blaa'], self.conf.blaa) -class OptNameSeparatorTestCast(BaseTestCase): +class OptNameSeparatorTestCase(BaseTestCase): + # NOTE(bnemec): The broken* values in these scenarios are opt dests or + # config file names that are not actually valid, but can be added via a + # DeprecatedOpt. The tests only verify that those values do not end up + # in the final config object. scenarios = [ ('hyphen', dict(opt_name='foo-bar', @@ -2391,8 +2409,7 @@ class OptNameSeparatorTestCast(BaseTestCase): cf_name='foo_bar', broken_cf_name='foo-bar', cli_name='foo-bar', - broken_cli_name='foo_bar', - broken=True)), # FIXME(markmc): see #1279973 + hyphen=True)), ('underscore', dict(opt_name='foo_bar', opt_dest='foo_bar', @@ -2400,8 +2417,7 @@ class OptNameSeparatorTestCast(BaseTestCase): cf_name='foo_bar', broken_cf_name='foo-bar', cli_name='foo_bar', - broken_cli_name='foo_bar', - broken=False)), + hyphen=False)), ] def test_attribute_and_key_name(self): @@ -2447,11 +2463,7 @@ class OptNameSeparatorTestCast(BaseTestCase): self.conf.register_cli_opt(cfg.BoolOpt('foobar', deprecated_name=self.opt_name)) - # FIXME(markmc): this should be self.cli_name, see #1279973 - if self.broken: - self.conf(['--' + self.broken_cli_name]) - else: - self.conf(['--' + self.cli_name]) + self.conf(['--' + self.cli_name]) self.assertTrue(self.conf.foobar) @@ -2494,16 +2506,9 @@ class OptNameSeparatorTestCast(BaseTestCase): self.conf.register_opt(cfg.BoolOpt('foobar', deprecated_opts=oldopts)) - # FIXME(markmc): this should be self.cf_name, see #1279973 - if self.broken: - paths = self.create_tempfiles([('test', - '[DEFAULT]\n' + - self.broken_cf_name + - ' = True\n')]) - else: - paths = self.create_tempfiles([('test', - '[DEFAULT]\n' + - self.cf_name + ' = True\n')]) + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + + self.cf_name + ' = True\n')]) self.conf(['--config-file', paths[0]]) diff --git a/oslo_config/tests/test_generator.py b/oslo_config/tests/test_generator.py index c9af936b..8896b54b 100644 --- a/oslo_config/tests/test_generator.py +++ b/oslo_config/tests/test_generator.py @@ -103,6 +103,11 @@ class GeneratorTestCase(base.BaseTestCase): 'deprecated_opt_with_deprecated_group': cfg.StrOpt( 'bar', deprecated_name='foobar', deprecated_group='group1', help='deprecated'), + 'opt_with_DeprecatedOpt': cfg.BoolOpt( + 'foo-bar', + help='Opt with DeprecatedOpt', + deprecated_opts = [cfg.DeprecatedOpt('foo-bar', + group='deprecated')]), # Unknown Opt default must be a string 'unknown_type': cfg.Opt('unknown_opt', default='123', @@ -838,6 +843,18 @@ class GeneratorTestCase(base.BaseTestCase): # a string (string value) #str_opt = foo bar +''')), + ('opt_with_DeprecatedOpt', + dict(opts=[('test', [(None, [opts['opt_with_DeprecatedOpt']])])], + expected='''[DEFAULT] + +# +# From test +# + +# Opt with DeprecatedOpt (boolean value) +# Deprecated group/name - [deprecated]/foo_bar +#foo_bar = ''')), ]