Skip to content

Commit c13d221

Browse files
sudorandomsudorandom
committed
Fixes Code Smells
Removes assignment of unused variables Removes static/class methods from CLIRunnables Avoids directly setting built-in copyright variable
1 parent 5e499ef commit c13d221

40 files changed

Lines changed: 536 additions & 651 deletions

‎SoftLayer/CLI/core.py‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,7 @@ def parse_module_args(self, module_name, args):
120120
version=VERSION,
121121
argv=[module_name] + args,
122122
options_first=True)
123-
module = self.env.load_module(module_name)
124-
return module, arguments
123+
return arguments
125124

126125
def parse_command_args(self, module_name, command_name, args):
127126
command = self.env.get_command(module_name, command_name)
@@ -135,8 +134,7 @@ def parse(self, args):
135134
module_name = main_args['<module>']
136135

137136
# handle `sl <module> ...`
138-
module, module_args = self.parse_module_args(
139-
module_name, main_args['<args>'])
137+
module_args = self.parse_module_args(module_name, main_args['<args>'])
140138

141139
# get the command argument
142140
command_name = module_args.get('<command>')
@@ -172,24 +170,25 @@ def main(args=sys.argv[1:], env=Environment()):
172170
client = Client(config_file=command_args.get('--config'))
173171

174172
# Do the thing
175-
data = command.execute(client, command_args)
173+
runnable = command(client=client, env=env)
174+
data = runnable.execute(command_args)
176175
if data:
177-
format = command_args.get('--format', 'table')
178-
if format not in VALID_FORMATS:
179-
raise ArgumentError('Invalid format "%s"' % format)
180-
s = format_output(data, fmt=format)
176+
out_format = command_args.get('--format', 'table')
177+
if out_format not in VALID_FORMATS:
178+
raise ArgumentError('Invalid format "%s"' % out_format)
179+
s = format_output(data, fmt=out_format)
181180
if s:
182181
env.out(s)
183182

184183
if command_args.get('--timings'):
185-
format = command_args.get('--format', 'table')
184+
out_format = command_args.get('--format', 'table')
186185
api_calls = client.get_last_calls()
187186
t = KeyValueTable(['call', 'time'])
188187

189-
for call, initiated, duration in api_calls:
188+
for call, _, duration in api_calls:
190189
t.add_row([call, duration])
191190

192-
env.err(format_output(t, fmt=format))
191+
env.err(format_output(t, fmt=out_format))
193192

194193
except InvalidCommand as e:
195194
env.err(resolver.get_module_help(e.module_name))

‎SoftLayer/CLI/environment.py‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,8 @@ def get_module_name(self, module_name):
6464
def load_module(self, module_name): # pragma: no cover
6565
try:
6666
module = import_module('SoftLayer.CLI.modules.%s' % module_name)
67-
for name, obj in inspect.getmembers(module):
67+
for _, obj in inspect.getmembers(module):
6868
if inspect.isclass(obj) and issubclass(obj, CLIRunnable):
69-
obj.env = self
7069
self.add_plugin(obj)
7170
return module
7271
except ImportError:
@@ -105,12 +104,9 @@ class CLIRunnable(object):
105104
options = [] # set by subclass
106105
action = None # set by subclass
107106

108-
env = None # gets set later by Environment.load_module()
107+
def __init__(self, client=None, env=None):
108+
self.client = client
109+
self.env = env
109110

110-
@staticmethod
111-
def add_additional_args(parser):
112-
pass
113-
114-
@staticmethod
115-
def execute(client, args):
111+
def execute(self, args):
116112
pass

‎SoftLayer/CLI/modules/bmc.py‎

Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -43,32 +43,31 @@ class BMCCreateOptions(CLIRunnable):
4343
action = 'create-options'
4444
options = ['datacenter', 'cpu', 'memory', 'os', 'disk', 'nic']
4545

46-
@classmethod
47-
def execute(cls, client, args):
46+
def execute(self, args):
4847
t = KeyValueTable(['Name', 'Value'])
4948
t.align['Name'] = 'r'
5049
t.align['Value'] = 'l'
5150

5251
show_all = True
53-
for opt_name in cls.options:
52+
for opt_name in self.options:
5453
if args.get("--" + opt_name):
5554
show_all = False
5655
break
5756

