Skip to content

Commit 154e5af

Browse files
allmightyspiffallmightyspiff
authored
Merge pull request #2155 from allmightyspiff/issues2154
Added more error handling around table output
2 parents c5e8d44 + 5e2b9b1 commit 154e5af

File tree

9 files changed

+328
-255
lines changed

9 files changed

+328
-255
lines changed

Diff for: ‎SoftLayer/CLI/block/detail.py

+22-47
Original file line numberDiff line numberDiff line change
@@ -31,77 +31,52 @@ def cli(env, volume_id):
3131
table.add_row(['Username', block_volume['username']])
3232
table.add_row(['Type', storage_type])
3333
table.add_row(['Capacity (GB)', capacity])
34-
table.add_row(['LUN Id', "%s" % block_volume['lunId']])
34+
table.add_row(['LUN Id', block_volume['lunId']])
3535

3636
if block_volume.get('provisionedIops'):
37-
table.add_row(['IOPs', float(block_volume['provisionedIops'])])
37+
table.add_row(['IOPs', block_volume['provisionedIops']])
3838

3939
if block_volume.get('storageTierLevel'):
40-
table.add_row([
41-
'Endurance Tier',
42-
block_volume['storageTierLevel'],
43-
])
44-
45-
table.add_row([
46-
'Data Center',
47-
block_volume['serviceResource']['datacenter']['name'],
48-
])
49-
table.add_row([
50-
'Target IP',
51-
block_volume['serviceResourceBackendIpAddress'],
52-
])
40+
table.add_row(['Endurance Tier', block_volume['storageTierLevel']])
41+
42+
table.add_row(['Data Center', block_volume['serviceResource']['datacenter']['name']])
43+
table.add_row(['Target IP', block_volume['serviceResourceBackendIpAddress']])
5344

5445
if block_volume['snapshotCapacityGb']:
55-
table.add_row([
56-
'Snapshot Capacity (GB)',
57-
block_volume['snapshotCapacityGb'],
58-
])
46+
table.add_row(['Snapshot Capacity (GB)', block_volume['snapshotCapacityGb']])
5947
if 'snapshotSizeBytes' in block_volume['parentVolume']:
60-
table.add_row([
61-
'Snapshot Used (Bytes)',
62-
block_volume['parentVolume']['snapshotSizeBytes'],
63-
])
48+
table.add_row(['Snapshot Used (Bytes)', block_volume['parentVolume']['snapshotSizeBytes']])
6449

65-
table.add_row(['# of Active Transactions', "%i"
66-
% block_volume['activeTransactionCount']])
50+
table.add_row(['# of Active Transactions', block_volume['activeTransactionCount']])
6751

6852
if block_volume['activeTransactions']:
6953
for trans in block_volume['activeTransactions']:
7054
if 'transactionStatus' in trans and 'friendlyName' in trans['transactionStatus']:
7155
table.add_row(['Ongoing Transaction', trans['transactionStatus']['friendlyName']])
7256

73-
table.add_row(['Replicant Count', "%u" % block_volume.get('replicationPartnerCount', 0)])
57+
table.add_row(['Replicant Count', block_volume.get('replicationPartnerCount', 0)])
7458

7559
if block_volume['replicationPartnerCount'] > 0:
7660
# This if/else temporarily handles a bug in which the SL API
7761
# returns a string or object for 'replicationStatus'; it seems that
7862
# the type is string for File volumes and object for Block volumes
7963
if 'message' in block_volume['replicationStatus']:
80-
table.add_row(['Replication Status', "%s"
81-
% block_volume['replicationStatus']['message']])
64+
table.add_row(['Replication Status', block_volume['replicationStatus']['message']])
8265
else:
83-
table.add_row(['Replication Status', "%s"
84-
% block_volume['replicationStatus']])
66+
table.add_row(['Replication Status', block_volume['replicationStatus']])
8567

