Fixed #2154 added more error handling around table output

Author: allmightyspiff

SoftLayer/CLI/block/detail.py

@@ -83,25 +83,21 @@ def cli(env, volume_id):
             table.add_row(['Replication Status', "%s"
                            % block_volume['replicationStatus']])
 
+        replicant_table = formatting.KeyValueTable(['Name', 'Value'])
+        replicant_table.align['Name'] = 'r'
+        replicant_table.align['Value'] = 'l'
         for replicant in block_volume['replicationPartners']:
-            replicant_table = formatting.Table(['Name',
-                                                'Value'])
-            replicant_table.add_row(['Replicant Id', replicant['id']])
             replicant_table.add_row([
-                'Volume Name',
-                utils.lookup(replicant, 'username')])
+                'Replicant Id', replicant['id']])
             replicant_table.add_row([
-                'Target IP',
-                utils.lookup(replicant, 'serviceResourceBackendIpAddress')])
+                'Volume Name', utils.lookup(replicant, 'username')])
             replicant_table.add_row([
-                'Data Center',
-                utils.lookup(replicant,
-                             'serviceResource', 'datacenter', 'name')])
+                'Target IP',  utils.lookup(replicant, 'serviceResourceBackendIpAddress')])
             replicant_table.add_row([
-                'Schedule',
-                utils.lookup(replicant,
-                             'replicationSchedule', 'type', 'keyname')])
-            table.add_row(['Replicant Volumes', replicant_table])
+                'Data Center', utils.lookup(replicant, 'serviceResource', 'datacenter', 'name')])
+            replicant_table.add_row([
+                'Schedule', utils.lookup(replicant, 'replicationSchedule', 'type', 'keyname')])
+        table.add_row(['Replicant Volumes', replicant_table])
 
     if block_volume.get('originalVolumeSize'):
         original_volume_info = formatting.Table(['Property', 'Value'])

SoftLayer/CLI/file/detail.py

@@ -37,54 +37,27 @@ def cli(env, volume_id):
     table.add_row(['Type', storage_type])
     table.add_row(['Capacity (GB)', get_capacity(file_volume)])
 
-    used_space = int(file_volume['bytesUsed']) \
-        if file_volume['bytesUsed'] else 0
-    if used_space < (1 << 10):
-        table.add_row(['Used Space', "%dB" % used_space])
-    elif used_space < (1 << 20):
-        table.add_row(['Used Space', "%dKB" % (used_space / (1 << 10))])
-    elif used_space < (1 << 30):
-        table.add_row(['Used Space', "%dMB" % (used_space / (1 << 20))])
-    else:
-        table.add_row(['Used Space', "%dGB" % (used_space / (1 << 30))])
+    used_space = formatting.convert_sizes(file_volume.get('bytes_used', 0), "GB", False)
+    table.add_row(['Used Space', used_space])
 
     if file_volume.get('provisionedIops'):
         table.add_row(['IOPs', float(file_volume['provisionedIops'])])
 
     if file_volume.get('storageTierLevel'):
-        table.add_row([
-            'Endurance Tier',
-            file_volume['storageTierLevel'],
-        ])
-
-    table.add_row([
-        'Data Center',
-        file_volume['serviceResource']['datacenter']['name'],
-    ])
-    table.add_row([
-        'Target IP',
-        file_volume['serviceResourceBackendIpAddress'],
-    ])
+        table.add_row(['Endurance Tier', file_volume['storageTierLevel']])
+
+    table.add_row(['Data Center', file_volume['serviceResource']['datacenter']['name']])
+    table.add_row(['Target IP', file_volume['serviceResourceBackendIpAddress']])
 
     if file_volume['fileNetworkMountAddress']:
-        table.add_row([
-            'Mount Address',
-            file_volume['fileNetworkMountAddress'],
-        ])
+        table.add_row(['Mount Address', file_volume['fileNetworkMountAddress']])
 
     if file_volume['snapshotCapacityGb']:
-        table.add_row([
-            'Snapshot Capacity (GB)',
-            file_volume['snapshotCapacityGb'],
-        ])
+        table.add_row(['Snapshot Capacity (GB)', file_volume['snapshotCapacityGb']])
         if 'snapshotSizeBytes' in file_volume['parentVolume']:
-            table.add_row([
-                'Snapshot Used (Bytes)',
-                file_volume['parentVolume']['snapshotSizeBytes'],
-            ])
+            table.add_row(['Snapshot Used (Bytes)', file_volume['parentVolume']['snapshotSizeBytes']])
 
-    table.add_row(['# of Active Transactions', "%i"
-                   % file_volume['activeTransactionCount']])
+    table.add_row(["# of Active Transactions", file_volume['activeTransactionCount']])
 
     if file_volume['activeTransactions']:
         for trans in file_volume['activeTransactions']:
@@ -98,32 +71,25 @@ def cli(env, volume_id):
         # returns a string or object for 'replicationStatus'; it seems that
         # the type is string for File volumes and object for Block volumes
         if 'message' in file_volume['replicationStatus']:
-            table.add_row(['Replication Status', "%s"
-                           % file_volume['replicationStatus']['message']])
+            table.add_row(['Replication Status', file_volume['replicationStatus']['message']])
         else:
-            table.add_row(['Replication Status', "%s"
-                           % file_volume['replicationStatus']])
+            table.add_row(['Replication Status', file_volume['replicationStatus']])
 
-        replicant_list = []
+        replicant_table = formatting.KeyValueTable(['Name', 'Value'])
+        replicant_table.align['Name'] = 'r'
+        replicant_table.align['Value'] = 'l'
         for replicant in file_volume['replicationPartners']:
-            replicant_table = formatting.Table(['Replicant ID',
-                                                replicant['id']])
             replicant_table.add_row([
-                'Volume Name',
-                utils.lookup(replicant, 'username')])
+                'Volume ID', replicant.get('id')])
+            replicant_table.add_row([
+                'Volume Name', utils.lookup(replicant, 'username')])
             replicant_table.add_row([
-                'Target IP',
-                utils.lookup(replicant, 'serviceResourceBackendIpAddress')])
+                'Target IP', utils.lookup(replicant, 'serviceResourceBackendIpAddress')])
             replicant_table.add_row([
-                'Data Center',
-                utils.lookup(replicant,
-                             'serviceResource', 'datacenter', 'name')])
+                'Data Center', utils.lookup(replicant, 'serviceResource', 'datacenter', 'name')])
             replicant_table.add_row([
-                'Schedule',
-                utils.lookup(replicant,
-                             'replicationSchedule', 'type', 'keyname')])
-            replicant_list.append(replicant_table)
-        table.add_row(['Replicant Volumes', replicant_list])
+                'Schedule', utils.lookup(replicant, 'replicationSchedule', 'type', 'keyname')])
+        table.add_row(['Replicant Volumes', replicant_table])
 
     if file_volume.get('originalVolumeSize'):
         original_volume_info = formatting.Table(['Property', 'Value'])

SoftLayer/CLI/formatting.py

@@ -14,6 +14,7 @@
 
 import click
 from rich import box
+from rich.errors import NotRenderableError
 from rich.table import Table as rTable
 
 from SoftLayer.CLI import exceptions
@@ -392,7 +393,15 @@ def prettytable(self, fmt='table', theme=None):
             table.add_column(col, justify=justify, style=style)
 
         for row in self.rows:
-            table.add_row(*row)
+            try:
+                table.add_row(*row)
+            # Generally you will see this if one of the columns in the row is a list or dict
+            except NotRenderableError:
+                forced_row = []
+                for i in row:
+                    forced_row.append(str(i))
+                table.add_row(*forced_row)
+
         return table
 
 

SoftLayer/fixtures/SoftLayer_Network_Storage.py

@@ -334,4 +334,107 @@
     'storageType': {
         'keyName': 'ENDURANCE_BLOCK_STORAGE'
     }
+}
+
+
+FILE_DETAIL_ISSUE2154 = {
+    "capacityGb": 150,
+    "id": 609491933,
+    "username": "SL02SV1414935_187",
+    "activeTransactionCount": 0,
+    "replicationPartnerCount": 1,
+    "fileNetworkMountAddress": "fsf-natestdal0505-fcb-fz.service.softlayer.com:/SL02SV1414935_187/data01",
+    "originalVolumeSize": "20",
+    "provisionedIops": "2000",
+    "replicationStatus": "FAILOVER_COMPLETED",
+    "serviceResourceBackendIpAddress": "fsf-natestdal0505-fcb-fz.service.softlayer.com",
+    "snapshotCapacityGb": "5",
+    "activeTransactions": [
+        {
+            "createDate": "",
+            "elapsedSeconds": 111763,
+            "guestId": "",
+            "hardwareId": "",
+            "id": "",
+            "modifyDate": "",
+            "statusChangeDate": "",
+            "transactionGroup": {
+                "name": "Volume Modification"
+            },
+            "transactionStatus": {
+                "friendlyName": "In Progress"
+            }
+        }
+    ],
+    "parentVolume": {
+        "accountId": 1414935,
+        "capacityGb": 120,
+        "createDate": "2024-05-16T02:28:02-05:00",
+        "guestId": "",
+        "hardwareId": "",
+        "hostId": "",
+        "id": 609491967,
+        "nasType": "SNAPSHOT",
+        "notes": "vol_duplicate_snapshot_2024-05-16_0228",
+        "serviceProviderId": 1,
+        "storageTypeId": "16",
+        "upgradableFlag": True,
+        "username": "SL02SV1414935_187",
+        "serviceResourceBackendIpAddress": "fsf-natestdal0505-fcb-fz.service.softlayer.com",
+        "serviceResourceName": "Storage Type 02 Aggregate natestdal0505-fc-d",
+        "snapshotSizeBytes": "0"
+    },
+    "replicationPartners": [
+        {
+            "id": 609491945,
+            "username": "SL02SV1414935_187_REP_1",
+            "serviceResourceBackendIpAddress": "fsf-natestdal0505-ffb-fz.service.softlayer.com",
+            "replicationSchedule": {
+                "active": 1,
+                "createDate": "2024-05-16T01:20:19-05:00",
+                "id": 666339,
+                "modifyDate": "",
+                "name": "SL02SV1414935_187_HOURLY_REP",
+                "partnershipId": "",
+                "typeId": 32,
+                "volumeId": 609491933,
+                "type": {
+                    "keyname": "REPLICATION_HOURLY"
+                }
+            },
+            "serviceResource": {
+                "backendIpAddress": "fsf-natestdal0505-ffb-fz.service.softlayer.com",
+                "id": 57365,
+                "name": "Storage Type 02 Aggregate natestdal0505-ff-d",
+                "datacenter": {
+                    "name": "dal10"
+                },
+                "type": {
+                    "type": "NETAPP_STOR_AGGR"
+                }
+            }
+        }
+    ],
+    "serviceResource": {
+        "backendIpAddress": "fsf-natestdal0505-fcb-fz.service.softlayer.com",
+        "id": 52292,
+        "name": "Storage Type 02 Aggregate natestdal0505-fc-d",
+        "attributes": [
+            {
+                "value": "2",
+                "attributeType": {
+                    "keyname": "STAAS_VERSION"
+                }
+            }
+        ],
+        "datacenter": {
+            "name": "lon02"
+        },
+        "type": {
+            "type": "NETAPP_STOR_AGGR"
+        }
+    },
+    "storageType": {
+        "keyName": "PERFORMANCE_FILE_STORAGE"
     }
+}