58-
mgr = HardwareManager(client)
57+
mgr = HardwareManager(self.client)
5958

6059
bmi_options = mgr.get_bare_metal_create_options()
6160

6261
if args['--all']:
6362
show_all = True
6463

6564
if args['--datacenter'] or show_all:
66-
results = cls.get_create_options(bmi_options, 'datacenter')[0]
65+
results = self.get_create_options(bmi_options, 'datacenter')[0]
6766

6867
t.add_row([results[0], listing(sorted(results[1]))])
6968

7069
if args['--cpu'] or args['--memory'] or show_all:
71-
results = cls.get_create_options(bmi_options, 'cpu')
70+
results = self.get_create_options(bmi_options, 'cpu')
7271
memory_cpu_table = Table(['memory', 'cpu'])
7372
for result in results:
7473
memory_cpu_table.add_row([
@@ -80,7 +79,7 @@ def execute(cls, client, args):
8079
t.add_row(['memory/cpu', memory_cpu_table])
8180

8281
if args['--os'] or show_all:
83-
results = cls.get_create_options(bmi_options, 'os')
82+
results = self.get_create_options(bmi_options, 'os')
8483

8584
for result in results:
8685
t.add_row([
@@ -91,22 +90,21 @@ def execute(cls, client, args):
9190
)])
9291

9392
if args['--disk'] or show_all:
94-
results = cls.get_create_options(bmi_options, 'disk')[0]
93+
results = self.get_create_options(bmi_options, 'disk')[0]
9594

9695
t.add_row([results[0], listing(
9796
[item[0] for item in sorted(results[1])])])
9897

9998
if args['--nic'] or show_all:
100-
results = cls.get_create_options(bmi_options, 'nic')
99+
results = self.get_create_options(bmi_options, 'nic')
101100

102101
for result in results:
103102
t.add_row([result[0], listing(
104103
[item[0] for item in sorted(result[1],)])])
105104

106105
return t
107106

108-
@classmethod
109-
def get_create_options(cls, bmi_options, section, pretty=True):
107+
def get_create_options(self, bmi_options, section, pretty=True):
110108
""" This method can be used to parse the bare metal instance creation
111109
options into different sections. This can be useful for data validation
112110
as well as printing the options on a help screen.
@@ -257,7 +255,8 @@ def _generate_windows_code(description):
257255
dual.append((str(int(item['capacity'])) + '_DUAL',
258256
item['price_id']))
259257
else:
260-
single.append((str(int(item['capacity'])), item['price_id']))
258+
single.append((str(int(item['capacity'])),
259+
item['price_id']))
261260

262261
return [('single nic', single), ('dual nic', dual)]
263262

@@ -304,10 +303,9 @@ class CreateBMCInstance(CLIRunnable):
304303
options = ['confirm']
305304
required_params = ['--hostname', '--domain', '--cpu', '--memory', '--os']
306305

307-
@classmethod
308-
def execute(cls, client, args):
306+
def execute(self, args):
309307
update_with_template_args(args)
310-
mgr = HardwareManager(client)
308+
mgr = HardwareManager(self.client)
311309

312310
# Disks will be a comma-separated list. Let's make it a real list.
313311
if isinstance(args.get('--disk'), str):
@@ -317,7 +315,7 @@ def execute(cls, client, args):
317315
if isinstance(args.get('--key'), str):
318316
args['--key'] = args.get('--key').split(',')
319317

320-
cls._validate_args(args)
318+
self._validate_args(args)
321319

322320
bmi_options = mgr.get_bare_metal_create_options()
323321

@@ -328,9 +326,9 @@ def execute(cls, client, args):
328326
}
329327

330328
# Validate the CPU/Memory combination and get the price ID
331-
server_core = cls._get_cpu_and_memory_price_ids(bmi_options,
332-
args['--cpu'],
333-
args['--memory'])
329+
server_core = self._get_cpu_and_memory_price_ids(bmi_options,
330+
args['--cpu'],
331+
args['--memory'])
334332

335333
if server_core:
336334
order['server'] = server_core
@@ -340,8 +338,8 @@ def execute(cls, client, args):
340338
order['hourly'] = args['--hourly']
341339

342340
# Convert the OS code back into a price ID
343-
os_price = cls._get_price_id_from_options(bmi_options, 'os',
344-
args['--os'])
341+
os_price = self._get_price_id_from_options(bmi_options, 'os',
342+
args['--os'])
345343

346344
if os_price:
347345
order['os'] = os_price
@@ -353,22 +351,22 @@ def execute(cls, client, args):
353351
# Set the disk size
354352
disk_prices = []
355353
for disk in args.get('--disk'):
356-
disk_price = cls._get_price_id_from_options(bmi_options, 'disk',
357-
disk)
354+
disk_price = self._get_price_id_from_options(bmi_options, 'disk',
355+
disk)
358356

359357
if disk_price:
360358
disk_prices.append(disk_price)
361359

362360
if not disk_prices:
363-
disk_prices.append(cls._get_default_value(bmi_options, 'disk0'))
361+
disk_prices.append(self._get_default_value(bmi_options, 'disk0'))
364362

365363
order['disks'] = disk_prices
366364

367365
# Set the port speed
368366
port_speed = args.get('--network') or 10
369367

370-
nic_price = cls._get_price_id_from_options(bmi_options, 'nic',
371-
port_speed)
368+
nic_price = self._get_price_id_from_options(bmi_options, 'nic',
369+
port_speed)
372370

373371
if nic_price:
374372
order['port_speed'] = nic_price
@@ -379,8 +377,8 @@ def execute(cls, client, args):
379377
if args.get('--key'):
380378
keys = []
381379
for key in args.get('--key'):
382-
key_id = resolve_id(SshKeyManager(client).resolve_ids, key,
383-
'SshKey')
380+
key_id = resolve_id(SshKeyManager(self.client).resolve_ids,
381+
key, 'SshKey')
384382
keys.append(key_id)
385383
order['ssh_keys'] = keys
386384

@@ -441,9 +439,8 @@ def execute(cls, client, args):
441439

442440
return output
443441

444-
@classmethod
445-
def _validate_args(cls, args):
446-
invalid_args = [k for k in cls.required_params if args.get(k) is None]
442+
def _validate_args(self, args):
443+
invalid_args = [k for k in self.required_params if args.get(k) is None]
447444
if invalid_args:
448445
raise ArgumentError('Missing required options: %s'
449446
% ','.join(invalid_args))
@@ -460,8 +457,7 @@ def _validate_args(cls, args):
460457
if not any([args['--hourly'], args['--monthly']]):
461458
raise ArgumentError('One of [--hourly | --monthly] is required')
462459

463-
@classmethod
464-
def _get_cpu_and_memory_price_ids(cls, bmi_options, cpu_value,
460+
def _get_cpu_and_memory_price_ids(self, bmi_options, cpu_value,
465461
memory_value):
466462
bmi_obj = BMCCreateOptions()
467463
price_id = None
@@ -477,8 +473,7 @@ def _get_cpu_and_memory_price_ids(cls, bmi_options, cpu_value,
477473

478474
return price_id
479475

480-
@classmethod
481-
def _get_default_value(cls, bmi_options, option):
476+
def _get_default_value(self, bmi_options, option):
482477
if option not in bmi_options['categories']:
483478
return
484479

@@ -492,12 +487,11 @@ def _get_default_value(cls, bmi_options, option):
492487
]):
493488
return item['price_id']
494489

495-
@classmethod
496-
def _get_price_id_from_options(cls, bmi_options, option, value):
490+
def _get_price_id_from_options(self, bmi_options, option, value):
497491
bmi_obj = BMCCreateOptions()
498492
price_id = None
499493

500-
for k, v in bmi_obj.get_create_options(bmi_options, option, False):
494+
for _, v in bmi_obj.get_create_options(bmi_options, option, False):
501495
for item_options in v:
502496
if item_options[0] == value:
503497
price_id = item_options[1]
@@ -519,9 +513,8 @@ class CancelInstance(CLIRunnable):
519513
action = 'cancel'
520514
options = ['confirm']
521515

522-
@staticmethod
523-
def execute(client, args):
524-
hw = HardwareManager(client)
516+
def execute(self, args):
517+
hw = HardwareManager(self.client)
525518
hw_id = resolve_id(
526519
hw.resolve_ids, args.get('<identifier>'), 'hardware')
527520

0 commit comments

Comments
 (0)