From 10b03c6b0fa0659af24eb6236688823b399d802e Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 16 Mar 2018 14:04:02 +0000 Subject: [PATCH] sphinxext: Separate parsing of group and opts This makes it easier to reason about going forward as we're going to make some changes to how this is generated. Change-Id: Iedd19802e612a647bb2f8f1c133344323e597d0e --- oslo_config/sphinxext.py | 212 +++++++++++++++------------- oslo_config/tests/test_sphinxext.py | 66 ++++----- 2 files changed, 145 insertions(+), 133 deletions(-) diff --git a/oslo_config/sphinxext.py b/oslo_config/sphinxext.py index eb04da53..34a1ef08 100644 --- a/oslo_config/sphinxext.py +++ b/oslo_config/sphinxext.py @@ -90,10 +90,106 @@ def _get_choice_text(choice): return six.text_type(choice) -def _format_group(app, namespace, group_name, group_obj, opt_list): - group_name = group_name or 'DEFAULT' - app.debug('[oslo.config] %s %s' % (namespace, group_name)) +def _format_opt(opt, group_name): + opt_type = _TYPE_DESCRIPTIONS.get(type(opt), + 'unknown type') + yield '.. oslo.config:option:: %s' % opt.dest + yield '' + yield _indent(':Type: %s' % opt_type) + for default in generator._format_defaults(opt): + if default: + default = '``' + default + '``' + yield _indent(':Default: %s' % default) + if getattr(opt.type, 'min', None) is not None: + yield _indent(':Minimum Value: %s' % opt.type.min) + if getattr(opt.type, 'max', None) is not None: + yield _indent(':Maximum Value: %s' % opt.type.max) + if getattr(opt.type, 'choices', None): + choices_text = ', '.join([_get_choice_text(choice) + for choice in opt.type.choices]) + yield _indent(':Valid Values: %s' % choices_text) + try: + if opt.mutable: + yield _indent( + ':Mutable: This option can be changed without restarting.' + ) + except AttributeError as err: + # NOTE(dhellmann): keystoneauth defines its own Opt class, + # and neutron (at least) returns instances of those + # classes instead of oslo_config Opt instances. The new + # mutable attribute is the first property where the API + # isn't supported in the external class, so we can use + # this failure to emit a warning. See + # https://bugs.launchpad.net/keystoneauth/+bug/1548433 for + # more details. + import warnings + if not isinstance(cfg.Opt, opt): + warnings.warn( + 'Incompatible option class for %s (%r): %s' % + (opt.dest, opt.__class__, err), + ) + else: + warnings.warn('Failed to fully format sample for %s: %s' % + (opt.dest, err)) + if opt.advanced: + yield _indent( + ':Advanced Option: Intended for advanced users and not used') + yield _indent( + 'by the majority of users, and might have a significant', 6) + yield _indent( + 'effect on stability and/or performance.', 6) + try: + help_text = opt.help % {'default': 'the value above'} + except (TypeError, KeyError, ValueError): + # There is no mention of the default in the help string, + # the string had some unknown key, or the string contained + # invalid formatting characters + help_text = opt.help + if help_text: + yield '' + yield _indent(help_text.strip()) + + # We don't bother outputting this if not using new-style choices with + # inline descriptions + if getattr(opt.type, 'choices', None) and not all( + x is None for x in opt.type.choices.values()): + yield '' + yield _indent('.. rubric:: Possible values') + for choice in opt.type.choices: + yield '' + yield _indent(_get_choice_text(choice)) + yield _indent(_indent( + opt.type.choices[choice] or '')) + + if opt.deprecated_opts: + yield '' + for line in _list_table( + ['Group', 'Name'], + ((d.group or group_name, + d.name or opt.dest or 'UNSET') + for d in opt.deprecated_opts), + title='Deprecated Variations'): + yield _indent(line) + + if opt.deprecated_for_removal: + yield '' + yield _indent('.. warning::') + if opt.deprecated_since: + yield _indent(' This option is deprecated for removal ' + 'since %s.' % opt.deprecated_since) + else: + yield _indent(' This option is deprecated for removal.') + yield _indent(' Its value may be silently ignored ') + yield _indent(' in the future.') + if opt.deprecated_reason: + yield '' + yield _indent(' :Reason: ' + opt.deprecated_reason) + + yield '' + + +def _format_group(namespace, group_name, group_obj): yield '.. oslo.config:group:: %s' % group_name if namespace: yield ' :namespace: %s' % namespace @@ -103,103 +199,17 @@ def _format_group(app, namespace, group_name, group_obj, opt_list): yield _indent(group_obj.help.rstrip()) yield '' + +def _format_group_opts(app, namespace, group_name, group_obj, opt_list): + group_name = group_name or 'DEFAULT' + app.debug('[oslo.config] %s %s' % (namespace, group_name)) + + for line in _format_group(namespace, group_name, group_obj): + yield line + for opt in opt_list: - opt_type = _TYPE_DESCRIPTIONS.get(type(opt), - 'unknown type') - yield '.. oslo.config:option:: %s' % opt.dest - yield '' - yield _indent(':Type: %s' % opt_type) - for default in generator._format_defaults(opt): - if default: - default = '``' + default + '``' - yield _indent(':Default: %s' % default) - if getattr(opt.type, 'min', None) is not None: - yield _indent(':Minimum Value: %s' % opt.type.min) - if getattr(opt.type, 'max', None) is not None: - yield _indent(':Maximum Value: %s' % opt.type.max) - if getattr(opt.type, 'choices', None): - choices_text = ', '.join([_get_choice_text(choice) - for choice in opt.type.choices]) - yield _indent(':Valid Values: %s' % choices_text) - try: - if opt.mutable: - yield _indent( - ':Mutable: This option can be changed without restarting.' - ) - except AttributeError as err: - # NOTE(dhellmann): keystoneauth defines its own Opt class, - # and neutron (at least) returns instances of those - # classes instead of oslo_config Opt instances. The new - # mutable attribute is the first property where the API - # isn't supported in the external class, so we can use - # this failure to emit a warning. See - # https://bugs.launchpad.net/keystoneauth/+bug/1548433 for - # more details. - import warnings - if not isinstance(cfg.Opt, opt): - warnings.warn( - 'Incompatible option class for %s (%r): %s' % - (opt.dest, opt.__class__, err), - ) - else: - warnings.warn('Failed to fully format sample for %s: %s' % - (opt.dest, err)) - if opt.advanced: - yield _indent( - ':Advanced Option: Intended for advanced users and not used') - yield _indent( - 'by the majority of users, and might have a significant', 6) - yield _indent( - 'effect on stability and/or performance.', 6) - - try: - help_text = opt.help % {'default': 'the value above'} - except (TypeError, KeyError, ValueError): - # There is no mention of the default in the help string, - # the string had some unknown key, or the string contained - # invalid formatting characters - help_text = opt.help - if help_text: - yield '' - yield _indent(help_text) - - # We don't bother outputting this if not using new-style choices with - # inline descriptions - if getattr(opt.type, 'choices', None) and not all( - x is None for x in opt.type.choices.values()): - yield '' - yield _indent('.. rubric:: Possible values') - for choice in opt.type.choices: - yield '' - yield _indent(_get_choice_text(choice)) - yield _indent(_indent( - opt.type.choices[choice] or '')) - - if opt.deprecated_opts: - yield '' - for line in _list_table( - ['Group', 'Name'], - ((d.group or group_name, - d.name or opt.dest or 'UNSET') - for d in opt.deprecated_opts), - title='Deprecated Variations'): - yield _indent(line) - - if opt.deprecated_for_removal: - yield '' - yield _indent('.. warning::') - if opt.deprecated_since: - yield _indent(' This option is deprecated for removal ' - 'since %s.' % opt.deprecated_since) - else: - yield _indent(' This option is deprecated for removal.') - yield _indent(' Its value may be silently ignored ') - yield _indent(' in the future.') - if opt.deprecated_reason: - yield '' - yield _indent(' :Reason: ' + opt.deprecated_reason) - - yield '' + for line in _format_opt(opt, group_name): + yield line def _format_option_help(app, namespaces, split_namespaces): @@ -220,7 +230,7 @@ def _format_option_help(app, namespaces, split_namespaces): group = None if group_name is None: group_name = 'DEFAULT' - lines = _format_group( + lines = _format_group_opts( app=app, namespace=namespace, group_name=group_name, @@ -247,7 +257,7 @@ def _format_option_help(app, namespaces, split_namespaces): group_objs.setdefault(group_name, group) by_section.setdefault(group_name, []).extend(group_opts) for group_name, group_opts in sorted(by_section.items()): - lines = _format_group( + lines = _format_group_opts( app=app, namespace=None, group_name=group_name, diff --git a/oslo_config/tests/test_sphinxext.py b/oslo_config/tests/test_sphinxext.py index b1f4410d..5a89e8aa 100644 --- a/oslo_config/tests/test_sphinxext.py +++ b/oslo_config/tests/test_sphinxext.py @@ -23,7 +23,7 @@ class FormatGroupTest(base.BaseTestCase): def test_none_in_default(self): # option with None group placed in DEFAULT - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -45,7 +45,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_with_default_value(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -68,7 +68,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_with_min(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -89,7 +89,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_with_min_0(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -110,7 +110,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_with_max(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -131,7 +131,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_with_max_0(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -152,7 +152,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_with_choices(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -173,7 +173,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_with_choices_with_descriptions(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -218,7 +218,7 @@ class FormatGroupTest(base.BaseTestCase): def test_group_obj_without_help(self): # option with None group placed in DEFAULT - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name='group', @@ -236,7 +236,7 @@ class FormatGroupTest(base.BaseTestCase): def test_group_obj_with_help(self): # option with None group placed in DEFAULT - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name='group', @@ -255,7 +255,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_deprecated_opts_without_deprecated_group(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -284,7 +284,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_deprecated_opts_with_deprecated_group(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -314,7 +314,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_deprecated_for_removal(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -332,7 +332,7 @@ class FormatGroupTest(base.BaseTestCase): self.assertIn('since 13.0', results) def test_mutable(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -353,7 +353,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_not_mutable(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -373,7 +373,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_advanced(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -396,7 +396,7 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) def test_not_advanced(self): - results = '\n'.join(list(sphinxext._format_group( + results = '\n'.join(list(sphinxext._format_group_opts( app=mock.Mock(), namespace=None, group_name=None, @@ -419,8 +419,8 @@ class FormatGroupTest(base.BaseTestCase): class FormatOptionHelpTest(base.BaseTestCase): @mock.patch('oslo_config.generator._list_opts') - @mock.patch('oslo_config.sphinxext._format_group') - def test_split_namespaces(self, _format_group, _list_opts): + @mock.patch('oslo_config.sphinxext._format_group_opts') + def test_split_namespaces(self, _format_group_opts, _list_opts): _list_opts.return_value = [ ('namespace1', [(None, ['opt1'])]), ('namespace2', [(None, ['opt2'])]), @@ -429,14 +429,14 @@ class FormatOptionHelpTest(base.BaseTestCase): app=None, namespaces=['namespace1', 'namespace2'], split_namespaces=True)) - _format_group.assert_any_call( + _format_group_opts.assert_any_call( app=None, namespace='namespace1', group_name='DEFAULT', group_obj=None, opt_list=['opt1'], ) - _format_group.assert_any_call( + _format_group_opts.assert_any_call( app=None, namespace='namespace2', group_name='DEFAULT', @@ -445,8 +445,8 @@ class FormatOptionHelpTest(base.BaseTestCase): ) @mock.patch('oslo_config.generator._list_opts') - @mock.patch('oslo_config.sphinxext._format_group') - def test_dont_split_namespaces(self, _format_group, _list_opts): + @mock.patch('oslo_config.sphinxext._format_group_opts') + def test_dont_split_namespaces(self, _format_group_opts, _list_opts): _list_opts.return_value = [ ('namespace1', [(None, ['opt1'])]), ('namespace2', [(None, ['opt2'])]), @@ -455,7 +455,7 @@ class FormatOptionHelpTest(base.BaseTestCase): app=None, namespaces=['namespace1', 'namespace2'], split_namespaces=False)) - _format_group.assert_called_once_with( + _format_group_opts.assert_called_once_with( app=None, namespace=None, group_name='DEFAULT', @@ -464,8 +464,9 @@ class FormatOptionHelpTest(base.BaseTestCase): ) @mock.patch('oslo_config.generator._list_opts') - @mock.patch('oslo_config.sphinxext._format_group') - def test_dont_split_namespaces_with_group(self, _format_group, _list_opts): + @mock.patch('oslo_config.sphinxext._format_group_opts') + def test_dont_split_namespaces_with_group(self, _format_group_opts, + _list_opts): grp_obj = cfg.OptGroup('grp1') _list_opts.return_value = [ ('namespace1', [(grp_obj, ['opt1'])]), @@ -475,7 +476,7 @@ class FormatOptionHelpTest(base.BaseTestCase): app=None, namespaces=['namespace1', 'namespace2'], split_namespaces=False)) - _format_group.assert_any_call( + _format_group_opts.assert_any_call( app=None, namespace=None, group_name='grp1', @@ -484,8 +485,9 @@ class FormatOptionHelpTest(base.BaseTestCase): ) @mock.patch('oslo_config.generator._list_opts') - @mock.patch('oslo_config.sphinxext._format_group') - def test_split_namespaces_with_group(self, _format_group, _list_opts): + @mock.patch('oslo_config.sphinxext._format_group_opts') + def test_split_namespaces_with_group(self, _format_group_opts, + _list_opts): grp_obj = cfg.OptGroup('grp1') _list_opts.return_value = [ ('namespace1', [(grp_obj, ['opt1'])]), @@ -495,15 +497,15 @@ class FormatOptionHelpTest(base.BaseTestCase): app=None, namespaces=['namespace1', 'namespace2'], split_namespaces=True)) - print(_format_group.call_args_list) - _format_group.assert_any_call( + print(_format_group_opts.call_args_list) + _format_group_opts.assert_any_call( app=None, namespace='namespace1', group_name='grp1', group_obj=grp_obj, opt_list=['opt1'], ) - _format_group.assert_any_call( + _format_group_opts.assert_any_call( app=None, namespace='namespace2', group_name='grp1',