tests/CLI/formatting_table_tests.py

@@ -4,6 +4,8 @@
 
     :license: MIT, see LICENSE for more details.
 """
+from rich.console import Console
+
 
 from SoftLayer.CLI import exceptions
 from SoftLayer.CLI import formatting
@@ -21,6 +23,48 @@ def test_boolean_table(self):
         table.add_row(["entry1"])
         self.assertTrue(table)
 
+    def test_key_value_table(self):
+        expected = """┌───────────┬─────────────────────────┐
+│    Key    │          Value          │
+├───────────┼─────────────────────────┤
+│   First   │           One           │
+│ Sub Table │ ┌─────────┬───────────┐ │
+│           │ │ Sub Key │ Sub Value │ │
+│           │ ├─────────┼───────────┤ │
+│           │ │ Second  │    Two    │ │
+│           │ └─────────┴───────────┘ │
+└───────────┴─────────────────────────┘
+"""
+        table = formatting.KeyValueTable(["Key", "Value"])
+        table.add_row(["First", "One"])
+        sub_table = formatting.KeyValueTable(["Sub Key", "Sub Value"])
+        sub_table.add_row(["Second", "Two"])
+        table.add_row(["Sub Table", sub_table])
+        console = Console()
+
+        with console.capture() as capture:
+            to_print = formatting.format_output(table)
+            console.print(to_print)
+        result = capture.get()
+        self.assertEqual(expected, result)
+
+    def test_unrenderable_recovery_table(self):
+        expected = """│ Sub Table │ [<rich.table.Table object at"""
+        table = formatting.KeyValueTable(["Key", "Value"])
+        table.add_row(["First", "One"])
+        sub_table = formatting.KeyValueTable(["Sub Key", "Sub Value"])
+        sub_table.add_row(["Second", "Two"])
+        # You can't have a list of tables, that isn't handled very well.
+        listable = [sub_table]
+        table.add_row(["Sub Table", listable])
+        console = Console()
+        with console.capture() as capture:
+            to_print = formatting.format_output(table)
+            console.print(to_print)
+        result = capture.get()
+        print(result)
+        self.assertIn(expected, result)
+
 
 class IterToTableTests(testing.TestCase):
 

tests/CLI/modules/file_tests.py

@@ -5,6 +5,7 @@
     :license: MIT, see LICENSE for more details.
 """
 from SoftLayer.CLI import exceptions
+from SoftLayer.fixtures import SoftLayer_Network_Storage
 from SoftLayer import SoftLayerError
 from SoftLayer import testing
 
@@ -220,6 +221,13 @@ def test_volume_detail_name_identifier(self):
         self.assert_called_with('SoftLayer_Network_Storage', 'getObject', identifier=1)
         self.assert_no_fail(result)
 
+    def test_volume_detail_issues2154(self):
+        lun_mock = self.set_mock('SoftLayer_Network_Storage', 'getObject')
+        lun_mock.return_value = SoftLayer_Network_Storage.FILE_DETAIL_ISSUE2154
+        result = self.run_command(['--format=table', 'file', 'volume-detail', '1234'])
+        self.assert_no_fail(result)
+        self.assertIn("SL02SV1414935_187", result.output)
+
     def test_volume_order_performance_iops_not_given(self):
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=performance', '--size=20',