diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py index 1c003ea5..5bd81bff 100644 --- a/oslo_config/cfg.py +++ b/oslo_config/cfg.py @@ -1225,7 +1225,8 @@ class StrOpt(Opt): Option with ``type`` :class:`oslo_config.types.String` :param name: the option's name - :param choices: Optional sequence of valid values. + :param choices: Optional sequence of either valid values or tuples of valid + values with descriptions. :param quotes: If True and string is enclosed with single or double quotes, will strip those quotes. :param regex: Optional regular expression (string or compiled @@ -1248,6 +1249,10 @@ class StrOpt(Opt): .. versionchanged:: 2.7 Added *max_length* parameter + + .. versionchanged:: 5.2 + The *choices* parameter will now accept a sequence of tuples, where each + tuple is of form (*choice*, *description*) """ def __init__(self, name, choices=None, quotes=None, @@ -1275,10 +1280,11 @@ class StrOpt(Opt): if getattr(self.type, 'choices', None): choices_text = ', '.join([self._get_choice_text(choice) for choice in self.type.choices]) - if kwargs['help'] is not None: - kwargs['help'] += (' Allowed values: %s\n' % choices_text) - else: - kwargs['help'] = (' Allowed values: %s\n' % choices_text) + if kwargs['help'] is None: + kwargs['help'] = '' + + kwargs['help'].rstrip('\n') + kwargs['help'] += '\n Allowed values: %s\n' % choices_text return kwargs @@ -1448,7 +1454,8 @@ class PortOpt(Opt): :param name: the option's name :param min: minimum value the port can take :param max: maximum value the port can take - :param choices: Optional sequence of valid values. + :param choices: Optional sequence of either valid values or tuples of valid + values with descriptions. :param \*\*kwargs: arbitrary keyword arguments passed to :class:`Opt` .. versionadded:: 2.6 @@ -1458,6 +1465,9 @@ class PortOpt(Opt): Allow port number with 0. .. versionchanged:: 3.16 Added *min* and *max* parameters. + .. versionchanged:: 5.2 + The *choices* parameter will now accept a sequence of tuples, where each + tuple is of form (*choice*, *description*) """ def __init__(self, name, min=None, max=None, choices=None, **kwargs): diff --git a/oslo_config/generator.py b/oslo_config/generator.py index 7f622be7..7b6e5ad4 100644 --- a/oslo_config/generator.py +++ b/oslo_config/generator.py @@ -66,14 +66,18 @@ _generator_opts = [ 'longer help text for Sphinx documents.'), cfg.StrOpt( 'format', - help='Desired format for the output. "ini" is the only one which can ' - 'be used directly with oslo.config. "json" and "yaml" are ' - 'intended for third-party tools that want to write config files ' - 'based on the sample config data. "rst" can be used to dump ' - 'the text given to sphinx when building documentation using ' - 'the sphinx extension, for debugging.', + help='Desired format for the output.', default='ini', - choices=['ini', 'json', 'yaml', 'rst'], + choices=[ + ('ini', 'The only format that can be used directly with ' + 'oslo.config.'), + ('json', 'Intended for third-party tools that want to write ' + 'config files based on the sample config data.'), + ('yaml', 'Same as json'), + ('rst', 'Can be used to dump the text given to Sphinx when ' + 'building documentation using the Sphinx extension. ' + 'Useful for debugging,') + ], dest='format_'), ] @@ -257,9 +261,12 @@ class _OptFormatter(object): lines.append('# Maximum value: %d\n' % opt.type.max) if getattr(opt.type, 'choices', None): - choices_text = ', '.join([self._get_choice_text(choice) - for choice in opt.type.choices]) - lines.append('# Allowed values: %s\n' % choices_text) + lines.append('# Possible values:\n') + for choice in opt.type.choices: + help_text = '%s - %s' % ( + self._get_choice_text(choice), + opt.type.choices[choice] or '') + lines.extend(self._format_help(help_text)) try: if opt.mutable: @@ -581,9 +588,14 @@ def _build_entry(opt, group, namespace, conf): entry = {key: value for key, value in opt.__dict__.items() if not key.startswith('_')} entry['namespace'] = namespace - # In some types, choices is explicitly set to None. Force it to [] so it - # is always an iterable type. - entry['choices'] = getattr(entry['type'], 'choices', []) or [] + # Where present, we store choices as an OrderedDict. The default repr for + # this is not very machine readable, thus, it is switched to a list of + # tuples here. In addition, in some types, choices is explicitly set to + # None. Force these cases to [] so it is always an iterable type. + if getattr(entry['type'], 'choices', None): + entry['choices'] = list(entry['type'].choices.items()) + else: + entry['choices'] = [] entry['min'] = getattr(entry['type'], 'min', None) entry['max'] = getattr(entry['type'], 'max', None) entry['type'] = _format_type_name(entry['type']) diff --git a/oslo_config/sphinxext.py b/oslo_config/sphinxext.py index 580d605d..bfd359b8 100644 --- a/oslo_config/sphinxext.py +++ b/oslo_config/sphinxext.py @@ -165,6 +165,18 @@ def _format_group(app, namespace, group_name, group_obj, opt_list): yield _indent(help_text) yield '' + # 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 _indent('.. rubric:: Possible values') + yield '' + for choice in opt.type.choices: + yield _indent(_get_choice_text(choice)) + yield _indent(_indent( + opt.type.choices[choice] or '')) + yield '' + if opt.deprecated_opts: for line in _list_table( ['Group', 'Name'], diff --git a/oslo_config/tests/test_generator.py b/oslo_config/tests/test_generator.py index 4c92eb50..e01bbab3 100644 --- a/oslo_config/tests/test_generator.py +++ b/oslo_config/tests/test_generator.py @@ -431,7 +431,12 @@ class GeneratorTestCase(base.BaseTestCase): # # a string with choices (string value) -# Allowed values: , '', a, b, c +# Possible values: +# - +# '' - +# a - +# b - +# c - #choices_opt = a ''')), ('deprecated opt without deprecated group', @@ -1219,7 +1224,13 @@ class MachineReadableGeneratorTestCase(base.BaseTestCase): 'help': '', 'standard_opts': ['choices_opt'], 'opts': [{'advanced': False, - 'choices': (None, '', 'a', 'b', 'c'), + 'choices': [ + (None, None), + ('', None), + ('a', None), + ('b', None), + ('c', None) + ], 'default': 'a', 'deprecated_for_removal': False, 'deprecated_opts': [], diff --git a/oslo_config/tests/test_sphinxext.py b/oslo_config/tests/test_sphinxext.py index 416dabee..aee69e1d 100644 --- a/oslo_config/tests/test_sphinxext.py +++ b/oslo_config/tests/test_sphinxext.py @@ -179,6 +179,51 @@ class FormatGroupTest(base.BaseTestCase): ''').lstrip(), results) + def test_with_choices_with_descriptions(self): + results = '\n'.join(list(sphinxext._format_group( + app=mock.Mock(), + namespace=None, + group_name=None, + group_obj=None, + opt_list=[ + cfg.StrOpt( + 'opt_name', + choices=[ + ('a', 'a is the best'), + ('b', 'Actually, may-b I am better'), + ('c', 'c, I am clearly the greatest'), + (None, 'I am having none of this'), + ('', '')]), + ], + ))) + self.assertEqual(textwrap.dedent(''' + .. oslo.config:group:: DEFAULT + + .. oslo.config:option:: opt_name + + :Type: string + :Default: ```` + :Valid Values: a, b, c, , '' + + .. rubric:: Possible values + + a + a is the best + + b + Actually, may-b I am better + + c + c, I am clearly the greatest + + + I am having none of this + + '' + + + ''').lstrip(), results) + def test_group_obj_without_help(self): # option with None group placed in DEFAULT results = '\n'.join(list(sphinxext._format_group( diff --git a/oslo_config/tests/test_types.py b/oslo_config/tests/test_types.py index 1e357bde..dc8c501f 100644 --- a/oslo_config/tests/test_types.py +++ b/oslo_config/tests/test_types.py @@ -67,6 +67,11 @@ class StringTypeTests(TypeTestHelper, unittest.TestCase): self.type_instance = types.String(choices=('foo', 'bar')) self.assertConvertedValue('foo', 'foo') + def test_listed_value_dict(self): + self.type_instance = types.String(choices=[ + ('foo', 'ab'), ('bar', 'xy')]) + self.assertConvertedValue('foo', 'foo') + def test_unlisted_value(self): self.type_instance = types.String(choices=['foo', 'bar']) self.assertInvalid('baz') @@ -98,7 +103,11 @@ class StringTypeTests(TypeTestHelper, unittest.TestCase): def test_repr_with_choices_tuple(self): t = types.String(choices=('foo', 'bar')) - self.assertEqual('String(choices=(\'foo\', \'bar\'))', repr(t)) + self.assertEqual('String(choices=[\'foo\', \'bar\'])', repr(t)) + + def test_repr_with_choices_dict(self): + t = types.String(choices=[('foo', 'ab'), ('bar', 'xy')]) + self.assertEqual('String(choices=[\'foo\', \'bar\'])', repr(t)) def test_equal(self): self.assertTrue(types.String() == types.String()) @@ -108,9 +117,8 @@ class StringTypeTests(TypeTestHelper, unittest.TestCase): t2 = types.String(choices=['foo', 'bar']) t3 = types.String(choices=('foo', 'bar')) t4 = types.String(choices=['bar', 'foo']) - self.assertTrue(t1 == t2) - self.assertTrue(t1 == t3) - self.assertTrue(t1 == t4) + t5 = types.String(choices=[('foo', 'ab'), ('bar', 'xy')]) + self.assertTrue(t1 == t2 == t3 == t4 == t5) def test_not_equal_with_different_choices(self): t1 = types.String(choices=['foo', 'bar']) @@ -282,7 +290,11 @@ class IntegerTypeTests(TypeTestHelper, unittest.TestCase): def test_repr_with_choices_tuple(self): t = types.Integer(choices=(80, 457)) - self.assertEqual('Integer(choices=(80, 457))', repr(t)) + self.assertEqual('Integer(choices=[80, 457])', repr(t)) + + def test_repr_with_choices_dict(self): + t = types.Integer(choices=[(80, 'ab'), (457, 'xy')]) + self.assertEqual('Integer(choices=[80, 457])', repr(t)) def test_equal(self): self.assertTrue(types.Integer() == types.Integer()) @@ -302,8 +314,8 @@ class IntegerTypeTests(TypeTestHelper, unittest.TestCase): t1 = types.Integer(choices=[80, 457]) t2 = types.Integer(choices=[457, 80]) t3 = types.Integer(choices=(457, 80)) - self.assertTrue(t1 == t2) - self.assertTrue(t1 == t3) + t4 = types.Integer(choices=[(80, 'ab'), (457, 'xy')]) + self.assertTrue(t1 == t2 == t3 == t4) def test_not_equal(self): self.assertFalse(types.Integer(min=123) == types.Integer(min=456)) @@ -369,21 +381,24 @@ class IntegerTypeTests(TypeTestHelper, unittest.TestCase): self.assertRaises(ValueError, t, 201) self.assertRaises(ValueError, t, -457) - def test_with_choices_list(self): - t = types.Integer(choices=[80, 457]) + def _test_with_choices(self, t): self.assertRaises(ValueError, t, 1) self.assertRaises(ValueError, t, 200) self.assertRaises(ValueError, t, -457) t(80) t(457) + def test_with_choices_list(self): + t = types.Integer(choices=[80, 457]) + self._test_with_choices(t) + def test_with_choices_tuple(self): t = types.Integer(choices=(80, 457)) - self.assertRaises(ValueError, t, 1) - self.assertRaises(ValueError, t, 200) - self.assertRaises(ValueError, t, -457) - t(80) - t(457) + self._test_with_choices(t) + + def test_with_choices_dict(self): + t = types.Integer(choices=[(80, 'ab'), (457, 'xy')]) + self._test_with_choices(t) class FloatTypeTests(TypeTestHelper, unittest.TestCase): @@ -865,16 +880,29 @@ class PortTypeTests(TypeTestHelper, unittest.TestCase): def test_repr_with_choices_tuple(self): t = types.Port(choices=(80, 457)) - self.assertEqual('Port(choices=(80, 457))', repr(t)) + self.assertEqual('Port(choices=[80, 457])', repr(t)) - def test_choices(self): - t = types.Port(choices=[80, 457]) + def _test_with_choices(self, t): self.assertRaises(ValueError, t, 1) self.assertRaises(ValueError, t, 200) + self.assertRaises(ValueError, t, -457) t(80) t(457) + def test_with_choices_list(self): + t = types.Port(choices=[80, 457]) + self._test_with_choices(t) + + def test_with_choices_tuple(self): + t = types.Port(choices=(80, 457)) + self._test_with_choices(t) + + def test_with_choices_dict(self): + t = types.Port(choices=[(80, 'ab'), (457, 'xy')]) + self._test_with_choices(t) + def test_invalid_choices(self): + """Check for choices that are specifically invalid for ports.""" self.assertRaises(ValueError, types.Port, choices=[-1, 457]) self.assertRaises(ValueError, types.Port, choices=[1, 2, 3, 65536]) @@ -896,8 +924,8 @@ class PortTypeTests(TypeTestHelper, unittest.TestCase): t1 = types.Port(choices=[80, 457]) t2 = types.Port(choices=[457, 80]) t3 = types.Port(choices=(457, 80)) - self.assertTrue(t1 == t2) - self.assertTrue(t1 == t3) + t4 = types.Port(choices=[(457, 'ab'), (80, 'xy')]) + self.assertTrue(t1 == t2 == t3 == t4) def test_not_equal(self): self.assertFalse(types.Port(min=123) == types.Port(min=456)) @@ -973,19 +1001,3 @@ class PortTypeTests(TypeTestHelper, unittest.TestCase): t = types.Port(max=0) self.assertRaises(ValueError, t, 1) t(0) - - def test_with_choices_list(self): - t = types.Port(choices=[80, 457]) - self.assertRaises(ValueError, t, 1) - self.assertRaises(ValueError, t, 200) - self.assertRaises(ValueError, t, -457) - t(80) - t(457) - - def test_with_choices_tuple(self): - t = types.Port(choices=(80, 457)) - self.assertRaises(ValueError, t, 1) - self.assertRaises(ValueError, t, 200) - self.assertRaises(ValueError, t, -457) - t(80) - t(457) diff --git a/oslo_config/types.py b/oslo_config/types.py index 976f4972..4462b64f 100644 --- a/oslo_config/types.py +++ b/oslo_config/types.py @@ -19,6 +19,7 @@ Use these classes as values for the `type` argument to .. versionadded:: 1.3 """ +import collections import operator import re import warnings @@ -68,8 +69,8 @@ class String(ConfigType): String values do not get transformed and are returned as str objects. - :param choices: Optional sequence of valid values. Mutually - exclusive with 'regex'. + :param choices: Optional sequence of either valid values or tuples of valid + values with descriptions. Mutually exclusive with 'regex'. :param quotes: If True and string is enclosed with single or double quotes, will strip those quotes. Will signal error if string have quote at the beginning and no quote at @@ -96,6 +97,10 @@ class String(ConfigType): .. versionchanged:: 2.7 Added *max_length* parameter. Added *type_name* parameter. + + .. versionchanged:: 5.2 + The *choices* parameter will now accept a sequence of tuples, where each + tuple is of form (*choice*, *description*) """ def __init__(self, choices=None, quotes=False, regex=None, @@ -109,10 +114,17 @@ class String(ConfigType): self.quotes = quotes self.max_length = max_length or 0 - self.choices = choices + if choices is not None: + if not all(isinstance(choice, tuple) for choice in choices): + choices = [(choice, None) for choice in choices] + + self.choices = collections.OrderedDict(choices) + else: + self.choices = None + self.lower_case_choices = None if self.choices is not None and self.ignore_case: - self.lower_case_choices = [c.lower() for c in choices] + self.lower_case_choices = [c.lower() for c in self.choices] self.regex = regex if self.regex is not None: @@ -146,7 +158,7 @@ class String(ConfigType): # Check for case insensitive processed_value, choices = ((value.lower(), self.lower_case_choices) if self.ignore_case else - (value, self.choices)) + (value, self.choices.keys())) if processed_value in choices: return value @@ -158,7 +170,7 @@ class String(ConfigType): def __repr__(self): details = [] if self.choices is not None: - details.append("choices={!r}".format(self.choices)) + details.append("choices={!r}".format(list(self.choices.keys()))) if self.regex: details.append("regex=%r" % self.regex.pattern) if details: @@ -168,11 +180,12 @@ class String(ConfigType): def __eq__(self, other): return ( (self.__class__ == other.__class__) and - (set(self.choices) == set(other.choices) if - self.choices and other.choices else - self.choices == other.choices) and (self.quotes == other.quotes) and - (self.regex == other.regex) + (self.regex == other.regex) and + (set([x for x in self.choices or []]) == + set([x for x in other.choices or []]) if + self.choices and other.choices else + self.choices == other.choices) ) def _formatter(self, value): @@ -252,9 +265,14 @@ class Number(ConfigType): :param type_name: Type name to be used in the sample config file. :param min: Optional check that value is greater than or equal to min. :param max: Optional check that value is less than or equal to max. - :param choices: Optional sequence of valid values. + :param choices: Optional sequence of either valid values or tuples of valid + values with descriptions. .. versionadded:: 3.14 + + .. versionchanged:: 5.2 + The *choices* parameter will now accept a sequence of tuples, where each + tuple is of form (*choice*, *description*) """ def __init__(self, num_type, type_name, @@ -263,15 +281,24 @@ class Number(ConfigType): if min is not None and max is not None and max < min: raise ValueError('Max value is less than min value') - invalid_choices = [c for c in choices or [] + + if choices is not None: + if not all(isinstance(choice, tuple) for choice in choices): + choices = [(choice, None) for choice in choices] + + self.choices = collections.OrderedDict(choices) + else: + self.choices = None + + invalid_choices = [c for c in self.choices or [] if (min is not None and min > c) or (max is not None and max < c)] if invalid_choices: raise ValueError("Choices %s are out of bounds [%s..%s]" % (invalid_choices, min, max)) + self.min = min self.max = max - self.choices = choices self.num_type = num_type def __call__(self, value): @@ -297,7 +324,7 @@ class Number(ConfigType): def __repr__(self): props = [] if self.choices is not None: - props.append("choices={!r}".format(self.choices)) + props.append("choices={!r}".format(list(self.choices.keys()))) else: if self.min is not None: props.append('min=%g' % self.min) @@ -313,7 +340,8 @@ class Number(ConfigType): (self.__class__ == other.__class__) and (self.min == other.min) and (self.max == other.max) and - (set(self.choices) == set(other.choices) if + (set([x for x in self.choices or []]) == + set([x for x in other.choices or []]) if self.choices and other.choices else self.choices == other.choices) ) @@ -332,7 +360,8 @@ class Integer(Number): :param min: Optional check that value is greater than or equal to min. :param max: Optional check that value is less than or equal to max. :param type_name: Type name to be used in the sample config file. - :param choices: Optional sequence of valid values. + :param choices: Optional sequence of either valid values or tuples of valid + values with descriptions. .. versionchanged:: 2.4 The class now honors zero for *min* and *max* parameters. @@ -346,6 +375,10 @@ class Integer(Number): .. versionchanged:: 3.16 *choices* is no longer mutually exclusive with *min*/*max*. If those are supplied, all choices are verified to be within the range. + + .. versionchanged:: 5.2 + The *choices* parameter will now accept a sequence of tuples, where each + tuple is of form (*choice*, *description*) """ def __init__(self, min=None, max=None, type_name='integer value', @@ -385,9 +418,14 @@ class Port(Integer): :param min: Optional check that value is greater than or equal to min. :param max: Optional check that value is less than or equal to max. :param type_name: Type name to be used in the sample config file. - :param choices: Optional sequence of valid values. + :param choices: Optional sequence of either valid values or tuples of valid + values with descriptions. .. versionadded:: 3.16 + + .. versionchanged:: 5.2 + The *choices* parameter will now accept a sequence of tuples, where each + tuple is of form (*choice*, *description*) """ PORT_MIN = 0 diff --git a/releasenotes/notes/support-choice-descriptions-8b2d0c14fbd16b2a.yaml b/releasenotes/notes/support-choice-descriptions-8b2d0c14fbd16b2a.yaml new file mode 100644 index 00000000..11673815 --- /dev/null +++ b/releasenotes/notes/support-choice-descriptions-8b2d0c14fbd16b2a.yaml @@ -0,0 +1,14 @@ +--- +features: + - | + String, Number, Integer, Float and Port now support value-description + tuples in the interable provided for the *choice* parameter. Support for + value-only definitions is retained. + - | + StringOpt and PortOpt now support a value-description tuples in the + iterable provided for the *choice* parameter. Support for value-only + definitions is retained. + - | + *oslo-config-generator* and the Sphinx extension will now output + descriptions for option choices where provided. This will impact tooling + that relies on the *yaml* and *json* output of the former.