compute: Reorder building of columns for 'server list'

This has no impact on the end result, but it should make fixing issues
introduced by API microversion 2.69 a little easier.

Change-Id: I7d70eac8aa1a6197ed05a49f071e6899ec219c03
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>

Author: stephenfin

openstackclient/compute/v2/server.py

@@ -2204,15 +2204,19 @@ def take_action(self, parsed_args):
         # flavor name is given, map it to ID.
         flavor_id = None
         if parsed_args.flavor:
-            flavor_id = utils.find_resource(compute_client.flavors,
-                                            parsed_args.flavor).id
+            flavor_id = utils.find_resource(
+                compute_client.flavors,
+                parsed_args.flavor,
+            ).id
 
         # Nova only supports list servers searching by image ID. So if a
         # image name is given, map it to ID.
         image_id = None
         if parsed_args.image:
-            image_id = image_client.find_image(parsed_args.image,
-                                               ignore_missing=False).id
+            image_id = image_client.find_image(
+                parsed_args.image,
+                ignore_missing=False,
+            ).id
 
         search_opts = {
             'reservation_id': parsed_args.reservation_id,
@@ -2320,95 +2324,93 @@ def take_action(self, parsed_args):
             try:
                 iso8601.parse_date(search_opts['changes-since'])
             except (TypeError, iso8601.ParseError):
+                msg = _('Invalid changes-since value: %s')
                 raise exceptions.CommandError(
-                    _('Invalid changes-since value: %s') %
-                    search_opts['changes-since']
+                    msg % search_opts['changes-since']
                 )
 
+        columns = (
+            'id',
+            'name',
+            'status',
+        )
+        column_headers = (
+            'ID',
+            'Name',
+            'Status',
+        )
+
         if parsed_args.long:
-            columns = (
-                'ID',
-                'Name',
-                'Status',
+            columns += (
                 'OS-EXT-STS:task_state',
                 'OS-EXT-STS:power_state',
-                'Networks',
-                'Image Name',
-                'Image ID',
-                'Flavor Name',
-                'Flavor ID',
-                'OS-EXT-AZ:availability_zone',
-                'OS-EXT-SRV-ATTR:host',
-                'Metadata',
             )
-            column_headers = (
-                'ID',
-                'Name',
-                'Status',
+            column_headers += (
                 'Task State',
                 'Power State',
-                'Networks',
+            )
+
+        columns += ('networks',)
+        column_headers += ('Networks',)
+
+        if parsed_args.long:
+            columns += (
+                'image_name',
+                'image_id',
+            )
+            column_headers += (
                 'Image Name',
                 'Image ID',
+            )
+        else:
+            if parsed_args.no_name_lookup:
+                columns += ('image_id',)
+            else:
+                columns += ('image_name',)
+            column_headers += ('Image',)
+
+        if parsed_args.long:
+            columns += (
+                'flavor_name',
+                'flavor_id',
+            )
+            column_headers += (
                 'Flavor Name',
                 'Flavor ID',
-                'Availability Zone',
-                'Host',
-                'Properties',
             )
-            mixed_case_fields = [
-                'OS-EXT-STS:task_state',
-                'OS-EXT-STS:power_state',
-                'OS-EXT-AZ:availability_zone',
-                'OS-EXT-SRV-ATTR:host',
-            ]
         else:
             if parsed_args.no_name_lookup:
-                columns = (
-                    'ID',
-                    'Name',
-                    'Status',
-                    'Networks',
-                    'Image ID',
-                    'Flavor ID',
-                )
+                columns += ('flavor_id',)
             else:
-                columns = (
-                    'ID',
-                    'Name',
-                    'Status',
-                    'Networks',
-                    'Image Name',
-                    'Flavor Name',
-                )
-            column_headers = (
-                'ID',
-                'Name',
-                'Status',
-                'Networks',
-                'Image',
-                'Flavor',
+                columns += ('flavor_name',)
+            column_headers += ('Flavor',)
+
+        if parsed_args.long:
+            columns += (
+                'OS-EXT-AZ:availability_zone',
+                'OS-EXT-SRV-ATTR:host',
+                'metadata',
+            )
+            column_headers += (
+                'Availability Zone',
+                'Host',
+                'Properties',
             )
-            mixed_case_fields = []
 
         marker_id = None
 
         # support for additional columns
         if parsed_args.columns:
-            # convert tuple to list to edit them
-            column_headers = list(column_headers)
-            columns = list(columns)
-
             for c in parsed_args.columns:
                 if c in ('Project ID', 'project_id'):
-                    columns.append('tenant_id')
-                    column_headers.append('Project ID')
+                    columns += ('tenant_id',)
+                    column_headers += ('Project ID',)
                 if c in ('User ID', 'user_id'):
-                    columns.append('user_id')
-                    column_headers.append('User ID')
+                    columns += ('user_id',)
+                    column_headers += ('User ID',)
                 if c in ('Created At', 'created_at'):
-                    columns.append('created')
-                    column_headers.append('Created At')
+                    columns += ('created',)
+                    column_headers += ('Created At',)
 
             # convert back to tuple
             column_headers = tuple(column_headers)
@@ -2422,25 +2424,29 @@ def take_action(self, parsed_args):
             if parsed_args.deleted:
                 marker_id = parsed_args.marker
             else:
-                marker_id = utils.find_resource(compute_client.servers,
-                                                parsed_args.marker).id
+                marker_id = utils.find_resource(
+                    compute_client.servers,
+                    parsed_args.marker,
+                ).id
 
-        data = compute_client.servers.list(search_opts=search_opts,
-                                           marker=marker_id,
-                                           limit=parsed_args.limit)
+        data = compute_client.servers.list(
+            search_opts=search_opts,
+            marker=marker_id,
+            limit=parsed_args.limit)
 
         images = {}
         flavors = {}
         if data and not parsed_args.no_name_lookup:
-            # Create a dict that maps image_id to image object.
-            # Needed so that we can display the "Image Name" column.
-            # "Image Name" is not crucial, so we swallow any exceptions.
-            # The 'image' attribute can be an empty string if the server was
-            # booted from a volume.
+            # create a dict that maps image_id to image object, which is used
+            # to display the "Image Name" column. Note that 'image.id' can be
+            # empty for BFV instances and 'image' can be missing entirely if
+            # there are infra failures
             if parsed_args.name_lookup_one_by_one or image_id:
-                for i_id in set(filter(lambda x: x is not None,
-                                       (s.image.get('id') for s in data
-                                        if s.image))):
+                for i_id in set(
+                    s.image['id'] for s in data
+                    if s.image and s.image.get('id')
+                ):
+                    # "Image Name" is not crucial, so we swallow any exceptions
                     try:
                         images[i_id] = image_client.get_image(i_id)
                     except Exception:
@@ -2453,12 +2459,17 @@ def take_action(self, parsed_args):
                 except Exception:
                     pass
 
-            # Create a dict that maps flavor_id to flavor object.
-            # Needed so that we can display the "Flavor Name" column.
-            # "Flavor Name" is not crucial, so we swallow any exceptions.
+            # create a dict that maps flavor_id to flavor object, which is used
+            # to display the "Flavor Name" column. Note that 'flavor.id' is not
+            # present on microversion 2.47 or later and 'flavor' won't be
+            # present if there are infra failures
             if parsed_args.name_lookup_one_by_one or flavor_id:
-                for f_id in set(filter(lambda x: x is not None,
-                                       (s.flavor.get('id') for s in data))):
+                for f_id in set(
+                    s.flavor['id'] for s in data
+                    if s.flavor and s.flavor.get('id')
+                ):
+                    # "Flavor Name" is not crucial, so we swallow any
+                    # exceptions
                     try:
                         flavors[f_id] = compute_client.flavors.get(f_id)
                     except Exception:
@@ -2482,6 +2493,7 @@ def take_action(self, parsed_args):
                 # processing of the image and flavor informations.
                 if not hasattr(s, 'image') or not hasattr(s, 'flavor'):
                     continue
+
             if 'id' in s.image:
                 image = images.get(s.image['id'])
                 if image:
@@ -2494,6 +2506,7 @@ def take_action(self, parsed_args):
                 # able to grep for boot-from-volume servers when using the CLI.
                 s.image_name = IMAGE_STRING_FOR_BFV
                 s.image_id = IMAGE_STRING_FOR_BFV
+
             if 'id' in s.flavor:
                 flavor = flavors.get(s.flavor['id'])
                 if flavor:
@@ -2512,11 +2525,16 @@ def take_action(self, parsed_args):
             (
                 utils.get_item_properties(
                     s, columns,
-                    mixed_case_fields=mixed_case_fields,
+                    mixed_case_fields=(
+                        'OS-EXT-STS:task_state',
+                        'OS-EXT-STS:power_state',
+                        'OS-EXT-AZ:availability_zone',
+                        'OS-EXT-SRV-ATTR:host',
+                    ),
                     formatters={
                         'OS-EXT-STS:power_state': PowerStateColumn,
-                        'Networks': format_columns.DictListColumn,
-                        'Metadata': format_columns.DictColumn,
+                        'networks': format_columns.DictListColumn,
+                        'metadata': format_columns.DictColumn,
                     },
                 ) for s in data
             ),