68+
replicant_table = formatting.Table(['Id', 'Username', 'Target', 'Location', 'Schedule'])
69+
replicant_table.align['Name'] = 'r'
70+
replicant_table.align['Value'] = 'l'
8671
for replicant in block_volume['replicationPartners']:
87-
replicant_table = formatting.Table(['Name',
88-
'Value'])
89-
replicant_table.add_row(['Replicant Id', replicant['id']])
90-
replicant_table.add_row([
91-
'Volume Name',
92-
utils.lookup(replicant, 'username')])
9372
replicant_table.add_row([
94-
'Target IP',
95-
utils.lookup(replicant, 'serviceResourceBackendIpAddress')])
96-
replicant_table.add_row([
97-
'Data Center',
98-
utils.lookup(replicant,
99-
'serviceResource', 'datacenter', 'name')])
100-
replicant_table.add_row([
101-
'Schedule',
102-
utils.lookup(replicant,
103-
'replicationSchedule', 'type', 'keyname')])
104-
table.add_row(['Replicant Volumes', replicant_table])
73+
replicant.get('id'),
74+
utils.lookup(replicant, 'username'),
75+
utils.lookup(replicant, 'serviceResourceBackendIpAddress'),
76+
utils.lookup(replicant, 'serviceResource', 'datacenter', 'name'),
77+
utils.lookup(replicant, 'replicationSchedule', 'type', 'keyname')
78+
])
79+
table.add_row(['Replicant Volumes', replicant_table])
10580

10681
if block_volume.get('originalVolumeSize'):
10782
original_volume_info = formatting.Table(['Property', 'Value'])

Diff for: ‎SoftLayer/CLI/exceptions.py

+12-3
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ def __init__(self, code=0, *args):
1616
self.code = code
1717

1818
def __str__(self):
19-
return "<CLIHalt code=%s msg=%s>" % (self.code,
20-
getattr(self, 'message'))
21-
19+
message = getattr(self, 'message')
20+
return f"<CLIHalt code={self.code} msg={message}>"
2221
__repr__ = __str__
2322

2423

@@ -29,10 +28,20 @@ def __init__(self, msg, *args):
2928
super().__init__(code=2, *args)
3029
self.message = msg
3130

31+
def __str__(self):
32+
message = getattr(self, 'message')
33+
return f"<CLIAbort code={self.code} msg={message}>"
34+
__repr__ = __str__
35+
3236

3337
class ArgumentError(CLIAbort):
3438
"""Halt the execution of the command because of invalid arguments."""
3539

3640
def __init__(self, msg, *args):
3741
super().__init__(msg, *args)
3842
self.message = "Argument Error: %s" % msg
43+
44+
def __str__(self):
45+
message = getattr(self, 'message')
46+
return f"<ArgumentError code={self.code} msg={message}>"
47+
__repr__ = __str__

Diff for: ‎SoftLayer/CLI/file/detail.py

+23-60
Original file line numberDiff line numberDiff line change
@@ -37,54 +37,27 @@ def cli(env, volume_id):
3737
table.add_row(['Type', storage_type])
3838
table.add_row(['Capacity (GB)', get_capacity(file_volume)])
3939

40-
used_space = int(file_volume['bytesUsed']) \
41-
if file_volume['bytesUsed'] else 0
42-
if used_space < (1 << 10):
43-
table.add_row(['Used Space', "%dB" % used_space])
44-
elif used_space < (1 << 20):
45-
table.add_row(['Used Space', "%dKB" % (used_space / (1 << 10))])
46-
elif used_space < (1 << 30):
47-
table.add_row(['Used Space', "%dMB" % (used_space / (1 << 20))])
48-
else:
49-
table.add_row(['Used Space', "%dGB" % (used_space / (1 << 30))])
40+
used_space = formatting.convert_sizes(file_volume.get('bytes_used', 0), "GB", False)
41+
table.add_row(['Used Space', used_space])
5042

5143
if file_volume.get('provisionedIops'):
52-
table.add_row(['IOPs', float(file_volume['provisionedIops'])])
44+
table.add_row(['IOPs', file_volume['provisionedIops']])
5345

5446
if file_volume.get('storageTierLevel'):
55-
table.add_row([
56-
'Endurance Tier',
57-
file_volume['storageTierLevel'],
58-
])
59-
60-
table.add_row([
61-
'Data Center',
62-
file_volume['serviceResource']['datacenter']['name'],
63-
])
64-
table.add_row([
65-
'Target IP',
66-
file_volume['serviceResourceBackendIpAddress'],
67-
])
47+
table.add_row(['Endurance Tier', file_volume['storageTierLevel']])
48+
49+
table.add_row(['Data Center', file_volume['serviceResource']['datacenter']['name']])
50+
table.add_row(['Target IP', file_volume['serviceResourceBackendIpAddress']])
6851

