From b33ee3daf6eebc74fb176b1f2d8018e0e2214377 Mon Sep 17 00:00:00 2001 From: Michael McCune <msm@redhat.com> Date: Thu, 28 Apr 2016 15:15:57 -0400 Subject: [PATCH] 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 --- openstackclient/common/parseractions.py | 5 ++-- openstackclient/network/v2/subnet_pool.py | 3 +++ .../tests/network/v2/test_subnet_pool.py | 25 ++++++++++--------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/openstackclient/common/parseractions.py b/openstackclient/common/parseractions.py index c30058c8e7..77798f9024 100644 --- a/openstackclient/common/parseractions.py +++ b/openstackclient/common/parseractions.py @@ -155,9 +155,8 @@ class NonNegativeAction(argparse.Action): """ def __call__(self, parser, namespace, values, option_string=None): - try: - assert(int(values) >= 0) + if int(values) >= 0: setattr(namespace, self.dest, values) - except Exception: + else: msg = "%s expected a non-negative integer" % (str(option_string)) raise argparse.ArgumentTypeError(msg) diff --git a/openstackclient/network/v2/subnet_pool.py b/openstackclient/network/v2/subnet_pool.py index 435db2e116..f1174dda8c 100644 --- a/openstackclient/network/v2/subnet_pool.py +++ b/openstackclient/network/v2/subnet_pool.py @@ -91,6 +91,7 @@ def _add_prefix_options(parser, for_create=False): parser.add_argument( '--default-prefix-length', metavar='<default-prefix-length>', + type=int, action=parseractions.NonNegativeAction, help=_("Set subnet pool default prefix length") ) @@ -98,11 +99,13 @@ def _add_prefix_options(parser, for_create=False): '--min-prefix-length', metavar='<min-prefix-length>', action=parseractions.NonNegativeAction, + type=int, help=_("Set subnet pool minimum prefix length") ) parser.add_argument( '--max-prefix-length', metavar='<max-prefix-length>', + type=int, action=parseractions.NonNegativeAction, help=_("Set subnet pool maximum prefix length") ) diff --git a/openstackclient/tests/network/v2/test_subnet_pool.py b/openstackclient/tests/network/v2/test_subnet_pool.py index b40390a143..7797e4d053 100644 --- a/openstackclient/tests/network/v2/test_subnet_pool.py +++ b/openstackclient/tests/network/v2/test_subnet_pool.py @@ -153,9 +153,10 @@ class TestCreateSubnetPool(TestSubnetPool): self._subnet_pool.name, ] verifylist = [ - ('default_prefix_length', self._subnet_pool.default_prefixlen), - ('max_prefix_length', self._subnet_pool.max_prefixlen), - ('min_prefix_length', self._subnet_pool.min_prefixlen), + ('default_prefix_length', + int(self._subnet_pool.default_prefixlen)), + ('max_prefix_length', int(self._subnet_pool.max_prefixlen)), + ('min_prefix_length', int(self._subnet_pool.min_prefixlen)), ('name', self._subnet_pool.name), ('prefixes', ['10.0.10.0/24']), ] @@ -164,9 +165,9 @@ class TestCreateSubnetPool(TestSubnetPool): columns, data = (self.cmd.take_action(parsed_args)) self.network.create_subnet_pool.assert_called_once_with(**{ - 'default_prefixlen': self._subnet_pool.default_prefixlen, - 'max_prefixlen': self._subnet_pool.max_prefixlen, - 'min_prefixlen': self._subnet_pool.min_prefixlen, + 'default_prefixlen': int(self._subnet_pool.default_prefixlen), + 'max_prefixlen': int(self._subnet_pool.max_prefixlen), + 'min_prefixlen': int(self._subnet_pool.min_prefixlen), 'prefixes': ['10.0.10.0/24'], 'name': self._subnet_pool.name, }) @@ -397,8 +398,8 @@ class TestSetSubnetPool(TestSubnetPool): ] verifylist = [ ('name', 'noob'), - ('default_prefix_length', '8'), - ('min_prefix_length', '8'), + ('default_prefix_length', 8), + ('min_prefix_length', 8), ('subnet_pool', self._subnet_pool.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -407,8 +408,8 @@ class TestSetSubnetPool(TestSubnetPool): attrs = { 'name': 'noob', - 'default_prefixlen': '8', - 'min_prefixlen': '8', + 'default_prefixlen': 8, + 'min_prefixlen': 8, } self.network.update_subnet_pool.assert_called_once_with( self._subnet_pool, **attrs) @@ -423,7 +424,7 @@ class TestSetSubnetPool(TestSubnetPool): ] verifylist = [ ('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), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -434,7 +435,7 @@ class TestSetSubnetPool(TestSubnetPool): prefixes.extend(self._subnet_pool.prefixes) attrs = { 'prefixes': prefixes, - 'max_prefixlen': '16', + 'max_prefixlen': 16, } self.network.update_subnet_pool.assert_called_once_with( self._subnet_pool, **attrs)