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

Author: elmiko

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)

openstackclient/network/v2/subnet_pool.py

@@ -91,18 +91,21 @@ 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")
     )
     parser.add_argument(
         '--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")
     )

openstackclient/tests/network/v2/test_subnet_pool.py

@@ -153,9 +153,10 @@ def test_create_prefixlen_options(self):
             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 @@ def test_create_prefixlen_options(self):
         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 @@ def test_set_this(self):
         ]
         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 @@ def test_set_this(self):
 
         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 @@ def test_set_that(self):
         ]
         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 @@ def test_set_that(self):
         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)