6952
if file_volume['fileNetworkMountAddress']:
70-
table.add_row([
71-
'Mount Address',
72-
file_volume['fileNetworkMountAddress'],
73-
])
53+
table.add_row(['Mount Address', file_volume['fileNetworkMountAddress']])
7454

7555
if file_volume['snapshotCapacityGb']:
76-
table.add_row([
77-
'Snapshot Capacity (GB)',
78-
file_volume['snapshotCapacityGb'],
79-
])
56+
table.add_row(['Snapshot Capacity (GB)', file_volume['snapshotCapacityGb']])
8057
if 'snapshotSizeBytes' in file_volume['parentVolume']:
81-
table.add_row([
82-
'Snapshot Used (Bytes)',
83-
file_volume['parentVolume']['snapshotSizeBytes'],
84-
])
58+
table.add_row(['Snapshot Used (Bytes)', file_volume['parentVolume']['snapshotSizeBytes']])
8559

86-
table.add_row(['# of Active Transactions', "%i"
87-
% file_volume['activeTransactionCount']])
60+
table.add_row(["# of Active Transactions", file_volume['activeTransactionCount']])
8861

8962
if file_volume['activeTransactions']:
9063
for trans in file_volume['activeTransactions']:
@@ -98,32 +71,22 @@ def cli(env, volume_id):
9871
# returns a string or object for 'replicationStatus'; it seems that
9972
# the type is string for File volumes and object for Block volumes
10073
if 'message' in file_volume['replicationStatus']:
101-
table.add_row(['Replication Status', "%s"
102-
% file_volume['replicationStatus']['message']])
74+
table.add_row(['Replication Status', file_volume['replicationStatus']['message']])
10375
else:
104-
table.add_row(['Replication Status', "%s"
105-
% file_volume['replicationStatus']])
76+
table.add_row(['Replication Status', file_volume['replicationStatus']])
10677

107-
replicant_list = []
78+
replicant_table = formatting.Table(['Id', 'Username', 'Target', 'Location', 'Schedule'])
79+
replicant_table.align['Name'] = 'r'
80+
replicant_table.align['Value'] = 'l'
10881
for replicant in file_volume['replicationPartners']:
109-
replicant_table = formatting.Table(['Replicant ID',
110-
replicant['id']])
111-
replicant_table.add_row([
112-
'Volume Name',
113-
utils.lookup(replicant, 'username')])
11482
replicant_table.add_row([
115-
'Target IP',
116-
utils.lookup(replicant, 'serviceResourceBackendIpAddress')])
117-
replicant_table.add_row([
118-
'Data Center',
119-
utils.lookup(replicant,
120-
'serviceResource', 'datacenter', 'name')])
121-
replicant_table.add_row([
122-
'Schedule',
123-
utils.lookup(replicant,
124-
'replicationSchedule', 'type', 'keyname')])
125-
replicant_list.append(replicant_table)
126-
table.add_row(['Replicant Volumes', replicant_list])
83+
replicant.get('id'),
84+
utils.lookup(replicant, 'username'),
85+
utils.lookup(replicant, 'serviceResourceBackendIpAddress'),
86+
utils.lookup(replicant, 'serviceResource', 'datacenter', 'name'),
87+
utils.lookup(replicant, 'replicationSchedule', 'type', 'keyname')
88+
])
89+
table.add_row(['Replicant Volumes', replicant_table])
12790

12891
if file_volume.get('originalVolumeSize'):
12992
original_volume_info = formatting.Table(['Property', 'Value'])

Diff for: ‎SoftLayer/CLI/formatting.py

+10-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import click
1616
from rich import box
17+
from rich.errors import NotRenderableError
1718
from rich.table import Table as rTable
1819

1920
from SoftLayer.CLI import exceptions
@@ -392,7 +393,15 @@ def prettytable(self, fmt='table', theme=None):
392393
table.add_column(col, justify=justify, style=style)
393394

394395
for row in self.rows:
395-
table.add_row(*row)
396+
try:
397+
table.add_row(*row)
398+
# Generally you will see this if one of the columns in the row is a list or dict
399+
except NotRenderableError:
400+
forced_row = []
401+
for i in row:
402+
forced_row.append(str(i))
403+
table.add_row(*forced_row)
404+
396405
return table
397406

398407

Diff for: ‎SoftLayer/fixtures/SoftLayer_Network_Storage.py

