Avoid formatting *_extra_specs unless format is table

Currently some share type commands display multiple properties in
separate lines. This format is consistently used even when machine-
oriented output format like csv is used and makes parsing difficult.

With this change these properties are no longer formatted and dumped
in a raw json/dict format, unless the table format, which intends
human-readability, is used.

Change-Id: I91f3d6c38fc915e7f2ed2219b88623ec50d8c76c

manilaclient/osc/v2/share_types.py

@@ -36,7 +36,7 @@ ATTRIBUTES = [
 ]
 
 
-def format_share_type(share_type):
+def format_share_type(share_type, formatter='table'):
     # share_type_access:is_public (true/false) --> visibility (public/private)
     is_public = 'share_type_access:is_public'
     visibility = 'public' if share_type._info.get(is_public) else 'private'
@@ -48,15 +48,25 @@ def format_share_type(share_type):
     for key in share_type.required_extra_specs.keys():
         optional_extra_specs.pop(key, None)
 
-    share_type._info.update(
-        {
-            'visibility': visibility,
-            'required_extra_specs': utils.format_properties(
-                share_type.required_extra_specs),
-            'optional_extra_specs': utils.format_properties(
-                optional_extra_specs),
-        }
-    )
+    if formatter == 'table':
+        share_type._info.update(
+            {
+                'visibility': visibility,
+                'required_extra_specs': utils.format_properties(
+                    share_type.required_extra_specs),
+                'optional_extra_specs': utils.format_properties(
+                    optional_extra_specs),
+            }
+        )
+    else:
+        share_type._info.update(
+            {
+                'visibility': visibility,
+                'required_extra_specs': share_type.required_extra_specs,
+                'optional_extra_specs': optional_extra_specs,
+            }
+        )
+
     return share_type
 
 
@@ -187,7 +197,7 @@ class CreateShareType(command.ShowOne):
         kwargs['extra_specs'] = extra_specs
 
         share_type = share_client.share_types.create(**kwargs)
-        formatted_type = format_share_type(share_type)
+        formatted_type = format_share_type(share_type, parsed_args.formatter)
 
         return (ATTRIBUTES, oscutils.get_dict_properties(
                 formatted_type._info, ATTRIBUTES))
@@ -405,7 +415,8 @@ class ListShareType(command.Lister):
 
         formatted_types = []
         for share_type in share_types:
-            formatted_types.append(format_share_type(share_type))
+            formatted_types.append(format_share_type(share_type,
+                                                     parsed_args.formatter))
 
         values = (oscutils.get_dict_properties(
             s._info, ATTRIBUTES) for s in formatted_types)
@@ -435,7 +446,7 @@ class ShowShareType(command.ShowOne):
             share_client.share_types,
             parsed_args.share_type)
 
-        formatted_type = format_share_type(share_type)
+        formatted_type = format_share_type(share_type, parsed_args.formatter)
 
         return (ATTRIBUTES, oscutils.get_dict_properties(
             formatted_type._info, ATTRIBUTES))

manilaclient/tests/unit/osc/v2/test_share_type.py

@@ -71,6 +71,20 @@ class TestShareTypeCreate(TestShareType):
             self.new_share_type.description,
         ]
 
+        self.raw_data = [
+            self.new_share_type.id,
+            self.new_share_type.name,
+            'public',
+            self.new_share_type.is_default,
+            {'driver_handles_share_servers': True},
+            {'replication_type': 'readable',
+             'mount_snapshot_support': False,
+             'revert_to_snapshot_support': False,
+             'create_share_from_snapshot_support': True,
+             'snapshot_support': True},
+            self.new_share_type.description,
+        ]
+
     def test_share_type_create_required_args(self):
         """Verifies required arguments."""
 
@@ -97,6 +111,33 @@ class TestShareTypeCreate(TestShareType):
         self.assertCountEqual(COLUMNS, columns)
         self.assertCountEqual(self.data, data)
 
+    def test_share_type_create_json_fomrat(self):
+        """Verifies --format json."""
+
+        arglist = [
+            self.new_share_type.name,
+            'True',
+            '-f', 'json'
+        ]
+        verifylist = [
+            ('name', self.new_share_type.name),
+            ('spec_driver_handles_share_servers', 'True')
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.shares_mock.create.assert_called_with(
+            extra_specs={},
+            is_public=True,
+            name=self.new_share_type.name,
+            spec_driver_handles_share_servers=True
+        )
+
+        self.assertCountEqual(COLUMNS, columns)
+        self.assertCountEqual(self.raw_data, data)
+
     def test_share_type_create_missing_required_arg(self):
         """Verifies missing required arguments."""
 
@@ -554,6 +595,20 @@ class TestShareTypeShow(TestShareType):
             self.share_type.description,
         ]
 
+        self.raw_data = [
+            self.share_type.id,
+            self.share_type.name,
+            'public',
+            self.share_type.is_default,
+            {'driver_handles_share_servers': True},
+            {'replication_type': 'readable',
+             'mount_snapshot_support': False,
+             'revert_to_snapshot_support': False,
+             'create_share_from_snapshot_support': True,
+             'snapshot_support': True},
+            self.share_type.description,
+        ]
+
     def test_share_type_show(self):
         arglist = [
             self.share_type.id
@@ -568,3 +623,19 @@ class TestShareTypeShow(TestShareType):
 
         self.assertCountEqual(COLUMNS, columns)
         self.assertCountEqual(self.data, data)
+
+    def test_share_type_show_json_format(self):
+        arglist = [
+            self.share_type.id,
+            '-f', 'json',
+        ]
+        verifylist = [
+            ("share_type", self.share_type.id)
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+        self.shares_mock.get.assert_called_with(self.share_type.id)
+
+        self.assertCountEqual(COLUMNS, columns)
+        self.assertCountEqual(self.raw_data, data)

releasenotes/notes/share_type-skip-format-119595e62900e571.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    The following openstack subcommands no longer format
+    the required_extra_specs field and the optional_extra_specs field unless
+    the default ``table`` format is used.
+
+    - ``openstack share type create``
+    - ``openstack share type list``
+    - ``openstack share type show``