openstack/oslo.config
Code Issues Proposed changes

Merge "Assume positional arguments are required"

Browse Source
This commit is contained in:
Zuul 2019-12-20 10:32:44 +00:00 committed by Gerrit Code Review
commit 543daf4f7a
4 changed files with 34 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/source/reference/command-line.rst
View File

@ -17,6 +17,10 @@ constructor argument:
>>> conf.bar
['a', 'b']
By default, positional arguments are also required. You may opt-out of this
behavior by setting ``required=False``, to have an optional positional
argument.
Sub-Parsers
-----------

9
oslo_config/cfg.py
View File

@ -540,7 +540,7 @@ class Opt(object):
def __init__(self, name, type=None, dest=None, short=None,
default=None, positional=False, metavar=None, help=None,
secret=False, required=False,
secret=False, required=None,
deprecated_name=None, deprecated_group=None,
deprecated_opts=None, sample_default=None,
deprecated_for_removal=False, deprecated_reason=None,
@ -556,6 +556,11 @@ class Opt(object):
raise TypeError('type must be callable')
self.type = type
# By default, non-positional options are *optional*, and positional
# options are *required*.
if required is None:
required = True if positional else False
if dest is None:
self.dest = self.name.replace('-', '_')
else:
@ -751,7 +756,7 @@ class Opt(object):
if group is not None:
dest = group.name + '_' + dest
kwargs['dest'] = dest
else:
elif not self.required:
kwargs['nargs'] = '?'
kwargs.update({'default': None,
'metavar': self.metavar,

36
oslo_config/tests/test_cfg.py
View File

@ -234,7 +234,8 @@ class HelpTestCase(BaseTestCase):
def test_print_sorted_help_with_positionals(self):
f = moves.StringIO()
self.conf.register_cli_opt(cfg.StrOpt('pst', positional=True))
self.conf.register_cli_opt(
cfg.StrOpt('pst', positional=True, required=False))
self.conf.register_cli_opt(cfg.StrOpt('abc'))
self.conf.register_cli_opt(cfg.StrOpt('zba'))
self.conf.register_cli_opt(cfg.StrOpt('ghi'))
@ -813,7 +814,8 @@ class PositionalTestCase(BaseTestCase):
def _do_pos_test(self, opt_class, default, cli_args, value):
self.conf.register_cli_opt(opt_class('foo',
default=default,
positional=True))
positional=True,
required=False))
self.conf(cli_args)
@ -940,7 +942,7 @@ class PositionalTestCase(BaseTestCase):
self.assertRaises(SystemExit, self.conf, ['--help'])
self.assertIn(' foo\n', sys.stdout.getvalue())
self.assertRaises(cfg.RequiredOptError, self.conf, [])
self.assertRaises(SystemExit, self.conf, [])
def test_optional_positional_opt_defined(self):
self.conf.register_cli_opt(
@ -948,11 +950,7 @@ class PositionalTestCase(BaseTestCase):
self.useFixture(fixtures.MonkeyPatch('sys.stdout', moves.StringIO()))
self.assertRaises(SystemExit, self.conf, ['--help'])
# FIXME(dolphm): Due to bug 1676989, this argument appears as a
# required argument in the CLI help. Instead, the following
# commented-out code should work:
# self.assertIn(' [foo]\n', sys.stdout.getvalue())
self.assertIn(' foo\n', sys.stdout.getvalue())
self.assertIn(' [foo]\n', sys.stdout.getvalue())
self.conf(['bar'])
@ -965,11 +963,7 @@ class PositionalTestCase(BaseTestCase):
self.useFixture(fixtures.MonkeyPatch('sys.stdout', moves.StringIO()))
self.assertRaises(SystemExit, self.conf, ['--help'])
# FIXME(dolphm): Due to bug 1676989, this argument appears as a
# required argument in the CLI help. Instead, the following
# commented-out code should work:
# self.assertIn(' [foo]\n', sys.stdout.getvalue())
self.assertIn(' foo\n', sys.stdout.getvalue())
self.assertIn(' [foo]\n', sys.stdout.getvalue())
self.conf([])
@ -982,11 +976,7 @@ class PositionalTestCase(BaseTestCase):
self.useFixture(fixtures.MonkeyPatch('sys.stdout', moves.StringIO()))
self.assertRaises(SystemExit, self.conf, ['--help'])
# FIXME(dolphm): Due to bug 1676989, this argument appears as a
# required argument in the CLI help. Instead, the following
# commented-out code should work:
# self.assertIn(' [foo-bar]\n', sys.stdout.getvalue())
self.assertIn(' foo-bar\n', sys.stdout.getvalue())
self.assertIn(' [foo-bar]\n', sys.stdout.getvalue())
self.conf(['baz'])
self.assertTrue(hasattr(self.conf, 'foo_bar'))
@ -1002,11 +992,7 @@ class PositionalTestCase(BaseTestCase):
self.useFixture(fixtures.MonkeyPatch('sys.stdout', moves.StringIO()))
self.assertRaises(SystemExit, self.conf, ['--help'])
# FIXME(dolphm): Due to bug 1676989, this argument appears as a
# required argument in the CLI help. Instead, the following
# commented-out code should work:
# self.assertIn(' [foo-bar]\n', sys.stdout.getvalue())
self.assertIn(' foo-bar\n', sys.stdout.getvalue())
self.assertIn(' [foo-bar]\n', sys.stdout.getvalue())
self.conf([])
self.assertTrue(hasattr(self.conf, 'foo_bar'))
@ -1036,12 +1022,12 @@ class PositionalTestCase(BaseTestCase):
self.assertRaises(SystemExit, self.conf, ['--help'])
self.assertIn(' foo-bar\n', sys.stdout.getvalue())
self.assertRaises(cfg.RequiredOptError, self.conf, [])
self.assertRaises(SystemExit, self.conf, [])
def test_missing_required_cli_opt(self):
self.conf.register_cli_opt(
cfg.StrOpt('foo', required=True, positional=True))
self.assertRaises(cfg.RequiredOptError, self.conf, [])
self.assertRaises(SystemExit, self.conf, [])
def test_positional_opts_order(self):
self.conf.register_cli_opts((

12
releasenotes/notes/positional-arguments-are-required-22ddca72e6f523bf.yaml Normal file
View File

@ -0,0 +1,12 @@
---
upgrade:
- |
Positional options are now required by default, to match argparse's default
behavior. To revert this behavior (and maintain optional positional
arguments), you need to explicitly specify ``positional=True,
required=False`` as part of the options definition.
fixes:
- |
On the command line, oslo.config now returns command usage information from
argparse (instead of dumping a backtrace) when required arguments are
missing.