+103
Original file line numberDiff line numberDiff line change
@@ -334,4 +334,107 @@
334334
'storageType': {
335335
'keyName': 'ENDURANCE_BLOCK_STORAGE'
336336
}
337+
}
338+
339+
340+
FILE_DETAIL_ISSUE2154 = {
341+
"capacityGb": 150,
342+
"id": 609491933,
343+
"username": "SL02SV1414935_187",
344+
"activeTransactionCount": 0,
345+
"replicationPartnerCount": 1,
346+
"fileNetworkMountAddress": "fsf-natestdal0505-fcb-fz.service.softlayer.com:/SL02SV1414935_187/data01",
347+
"originalVolumeSize": "20",
348+
"provisionedIops": "2000",
349+
"replicationStatus": "FAILOVER_COMPLETED",
350+
"serviceResourceBackendIpAddress": "fsf-natestdal0505-fcb-fz.service.softlayer.com",
351+
"snapshotCapacityGb": "5",
352+
"activeTransactions": [
353+
{
354+
"createDate": "",
355+
"elapsedSeconds": 111763,
356+
"guestId": "",
357+
"hardwareId": "",
358+
"id": "",
359+
"modifyDate": "",
360+
"statusChangeDate": "",
361+
"transactionGroup": {
362+
"name": "Volume Modification"
363+
},
364+
"transactionStatus": {
365+
"friendlyName": "In Progress"
366+
}
367+
}
368+
],
369+
"parentVolume": {
370+
"accountId": 1414935,
371+
"capacityGb": 120,
372+
"createDate": "2024-05-16T02:28:02-05:00",
373+
"guestId": "",
374+
"hardwareId": "",
375+
"hostId": "",
376+
"id": 609491967,
377+
"nasType": "SNAPSHOT",
378+
"notes": "vol_duplicate_snapshot_2024-05-16_0228",
379+
"serviceProviderId": 1,
380+
"storageTypeId": "16",
381+
"upgradableFlag": True,
382+
"username": "SL02SV1414935_187",
383+
"serviceResourceBackendIpAddress": "fsf-natestdal0505-fcb-fz.service.softlayer.com",
384+
"serviceResourceName": "Storage Type 02 Aggregate natestdal0505-fc-d",
385+
"snapshotSizeBytes": "0"
386+
},
387+
"replicationPartners": [
388+
{
389+
"id": 609491945,
390+
"username": "SL02SV1414935_187_REP_1",
391+
"serviceResourceBackendIpAddress": "fsf-natestdal0505-ffb-fz.service.softlayer.com",
392+
"replicationSchedule": {
393+
"active": 1,
394+
"createDate": "2024-05-16T01:20:19-05:00",
395+
"id": 666339,
396+
"modifyDate": "",
397+
"name": "SL02SV1414935_187_HOURLY_REP",
398+
"partnershipId": "",
399+
"typeId": 32,
400+
"volumeId": 609491933,
401+
"type": {
402+
"keyname": "REPLICATION_HOURLY"
403+
}
404+
},
405+
"serviceResource": {
406+
"backendIpAddress": "fsf-natestdal0505-ffb-fz.service.softlayer.com",
407+
"id": 57365,
408+
"name": "Storage Type 02 Aggregate natestdal0505-ff-d",
409+
"datacenter": {
410+
"name": "dal10"
411+
},
412+
"type": {
413+
"type": "NETAPP_STOR_AGGR"
414+
}
415+
}
416+
}
417+
],
418+
"serviceResource": {
419+
"backendIpAddress": "fsf-natestdal0505-fcb-fz.service.softlayer.com",
420+
"id": 52292,
421+
"name": "Storage Type 02 Aggregate natestdal0505-fc-d",
422+
"attributes": [
423+
{
424+
"value": "2",
425+
"attributeType": {
426+
"keyname": "STAAS_VERSION"
427+
}
428+
}
429+
],
430+
"datacenter": {
431+
"name": "lon02"
432+
},
433+
"type": {
434+
"type": "NETAPP_STOR_AGGR"
435+
}
436+
},
437+
"storageType": {
438+
"keyName": "PERFORMANCE_FILE_STORAGE"
337439
}
440+
}

Diff for: ‎SoftLayer/testing/__init__.py

+1
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ def setUp(self): # NOQA
111111
self.env = environment.Environment()
112112
self.env.client = self.client
113113
self.set_up()
114+
self.maxDiff = None
114115

115116
def tearDown(self): # NOQA
116117
super().tearDown()

0 commit comments

Comments
 (0)