volume: Make better use of argparse

Change-Id: Ifeab60aaf18a9163465d4968c53e3ed66dad769b
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

Author: stephenfin

openstackclient/tests/unit/volume/v2/test_volume_type.py

@@ -80,8 +80,7 @@ def test_type_create_public(self):
         ]
         verifylist = [
             ("description", self.new_volume_type.description),
-            ("public", True),
-            ("private", False),
+            ("is_public", True),
             ("name", self.new_volume_type.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -107,8 +106,7 @@ def test_type_create_private(self):
         ]
         verifylist = [
             ("description", self.new_volume_type.description),
-            ("public", False),
-            ("private", True),
+            ("is_public", False),
             ("project", self.project.id),
             ("name", self.new_volume_type.name),
         ]
@@ -325,8 +323,7 @@ def test_type_list_without_options(self):
         arglist = []
         verifylist = [
             ("long", False),
-            ("private", False),
-            ("public", False),
+            ("is_public", None),
             ("default", False),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -343,8 +340,7 @@ def test_type_list_with_options(self):
         ]
         verifylist = [
             ("long", True),
-            ("private", False),
-            ("public", True),
+            ("is_public", True),
             ("default", False),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -360,8 +356,7 @@ def test_type_list_with_private_option(self):
         ]
         verifylist = [
             ("long", False),
-            ("private", True),
-            ("public", False),
+            ("is_public", False),
             ("default", False),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -378,8 +373,7 @@ def test_type_list_with_default_option(self):
         verifylist = [
             ("encryption_type", False),
             ("long", False),
-            ("private", False),
-            ("public", False),
+            ("is_public", None),
             ("default", True),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)

openstackclient/volume/v2/volume_type.py

@@ -124,13 +124,15 @@ def get_parser(self, prog_name):
         public_group.add_argument(
             "--public",
             action="store_true",
-            default=False,
+            dest="is_public",
+            default=None,
             help=_("Volume type is accessible to the public"),
         )
         public_group.add_argument(
             "--private",
-            action="store_true",
-            default=False,
+            action="store_false",
+            dest="is_public",
+            default=None,
             help=_("Volume type is not accessible to the public"),
         )
         parser.add_argument(
@@ -201,15 +203,13 @@ def take_action(self, parsed_args):
         identity_client = self.app.client_manager.identity
         volume_client = self.app.client_manager.volume
 
-        if parsed_args.project and not parsed_args.private:
+        if parsed_args.project and parsed_args.is_public is not False:
             msg = _("--project is only allowed with --private")
             raise exceptions.CommandError(msg)
 
         kwargs = {}
-        if parsed_args.public:
-            kwargs['is_public'] = True
-        if parsed_args.private:
-            kwargs['is_public'] = False
+        if parsed_args.is_public is not None:
+            kwargs['is_public'] = parsed_args.is_public
 
         volume_type = volume_client.volume_types.create(
             parsed_args.name, description=parsed_args.description, **kwargs
@@ -329,11 +329,15 @@ def get_parser(self, prog_name):
         public_group.add_argument(
             "--public",
             action="store_true",
+            dest="is_public",
+            default=None,
             help=_("List only public types"),
         )
         public_group.add_argument(
             "--private",
-            action="store_true",
+            action="store_false",
+            dest="is_public",
+            default=None,
             help=_("List only private types (admin only)"),
         )
         parser.add_argument(
@@ -348,8 +352,15 @@ def get_parser(self, prog_name):
 
     def take_action(self, parsed_args):
         volume_client = self.app.client_manager.volume
+
         if parsed_args.long:
-            columns = ['ID', 'Name', 'Is Public', 'Description', 'Extra Specs']
+            columns = [
+                'ID',
+                'Name',
+                'Is Public',
+                'Description',
+                'Extra Specs',
+            ]
             column_headers = [
                 'ID',
                 'Name',
@@ -360,15 +371,13 @@ def take_action(self, parsed_args):
         else:
             columns = ['ID', 'Name', 'Is Public']
             column_headers = ['ID', 'Name', 'Is Public']
+
         if parsed_args.default:
             data = [volume_client.volume_types.default()]
         else:
-            is_public = None
-            if parsed_args.public:
-                is_public = True
-            if parsed_args.private:
-                is_public = False
-            data = volume_client.volume_types.list(is_public=is_public)
+            data = volume_client.volume_types.list(
+                is_public=parsed_args.is_public
+            )
 
         formatters = {'Extra Specs': format_columns.DictColumn}