remove assert in favor an if/else

the assert usage in the NonNegativeAction has the potential to allow
unexpected behavior when the python is byte-compiled with optimization
turned on.

Changes
* remove assert in favor of if/else in NonNegativeAction class
* add type specifier to parser arguments for non-negative actions
* correct tests for new int based values

Change-Id: I093e7440b8beff4f179e2c4ed81daff82704c40e
Closes-Bug: #1576375
This commit is contained in:
Michael McCune 2016-04-28 15:15:57 -04:00
parent 9d7ccd9385
commit b33ee3daf6
3 changed files with 18 additions and 15 deletions
openstackclient

@ -155,9 +155,8 @@ class NonNegativeAction(argparse.Action):
""" """
def __call__(self, parser, namespace, values, option_string=None): def __call__(self, parser, namespace, values, option_string=None):
try: if int(values) >= 0:
assert(int(values) >= 0)
setattr(namespace, self.dest, values) setattr(namespace, self.dest, values)
except Exception: else:
msg = "%s expected a non-negative integer" % (str(option_string)) msg = "%s expected a non-negative integer" % (str(option_string))
raise argparse.ArgumentTypeError(msg) raise argparse.ArgumentTypeError(msg)

@ -91,6 +91,7 @@ def _add_prefix_options(parser, for_create=False):
parser.add_argument( parser.add_argument(
'--default-prefix-length', '--default-prefix-length',
metavar='<default-prefix-length>', metavar='<default-prefix-length>',
type=int,
action=parseractions.NonNegativeAction, action=parseractions.NonNegativeAction,
help=_("Set subnet pool default prefix length") help=_("Set subnet pool default prefix length")
) )
@ -98,11 +99,13 @@ def _add_prefix_options(parser, for_create=False):
'--min-prefix-length', '--min-prefix-length',
metavar='<min-prefix-length>', metavar='<min-prefix-length>',
action=parseractions.NonNegativeAction, action=parseractions.NonNegativeAction,
type=int,
help=_("Set subnet pool minimum prefix length") help=_("Set subnet pool minimum prefix length")
) )
parser.add_argument( parser.add_argument(
'--max-prefix-length', '--max-prefix-length',
metavar='<max-prefix-length>', metavar='<max-prefix-length>',
type=int,
action=parseractions.NonNegativeAction, action=parseractions.NonNegativeAction,
help=_("Set subnet pool maximum prefix length") help=_("Set subnet pool maximum prefix length")
) )

@ -153,9 +153,10 @@ class TestCreateSubnetPool(TestSubnetPool):
self._subnet_pool.name, self._subnet_pool.name,
] ]
verifylist = [ verifylist = [
('default_prefix_length', self._subnet_pool.default_prefixlen), ('default_prefix_length',
('max_prefix_length', self._subnet_pool.max_prefixlen), int(self._subnet_pool.default_prefixlen)),
('min_prefix_length', self._subnet_pool.min_prefixlen), ('max_prefix_length', int(self._subnet_pool.max_prefixlen)),
('min_prefix_length', int(self._subnet_pool.min_prefixlen)),
('name', self._subnet_pool.name), ('name', self._subnet_pool.name),
('prefixes', ['10.0.10.0/24']), ('prefixes', ['10.0.10.0/24']),
] ]
@ -164,9 +165,9 @@ class TestCreateSubnetPool(TestSubnetPool):
columns, data = (self.cmd.take_action(parsed_args)) columns, data = (self.cmd.take_action(parsed_args))
self.network.create_subnet_pool.assert_called_once_with(**{ self.network.create_subnet_pool.assert_called_once_with(**{
'default_prefixlen': self._subnet_pool.default_prefixlen, 'default_prefixlen': int(self._subnet_pool.default_prefixlen),
'max_prefixlen': self._subnet_pool.max_prefixlen, 'max_prefixlen': int(self._subnet_pool.max_prefixlen),
'min_prefixlen': self._subnet_pool.min_prefixlen, 'min_prefixlen': int(self._subnet_pool.min_prefixlen),
'prefixes': ['10.0.10.0/24'], 'prefixes': ['10.0.10.0/24'],
'name': self._subnet_pool.name, 'name': self._subnet_pool.name,
}) })
@ -397,8 +398,8 @@ class TestSetSubnetPool(TestSubnetPool):
] ]
verifylist = [ verifylist = [
('name', 'noob'), ('name', 'noob'),
('default_prefix_length', '8'), ('default_prefix_length', 8),
('min_prefix_length', '8'), ('min_prefix_length', 8),
('subnet_pool', self._subnet_pool.name), ('subnet_pool', self._subnet_pool.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -407,8 +408,8 @@ class TestSetSubnetPool(TestSubnetPool):
attrs = { attrs = {
'name': 'noob', 'name': 'noob',
'default_prefixlen': '8', 'default_prefixlen': 8,
'min_prefixlen': '8', 'min_prefixlen': 8,
} }
self.network.update_subnet_pool.assert_called_once_with( self.network.update_subnet_pool.assert_called_once_with(
self._subnet_pool, **attrs) self._subnet_pool, **attrs)
@ -423,7 +424,7 @@ class TestSetSubnetPool(TestSubnetPool):
] ]
verifylist = [ verifylist = [
('prefixes', ['10.0.1.0/24', '10.0.2.0/24']), ('prefixes', ['10.0.1.0/24', '10.0.2.0/24']),
('max_prefix_length', '16'), ('max_prefix_length', 16),
('subnet_pool', self._subnet_pool.name), ('subnet_pool', self._subnet_pool.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -434,7 +435,7 @@ class TestSetSubnetPool(TestSubnetPool):
prefixes.extend(self._subnet_pool.prefixes) prefixes.extend(self._subnet_pool.prefixes)
attrs = { attrs = {
'prefixes': prefixes, 'prefixes': prefixes,
'max_prefixlen': '16', 'max_prefixlen': 16,
} }
self.network.update_subnet_pool.assert_called_once_with( self.network.update_subnet_pool.assert_called_once_with(
self._subnet_pool, **attrs) self._subnet_pool, **attrs)