From 85c1458d01f78372f5bd2fd7230141eb6d23dd6f Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Thu, 19 Jul 2018 17:20:22 -0500 Subject: [PATCH 1/7] #1006 fixed ther call_iter generator, and used it in a few listing methods --- SoftLayer/API.py | 49 +++++++++++++--------------------- SoftLayer/CLI/hardware/list.py | 9 +++++-- SoftLayer/CLI/virt/list.py | 9 +++++-- SoftLayer/CLI/vlan/list.py | 9 +++++-- SoftLayer/managers/hardware.py | 3 ++- SoftLayer/managers/network.py | 10 ++++--- SoftLayer/managers/vs.py | 5 ++-- SoftLayer/transports.py | 16 ++++++++--- SoftLayer/utils.py | 6 +++++ 9 files changed, 70 insertions(+), 46 deletions(-) diff --git a/SoftLayer/API.py b/SoftLayer/API.py index 92ee27b10..2732d9909 100644 --- a/SoftLayer/API.py +++ b/SoftLayer/API.py @@ -267,40 +267,25 @@ def iter_call(self, service, method, *args, **kwargs): :param service: the name of the SoftLayer API service :param method: the method to call on the service - :param integer chunk: result size for each API call (defaults to 100) + :param integer limit: result size for each API call (defaults to 100) :param \\*args: same optional arguments that ``Service.call`` takes - :param \\*\\*kwargs: same optional keyword arguments that - ``Service.call`` takes + :param \\*\\*kwargs: same optional keyword arguments that ``Service.call`` takes """ - chunk = kwargs.pop('chunk', 100) - limit = kwargs.pop('limit', None) - offset = kwargs.pop('offset', 0) - if chunk <= 0: - raise AttributeError("Chunk size should be greater than zero.") + limit = kwargs.pop('limit', 100) + offset = kwargs.pop('offset', 0) - if limit: - chunk = min(chunk, limit) + if limit <= 0: + raise AttributeError("Limit size should be greater than zero.") result_count = 0 - kwargs['iter'] = False - while True: - if limit: - # We've reached the end of the results - if result_count >= limit: - break - - # Don't over-fetch past the given limit - if chunk + result_count > limit: - chunk = limit - result_count - - results = self.call(service, method, - offset=offset, limit=chunk, *args, **kwargs) - - # It looks like we ran out results - if not results: - break + results = self.call(service, method, offset=offset, limit=limit, *args, **kwargs) + + if results.total_count <= 0: + raise StopIteration + + while result_count < results.total_count: # Apparently this method doesn't return a list. # Why are you even iterating over this? @@ -312,11 +297,15 @@ def iter_call(self, service, method, *args, **kwargs): yield item result_count += 1 - offset += chunk - - if len(results) < chunk: + # Got less results than requested, we are at the end + if len(results) < limit: break + offset += limit + # Get the next results + results = self.call(service, method, offset=offset, limit=limit, *args, **kwargs) + raise StopIteration + def __repr__(self): return "Client(transport=%r, auth=%r)" % (self.transport, self.auth) diff --git a/SoftLayer/CLI/hardware/list.py b/SoftLayer/CLI/hardware/list.py index 2e264e3ac..ba2a457d1 100644 --- a/SoftLayer/CLI/hardware/list.py +++ b/SoftLayer/CLI/hardware/list.py @@ -55,8 +55,12 @@ help='Columns to display. [options: %s]' % ', '.join(column.name for column in COLUMNS), default=','.join(DEFAULT_COLUMNS), show_default=True) +@click.option('--limit', '-l', + help='How many results to get in one api call, default is 100', + default=100, + show_default=True) @environment.pass_env -def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, tag, columns): +def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, tag, columns, limit): """List hardware servers.""" manager = SoftLayer.HardwareManager(env.client) @@ -67,7 +71,8 @@ def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, tag, co datacenter=datacenter, nic_speed=network, tags=tag, - mask="mask(SoftLayer_Hardware_Server)[%s]" % columns.mask()) + mask="mask(SoftLayer_Hardware_Server)[%s]" % columns.mask(), + limit=limit) table = formatting.Table(columns.columns) table.sortby = sortby diff --git a/SoftLayer/CLI/virt/list.py b/SoftLayer/CLI/virt/list.py index b2cd64f62..3d62d635f 100644 --- a/SoftLayer/CLI/virt/list.py +++ b/SoftLayer/CLI/virt/list.py @@ -62,9 +62,13 @@ % ', '.join(column.name for column in COLUMNS), default=','.join(DEFAULT_COLUMNS), show_default=True) +@click.option('--limit', '-l', + help='How many results to get in one api call, default is 100', + default=100, + show_default=True) @environment.pass_env def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, - hourly, monthly, tag, columns): + hourly, monthly, tag, columns, limit): """List virtual servers.""" vsi = SoftLayer.VSManager(env.client) @@ -77,7 +81,8 @@ def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, datacenter=datacenter, nic_speed=network, tags=tag, - mask=columns.mask()) + mask=columns.mask(), + limit=limit) table = formatting.Table(columns.columns) table.sortby = sortby diff --git a/SoftLayer/CLI/vlan/list.py b/SoftLayer/CLI/vlan/list.py index 13dc2b774..84b1806d5 100644 --- a/SoftLayer/CLI/vlan/list.py +++ b/SoftLayer/CLI/vlan/list.py @@ -26,8 +26,12 @@ help='Filter by datacenter shortname (sng01, dal05, ...)') @click.option('--number', '-n', help='Filter by VLAN number') @click.option('--name', help='Filter by VLAN name') +@click.option('--limit', '-l', + help='How many results to get in one api call, default is 100', + default=100, + show_default=True) @environment.pass_env -def cli(env, sortby, datacenter, number, name): +def cli(env, sortby, datacenter, number, name, limit): """List VLANs.""" mgr = SoftLayer.NetworkManager(env.client) @@ -37,7 +41,8 @@ def cli(env, sortby, datacenter, number, name): vlans = mgr.list_vlans(datacenter=datacenter, vlan_number=number, - name=name) + name=name, + limit=limit) for vlan in vlans: table.add_row([ vlan['id'], diff --git a/SoftLayer/managers/hardware.py b/SoftLayer/managers/hardware.py index b23c9e1ef..7e0c33773 100644 --- a/SoftLayer/managers/hardware.py +++ b/SoftLayer/managers/hardware.py @@ -197,7 +197,8 @@ def list_hardware(self, tags=None, cpus=None, memory=None, hostname=None, utils.query_filter(private_ip)) kwargs['filter'] = _filter.to_dict() - return self.account.getHardware(**kwargs) + kwargs['iter'] = True + return self.client.call('Account', 'getHardware', **kwargs) @retry(logger=LOGGER) def get_hardware(self, hardware_id, **kwargs): diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py index 265f325bf..47e67f430 100644 --- a/SoftLayer/managers/network.py +++ b/SoftLayer/managers/network.py @@ -477,11 +477,11 @@ def list_subnets(self, identifier=None, datacenter=None, version=0, utils.query_filter(network_space)) kwargs['filter'] = _filter.to_dict() + kwargs['iter'] = True + return self.client.call('Account', 'getSubnets', **kwargs) - return self.account.getSubnets(**kwargs) - def list_vlans(self, datacenter=None, vlan_number=None, name=None, - **kwargs): + def list_vlans(self, datacenter=None, vlan_number=None, name=None, **kwargs): """Display a list of all VLANs on the account. This provides a quick overview of all VLANs including information about @@ -514,10 +514,12 @@ def list_vlans(self, datacenter=None, vlan_number=None, name=None, if 'mask' not in kwargs: kwargs['mask'] = DEFAULT_VLAN_MASK + kwargs['iter'] = True return self.account.getNetworkVlans(**kwargs) def list_securitygroups(self, **kwargs): """List security groups.""" + kwargs['iter'] = True return self.security_group.getAllObjects(**kwargs) def list_securitygroup_rules(self, group_id): @@ -525,7 +527,7 @@ def list_securitygroup_rules(self, group_id): :param int group_id: The security group to list rules for """ - return self.security_group.getRules(id=group_id) + return self.security_group.getRules(id=group_id, iter=True) def remove_securitygroup_rule(self, group_id, rule_id): """Remove a rule from a security group. diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py index f0b331447..60446ca3f 100644 --- a/SoftLayer/managers/vs.py +++ b/SoftLayer/managers/vs.py @@ -157,8 +157,9 @@ def list_instances(self, hourly=True, monthly=True, tags=None, cpus=None, utils.query_filter(private_ip)) kwargs['filter'] = _filter.to_dict() - func = getattr(self.account, call) - return func(**kwargs) + kwargs['iter'] = True + return self.client.call('Account', call, **kwargs) + @retry(logger=LOGGER) def get_instance(self, instance_id, **kwargs): diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index df86d7399..903493884 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -120,16 +120,26 @@ def __init__(self): #: Exception any exceptions that got caught self.exception = None + def __repr__(self): + """Prints out what this call is all about""" + param_list = ['identifier', 'mask', 'filter', 'args', 'limit', 'offset'] + pretty_mask = utils.clean_string(self.mask) + pretty_filter = self.filter + param_string = "id={id}, mask='{mask}', filter='{filter}', args={args}, limit={limit}, offset={offset}".format( + id=self.identifier, mask=pretty_mask, filter=pretty_filter, + args=self.args, limit=self.limit, offset=self.offset) + return "{service}::{method}({params})".format( + service=self.service, method=self.method, params=param_string) + class SoftLayerListResult(list): """A SoftLayer API list result.""" - def __init__(self, items, total_count): + def __init__(self, items=[], total_count=0): #: total count of items that exist on the server. This is useful when #: paginating through a large list of objects. self.total_count = total_count - super(SoftLayerListResult, self).__init__(items) @@ -441,7 +451,7 @@ def __call__(self, call): def pre_transport_log(self, call): """Prints a warning before calling the API """ - output = "Calling: {}::{}(id={})".format(call.service, call.method, call.identifier) + output = "Calling: {})".format(call) LOGGER.warning(output) def post_transport_log(self, call): diff --git a/SoftLayer/utils.py b/SoftLayer/utils.py index 07eb72edb..f5ec99ad5 100644 --- a/SoftLayer/utils.py +++ b/SoftLayer/utils.py @@ -209,3 +209,9 @@ def is_ready(instance, pending=False): if instance.get('provisionDate') and not reloading and not outstanding: return True return False + +def clean_string(string): + if string is None: + return '' + else: + return " ".join(string.split()) \ No newline at end of file From dd4d2f49d32b12837d9abd8c53e948ac99f094d2 Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Thu, 19 Jul 2018 17:25:21 -0500 Subject: [PATCH 2/7] added iter_call to sg --- SoftLayer/CLI/securitygroup/list.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/SoftLayer/CLI/securitygroup/list.py b/SoftLayer/CLI/securitygroup/list.py index 0ba9ba1d4..3aaabdf46 100644 --- a/SoftLayer/CLI/securitygroup/list.py +++ b/SoftLayer/CLI/securitygroup/list.py @@ -16,8 +16,12 @@ @click.option('--sortby', help='Column to sort by', type=click.Choice(COLUMNS)) +@click.option('--limit', '-l', + help='How many results to get in one api call, default is 100', + default=100, + show_default=True) @environment.pass_env -def cli(env, sortby): +def cli(env, sortby, limit): """List security groups.""" mgr = SoftLayer.NetworkManager(env.client) @@ -25,7 +29,7 @@ def cli(env, sortby): table = formatting.Table(COLUMNS) table.sortby = sortby - sgs = mgr.list_securitygroups() + sgs = mgr.list_securitygroups(limit=limit) for secgroup in sgs: table.add_row([ secgroup['id'], From c3e3c5a9349eb2ced713e4eaef79845939d08e52 Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Wed, 1 Aug 2018 16:38:33 -0500 Subject: [PATCH 3/7] \#1006 fixed iter_call and setup `vs list`, `hw list`, `vlan list` and to use it by default --- SoftLayer/API.py | 33 ++-- SoftLayer/CLI/virt/list.py | 1 - SoftLayer/managers/hardware.py | 2 + SoftLayer/testing/__init__.py | 4 +- tests/CLI/modules/vs_tests.py | 2 +- tests/api_tests.py | 33 ++-- tests/managers/block_tests.py | 324 +++++++++++++++---------------- tests/managers/hardware_tests.py | 1 + tests/managers/ordering_tests.py | 30 +-- 9 files changed, 227 insertions(+), 203 deletions(-) diff --git a/SoftLayer/API.py b/SoftLayer/API.py index 2732d9909..ff7f917f5 100644 --- a/SoftLayer/API.py +++ b/SoftLayer/API.py @@ -214,7 +214,9 @@ def call(self, service, method, *args, **kwargs): """ if kwargs.pop('iter', False): - return self.iter_call(service, method, *args, **kwargs) + # Most of the codebase assumes a non-generator will be returned, so casting to list + # keeps thsoe sections working + return list(self.iter_call(service, method, *args, **kwargs)) invalid_kwargs = set(kwargs.keys()) - VALID_CALL_ARGS if invalid_kwargs: @@ -279,19 +281,24 @@ def iter_call(self, service, method, *args, **kwargs): if limit <= 0: raise AttributeError("Limit size should be greater than zero.") + # Set to make unit tests, which call this function directly, play nice. + kwargs['iter'] = False result_count = 0 - results = self.call(service, method, offset=offset, limit=limit, *args, **kwargs) + keep_looping = True - if results.total_count <= 0: - raise StopIteration - - while result_count < results.total_count: + while keep_looping: + # Get the next results + results = self.call(service, method, offset=offset, limit=limit, *args, **kwargs) # Apparently this method doesn't return a list. # Why are you even iterating over this? - if not isinstance(results, list): - yield results - break + if not isinstance(results, transports.SoftLayerListResult): + if isinstance(results, list): + # Close enough, this makes testing a lot easier + results = transports.SoftLayerListResult(results, len(results)) + else: + yield results + raise StopIteration for item in results: yield item @@ -299,11 +306,13 @@ def iter_call(self, service, method, *args, **kwargs): # Got less results than requested, we are at the end if len(results) < limit: - break + keep_looping = False + # Got all the needed items + if result_count >= results.total_count: + keep_looping = False offset += limit - # Get the next results - results = self.call(service, method, offset=offset, limit=limit, *args, **kwargs) + raise StopIteration def __repr__(self): diff --git a/SoftLayer/CLI/virt/list.py b/SoftLayer/CLI/virt/list.py index 3d62d635f..8feee7b35 100644 --- a/SoftLayer/CLI/virt/list.py +++ b/SoftLayer/CLI/virt/list.py @@ -86,7 +86,6 @@ def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, table = formatting.Table(columns.columns) table.sortby = sortby - for guest in guests: table.add_row([value or formatting.blank() for value in columns.row(guest)]) diff --git a/SoftLayer/managers/hardware.py b/SoftLayer/managers/hardware.py index 7e0c33773..18d3dcbe0 100644 --- a/SoftLayer/managers/hardware.py +++ b/SoftLayer/managers/hardware.py @@ -532,10 +532,12 @@ def _get_ids_from_ip(self, ip): # pylint: disable=inconsistent-return-statement # Find the server via ip address. First try public ip, then private results = self.list_hardware(public_ip=ip, mask="id") if results: + print("PUB") return [result['id'] for result in results] results = self.list_hardware(private_ip=ip, mask="id") if results: + print("Found privet") return [result['id'] for result in results] def edit(self, hardware_id, userdata=None, hostname=None, domain=None, diff --git a/SoftLayer/testing/__init__.py b/SoftLayer/testing/__init__.py index 69c439039..981aa3824 100644 --- a/SoftLayer/testing/__init__.py +++ b/SoftLayer/testing/__init__.py @@ -18,6 +18,7 @@ from SoftLayer.CLI import core from SoftLayer.CLI import environment from SoftLayer.testing import xmlrpc +from SoftLayer.transports import SoftLayerListResult FIXTURE_PATH = os.path.abspath(os.path.join(__file__, '..', '..', 'fixtures')) @@ -39,7 +40,8 @@ def __call__(self, call): return self.mocked[key](call) # Fall back to another transport (usually with fixtures) - return self.transport(call) + return self.transport(call) + def set_mock(self, service, method): """Create a mock and return the mock object for the specific API call. diff --git a/tests/CLI/modules/vs_tests.py b/tests/CLI/modules/vs_tests.py index c7ce60be8..cd2281ae0 100644 --- a/tests/CLI/modules/vs_tests.py +++ b/tests/CLI/modules/vs_tests.py @@ -379,7 +379,7 @@ def test_create_with_wait_not_ready(self, confirm_mock): '--network=100', '--billing=hourly', '--datacenter=dal05', - '--wait=10']) + '--wait=1']) self.assertEqual(result.exit_code, 1) diff --git a/tests/api_tests.py b/tests/api_tests.py index db42ee350..458153de4 100644 --- a/tests/api_tests.py +++ b/tests/api_tests.py @@ -144,7 +144,10 @@ def test_service_iter_call_with_chunk(self, _iter_call): @mock.patch('SoftLayer.API.BaseClient.call') def test_iter_call(self, _call): # chunk=100, no limit - _call.side_effect = [list(range(100)), list(range(100, 125))] + _call.side_effect = [ + transports.SoftLayerListResult(range(100), 125), + transports.SoftLayerListResult(range(100, 125), 125) + ] result = list(self.client.iter_call('SERVICE', 'METHOD', iter=True)) self.assertEqual(list(range(125)), result) @@ -155,7 +158,11 @@ def test_iter_call(self, _call): _call.reset_mock() # chunk=100, no limit. Requires one extra request. - _call.side_effect = [list(range(100)), list(range(100, 200)), []] + _call.side_effect = [ + transports.SoftLayerListResult(range(100), 201), + transports.SoftLayerListResult(range(100, 200), 201), + transports.SoftLayerListResult([], 201) + ] result = list(self.client.iter_call('SERVICE', 'METHOD', iter=True)) self.assertEqual(list(range(200)), result) _call.assert_has_calls([ @@ -166,13 +173,16 @@ def test_iter_call(self, _call): _call.reset_mock() # chunk=25, limit=30 - _call.side_effect = [list(range(0, 25)), list(range(25, 30))] + _call.side_effect = [ + transports.SoftLayerListResult(range(0, 25), 30), + transports.SoftLayerListResult(range(25, 30), 30) + ] result = list(self.client.iter_call( - 'SERVICE', 'METHOD', iter=True, limit=30, chunk=25)) + 'SERVICE', 'METHOD', iter=True, limit=25)) self.assertEqual(list(range(30)), result) _call.assert_has_calls([ mock.call('SERVICE', 'METHOD', iter=False, limit=25, offset=0), - mock.call('SERVICE', 'METHOD', iter=False, limit=5, offset=25), + mock.call('SERVICE', 'METHOD', iter=False, limit=25, offset=25), ]) _call.reset_mock() @@ -185,26 +195,27 @@ def test_iter_call(self, _call): ]) _call.reset_mock() - # chunk=25, limit=30, offset=12 - _call.side_effect = [list(range(0, 25)), list(range(25, 30))] + _call.side_effect = [ + transports.SoftLayerListResult(range(0, 25), 30), + transports.SoftLayerListResult(range(25, 30), 30) + ] result = list(self.client.iter_call('SERVICE', 'METHOD', 'ARG', iter=True, - limit=30, - chunk=25, + limit=25, offset=12)) self.assertEqual(list(range(30)), result) _call.assert_has_calls([ mock.call('SERVICE', 'METHOD', 'ARG', iter=False, limit=25, offset=12), mock.call('SERVICE', 'METHOD', 'ARG', - iter=False, limit=5, offset=37), + iter=False, limit=25, offset=37), ]) # Chunk size of 0 is invalid self.assertRaises( AttributeError, lambda: list(self.client.iter_call('SERVICE', 'METHOD', - iter=True, chunk=0))) + iter=True, limit=0))) def test_call_invalid_arguments(self): self.assertRaises( diff --git a/tests/managers/block_tests.py b/tests/managers/block_tests.py index fa5dacb2b..bd5ab9d37 100644 --- a/tests/managers/block_tests.py +++ b/tests/managers/block_tests.py @@ -396,22 +396,22 @@ def test_order_block_volume_performance(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService', - 'packageId': 759, - 'prices': [ - {'id': 189433}, - {'id': 189443}, - {'id': 190113}, - {'id': 190173} - ], - 'volumeSize': 1000, - 'quantity': 1, - 'location': 449494, - 'iops': 2000, - 'useHourlyPricing': False, - 'osFormatType': {'keyName': 'LINUX'} - },) + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService', + 'packageId': 759, + 'prices': [ + {'id': 189433}, + {'id': 189443}, + {'id': 190113}, + {'id': 190173} + ], + 'volumeSize': 1000, + 'quantity': 1, + 'location': 449494, + 'iops': 2000, + 'useHourlyPricing': False, + 'osFormatType': {'keyName': 'LINUX'} + },) ) def test_order_block_volume_endurance(self): @@ -440,21 +440,21 @@ def test_order_block_volume_endurance(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService', - 'packageId': 759, - 'prices': [ - {'id': 189433}, - {'id': 189443}, - {'id': 194763}, - {'id': 194703} - ], - 'volumeSize': 1000, - 'quantity': 1, - 'location': 449494, - 'useHourlyPricing': False, - 'osFormatType': {'keyName': 'LINUX'} - },) + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService', + 'packageId': 759, + 'prices': [ + {'id': 189433}, + {'id': 189443}, + {'id': 194763}, + {'id': 194703} + ], + 'volumeSize': 1000, + 'quantity': 1, + 'location': 449494, + 'useHourlyPricing': False, + 'osFormatType': {'keyName': 'LINUX'} + },) ) def test_authorize_host_to_volume(self): @@ -564,17 +564,17 @@ def test_order_block_snapshot_space_upgrade(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_Network_' - 'Storage_Enterprise_SnapshotSpace_Upgrade', - 'packageId': 759, - 'prices': [ - {'id': 193853} - ], - 'quantity': 1, - 'location': 449500, - 'volumeId': 102, - 'useHourlyPricing': False - },) + 'complexType': 'SoftLayer_Container_Product_Order_Network_' + 'Storage_Enterprise_SnapshotSpace_Upgrade', + 'packageId': 759, + 'prices': [ + {'id': 193853} + ], + 'quantity': 1, + 'location': 449500, + 'volumeId': 102, + 'useHourlyPricing': False + },) ) def test_order_block_snapshot_space(self): @@ -593,17 +593,17 @@ def test_order_block_snapshot_space(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_Network_' - 'Storage_Enterprise_SnapshotSpace', - 'packageId': 759, - 'prices': [ - {'id': 193613} - ], - 'quantity': 1, - 'location': 449500, - 'volumeId': 102, - 'useHourlyPricing': False - },) + 'complexType': 'SoftLayer_Container_Product_Order_Network_' + 'Storage_Enterprise_SnapshotSpace', + 'packageId': 759, + 'prices': [ + {'id': 193613} + ], + 'quantity': 1, + 'location': 449500, + 'volumeId': 102, + 'useHourlyPricing': False + },) ) def test_order_block_replicant_os_type_not_found(self): @@ -649,26 +649,26 @@ def test_order_block_replicant_performance_os_type_given(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService', - 'packageId': 759, - 'prices': [ - {'id': 189433}, - {'id': 189443}, - {'id': 189993}, - {'id': 190053}, - {'id': 191193}, - {'id': 192033} - ], - 'volumeSize': 500, - 'quantity': 1, - 'location': 449494, - 'iops': 1000, - 'originVolumeId': 102, - 'originVolumeScheduleId': 978, - 'useHourlyPricing': False, - 'osFormatType': {'keyName': 'XEN'} - },) + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService', + 'packageId': 759, + 'prices': [ + {'id': 189433}, + {'id': 189443}, + {'id': 189993}, + {'id': 190053}, + {'id': 191193}, + {'id': 192033} + ], + 'volumeSize': 500, + 'quantity': 1, + 'location': 449494, + 'iops': 1000, + 'originVolumeId': 102, + 'originVolumeScheduleId': 978, + 'useHourlyPricing': False, + 'osFormatType': {'keyName': 'XEN'} + },) ) def test_order_block_replicant_endurance(self): @@ -690,25 +690,25 @@ def test_order_block_replicant_endurance(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService', - 'packageId': 759, - 'prices': [ - {'id': 189433}, - {'id': 189443}, - {'id': 193433}, - {'id': 193373}, - {'id': 193613}, - {'id': 194693} - ], - 'volumeSize': 500, - 'quantity': 1, - 'location': 449494, - 'originVolumeId': 102, - 'originVolumeScheduleId': 978, - 'useHourlyPricing': False, - 'osFormatType': {'keyName': 'LINUX'} - },) + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService', + 'packageId': 759, + 'prices': [ + {'id': 189433}, + {'id': 189443}, + {'id': 193433}, + {'id': 193373}, + {'id': 193613}, + {'id': 194693} + ], + 'volumeSize': 500, + 'quantity': 1, + 'location': 449494, + 'originVolumeId': 102, + 'originVolumeScheduleId': 978, + 'useHourlyPricing': False, + 'osFormatType': {'keyName': 'LINUX'} + },) ) def test_order_block_duplicate_origin_os_type_not_found(self): @@ -748,23 +748,23 @@ def test_order_block_duplicate_performance_no_duplicate_snapshot(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService', - 'packageId': 759, - 'prices': [ - {'id': 189433}, - {'id': 189443}, - {'id': 189993}, - {'id': 190053} - ], - 'volumeSize': 500, - 'quantity': 1, - 'location': 449500, - 'duplicateOriginVolumeId': 102, - 'osFormatType': {'keyName': 'LINUX'}, - 'iops': 1000, - 'useHourlyPricing': False - },)) + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService', + 'packageId': 759, + 'prices': [ + {'id': 189433}, + {'id': 189443}, + {'id': 189993}, + {'id': 190053} + ], + 'volumeSize': 500, + 'quantity': 1, + 'location': 449500, + 'duplicateOriginVolumeId': 102, + 'osFormatType': {'keyName': 'LINUX'}, + 'iops': 1000, + 'useHourlyPricing': False + },)) def test_order_block_duplicate_performance(self): mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') @@ -790,25 +790,25 @@ def test_order_block_duplicate_performance(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService', - 'packageId': 759, - 'prices': [ - {'id': 189433}, - {'id': 189443}, - {'id': 190113}, - {'id': 190173}, - {'id': 191193} - ], - 'volumeSize': 1000, - 'quantity': 1, - 'location': 449500, - 'duplicateOriginVolumeId': 102, - 'osFormatType': {'keyName': 'LINUX'}, - 'duplicateOriginSnapshotId': 470, - 'iops': 2000, - 'useHourlyPricing': False - },)) + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService', + 'packageId': 759, + 'prices': [ + {'id': 189433}, + {'id': 189443}, + {'id': 190113}, + {'id': 190173}, + {'id': 191193} + ], + 'volumeSize': 1000, + 'quantity': 1, + 'location': 449500, + 'duplicateOriginVolumeId': 102, + 'osFormatType': {'keyName': 'LINUX'}, + 'duplicateOriginSnapshotId': 470, + 'iops': 2000, + 'useHourlyPricing': False + },)) def test_order_block_duplicate_endurance_no_duplicate_snapshot(self): mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') @@ -828,22 +828,22 @@ def test_order_block_duplicate_endurance_no_duplicate_snapshot(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService', - 'packageId': 759, - 'prices': [ - {'id': 189433}, - {'id': 189443}, - {'id': 193433}, - {'id': 193373} - ], - 'volumeSize': 500, - 'quantity': 1, - 'location': 449500, - 'duplicateOriginVolumeId': 102, - 'osFormatType': {'keyName': 'LINUX'}, - 'useHourlyPricing': False - },)) + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService', + 'packageId': 759, + 'prices': [ + {'id': 189433}, + {'id': 189443}, + {'id': 193433}, + {'id': 193373} + ], + 'volumeSize': 500, + 'quantity': 1, + 'location': 449500, + 'duplicateOriginVolumeId': 102, + 'osFormatType': {'keyName': 'LINUX'}, + 'useHourlyPricing': False + },)) def test_order_block_duplicate_endurance(self): mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') @@ -868,24 +868,24 @@ def test_order_block_duplicate_endurance(self): 'SoftLayer_Product_Order', 'placeOrder', args=({ - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService', - 'packageId': 759, - 'prices': [ - {'id': 189433}, - {'id': 189443}, - {'id': 194763}, - {'id': 194703}, - {'id': 194943} - ], - 'volumeSize': 1000, - 'quantity': 1, - 'location': 449500, - 'duplicateOriginVolumeId': 102, - 'osFormatType': {'keyName': 'LINUX'}, - 'duplicateOriginSnapshotId': 470, - 'useHourlyPricing': False - },)) + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService', + 'packageId': 759, + 'prices': [ + {'id': 189433}, + {'id': 189443}, + {'id': 194763}, + {'id': 194703}, + {'id': 194943} + ], + 'volumeSize': 1000, + 'quantity': 1, + 'location': 449500, + 'duplicateOriginVolumeId': 102, + 'osFormatType': {'keyName': 'LINUX'}, + 'duplicateOriginSnapshotId': 470, + 'useHourlyPricing': False + },)) def test_order_block_modified_performance(self): mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') diff --git a/tests/managers/hardware_tests.py b/tests/managers/hardware_tests.py index 8184ebd93..add6389fa 100644 --- a/tests/managers/hardware_tests.py +++ b/tests/managers/hardware_tests.py @@ -37,6 +37,7 @@ def test_init_with_ordering_manager(self): self.assertEqual(mgr.ordering_manager, ordering_manager) def test_list_hardware(self): + # Cast result back to list because list_hardware is now a generator results = self.hardware.list_hardware() self.assertEqual(results, fixtures.SoftLayer_Account.getHardware) diff --git a/tests/managers/ordering_tests.py b/tests/managers/ordering_tests.py index 01548c5cb..729659ba6 100644 --- a/tests/managers/ordering_tests.py +++ b/tests/managers/ordering_tests.py @@ -339,14 +339,14 @@ def test_generate_order_with_preset(self): items = ['ITEM1', 'ITEM2'] preset = 'PRESET_KEYNAME' expected_order = {'orderContainers': [ - {'complexType': 'SoftLayer_Container_Foo', - 'location': 1854895, - 'packageId': 1234, - 'presetId': 5678, - 'prices': [{'id': 1111}, {'id': 2222}], - 'quantity': 1, - 'useHourlyPricing': True} - ]} + {'complexType': 'SoftLayer_Container_Foo', + 'location': 1854895, + 'packageId': 1234, + 'presetId': 5678, + 'prices': [{'id': 1111}, {'id': 2222}], + 'quantity': 1, + 'useHourlyPricing': True} + ]} mock_pkg, mock_preset, mock_get_ids = self._patch_for_generate() @@ -362,13 +362,13 @@ def test_generate_order(self): items = ['ITEM1', 'ITEM2'] complex_type = 'My_Type' expected_order = {'orderContainers': [ - {'complexType': 'My_Type', - 'location': 1854895, - 'packageId': 1234, - 'prices': [{'id': 1111}, {'id': 2222}], - 'quantity': 1, - 'useHourlyPricing': True} - ]} + {'complexType': 'My_Type', + 'location': 1854895, + 'packageId': 1234, + 'prices': [{'id': 1111}, {'id': 2222}], + 'quantity': 1, + 'useHourlyPricing': True} + ]} mock_pkg, mock_preset, mock_get_ids = self._patch_for_generate() From a1a9eb8f1b7b277f408c3a737aa11eac675a5222 Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Wed, 1 Aug 2018 16:45:16 -0500 Subject: [PATCH 4/7] autopep8 --- CONTRIBUTING.md | 15 +++++++++++++++ SoftLayer/CLI/hardware/list.py | 2 +- SoftLayer/CLI/securitygroup/list.py | 2 +- SoftLayer/CLI/virt/list.py | 2 +- SoftLayer/CLI/vlan/list.py | 2 +- SoftLayer/managers/network.py | 1 - SoftLayer/managers/vs.py | 1 - SoftLayer/testing/__init__.py | 3 +-- SoftLayer/transports.py | 4 ++-- SoftLayer/utils.py | 3 ++- 10 files changed, 24 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 460e436e7..0f6fd444a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,3 +12,18 @@ guidelines below. * Additional infomration can be found in our [contribution guide](http://softlayer-python.readthedocs.org/en/latest/dev/index.html) +## Code style + +Code is tested and style checked with tox, you can run the tox tests individually by doing `tox -e ` + +* `autopep8 -r -v -i --max-line-length 119 SoftLayer/` +* `autopep8 -r -v -i --max-line-length 119 tests/` +* `tox -e analysis` +* `tox -e py36` +* `git commit --message="# ` +* `git push origin ` +* create pull request + + + + diff --git a/SoftLayer/CLI/hardware/list.py b/SoftLayer/CLI/hardware/list.py index ba2a457d1..1a607880f 100644 --- a/SoftLayer/CLI/hardware/list.py +++ b/SoftLayer/CLI/hardware/list.py @@ -57,7 +57,7 @@ show_default=True) @click.option('--limit', '-l', help='How many results to get in one api call, default is 100', - default=100, + default=100, show_default=True) @environment.pass_env def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, tag, columns, limit): diff --git a/SoftLayer/CLI/securitygroup/list.py b/SoftLayer/CLI/securitygroup/list.py index 3aaabdf46..2159a17f7 100644 --- a/SoftLayer/CLI/securitygroup/list.py +++ b/SoftLayer/CLI/securitygroup/list.py @@ -18,7 +18,7 @@ type=click.Choice(COLUMNS)) @click.option('--limit', '-l', help='How many results to get in one api call, default is 100', - default=100, + default=100, show_default=True) @environment.pass_env def cli(env, sortby, limit): diff --git a/SoftLayer/CLI/virt/list.py b/SoftLayer/CLI/virt/list.py index 8feee7b35..3975ad333 100644 --- a/SoftLayer/CLI/virt/list.py +++ b/SoftLayer/CLI/virt/list.py @@ -64,7 +64,7 @@ show_default=True) @click.option('--limit', '-l', help='How many results to get in one api call, default is 100', - default=100, + default=100, show_default=True) @environment.pass_env def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, diff --git a/SoftLayer/CLI/vlan/list.py b/SoftLayer/CLI/vlan/list.py index 84b1806d5..44500532a 100644 --- a/SoftLayer/CLI/vlan/list.py +++ b/SoftLayer/CLI/vlan/list.py @@ -28,7 +28,7 @@ @click.option('--name', help='Filter by VLAN name') @click.option('--limit', '-l', help='How many results to get in one api call, default is 100', - default=100, + default=100, show_default=True) @environment.pass_env def cli(env, sortby, datacenter, number, name, limit): diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py index 47e67f430..b568e8896 100644 --- a/SoftLayer/managers/network.py +++ b/SoftLayer/managers/network.py @@ -480,7 +480,6 @@ def list_subnets(self, identifier=None, datacenter=None, version=0, kwargs['iter'] = True return self.client.call('Account', 'getSubnets', **kwargs) - def list_vlans(self, datacenter=None, vlan_number=None, name=None, **kwargs): """Display a list of all VLANs on the account. diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py index 60446ca3f..5c1719f9d 100644 --- a/SoftLayer/managers/vs.py +++ b/SoftLayer/managers/vs.py @@ -159,7 +159,6 @@ def list_instances(self, hourly=True, monthly=True, tags=None, cpus=None, kwargs['filter'] = _filter.to_dict() kwargs['iter'] = True return self.client.call('Account', call, **kwargs) - @retry(logger=LOGGER) def get_instance(self, instance_id, **kwargs): diff --git a/SoftLayer/testing/__init__.py b/SoftLayer/testing/__init__.py index 981aa3824..608286ff9 100644 --- a/SoftLayer/testing/__init__.py +++ b/SoftLayer/testing/__init__.py @@ -40,8 +40,7 @@ def __call__(self, call): return self.mocked[key](call) # Fall back to another transport (usually with fixtures) - return self.transport(call) - + return self.transport(call) def set_mock(self, service, method): """Create a mock and return the mock object for the specific API call. diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 903493884..4797d41fd 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -126,8 +126,8 @@ def __repr__(self): pretty_mask = utils.clean_string(self.mask) pretty_filter = self.filter param_string = "id={id}, mask='{mask}', filter='{filter}', args={args}, limit={limit}, offset={offset}".format( - id=self.identifier, mask=pretty_mask, filter=pretty_filter, - args=self.args, limit=self.limit, offset=self.offset) + id=self.identifier, mask=pretty_mask, filter=pretty_filter, + args=self.args, limit=self.limit, offset=self.offset) return "{service}::{method}({params})".format( service=self.service, method=self.method, params=param_string) diff --git a/SoftLayer/utils.py b/SoftLayer/utils.py index f5ec99ad5..bef1e935b 100644 --- a/SoftLayer/utils.py +++ b/SoftLayer/utils.py @@ -210,8 +210,9 @@ def is_ready(instance, pending=False): return True return False + def clean_string(string): if string is None: return '' else: - return " ".join(string.split()) \ No newline at end of file + return " ".join(string.split()) From 40a289815555288f334648e5b60b15a21075857f Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Wed, 1 Aug 2018 16:53:16 -0500 Subject: [PATCH 5/7] removed some debugging code --- SoftLayer/managers/hardware.py | 2 -- SoftLayer/testing/__init__.py | 2 -- SoftLayer/transports.py | 3 +-- SoftLayer/utils.py | 8 ++++++++ 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/SoftLayer/managers/hardware.py b/SoftLayer/managers/hardware.py index 18d3dcbe0..7e0c33773 100644 --- a/SoftLayer/managers/hardware.py +++ b/SoftLayer/managers/hardware.py @@ -532,12 +532,10 @@ def _get_ids_from_ip(self, ip): # pylint: disable=inconsistent-return-statement # Find the server via ip address. First try public ip, then private results = self.list_hardware(public_ip=ip, mask="id") if results: - print("PUB") return [result['id'] for result in results] results = self.list_hardware(private_ip=ip, mask="id") if results: - print("Found privet") return [result['id'] for result in results] def edit(self, hardware_id, userdata=None, hostname=None, domain=None, diff --git a/SoftLayer/testing/__init__.py b/SoftLayer/testing/__init__.py index 608286ff9..477815725 100644 --- a/SoftLayer/testing/__init__.py +++ b/SoftLayer/testing/__init__.py @@ -18,8 +18,6 @@ from SoftLayer.CLI import core from SoftLayer.CLI import environment from SoftLayer.testing import xmlrpc -from SoftLayer.transports import SoftLayerListResult - FIXTURE_PATH = os.path.abspath(os.path.join(__file__, '..', '..', 'fixtures')) diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 4797d41fd..3aa896f11 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -122,7 +122,6 @@ def __init__(self): def __repr__(self): """Prints out what this call is all about""" - param_list = ['identifier', 'mask', 'filter', 'args', 'limit', 'offset'] pretty_mask = utils.clean_string(self.mask) pretty_filter = self.filter param_string = "id={id}, mask='{mask}', filter='{filter}', args={args}, limit={limit}, offset={offset}".format( @@ -135,7 +134,7 @@ def __repr__(self): class SoftLayerListResult(list): """A SoftLayer API list result.""" - def __init__(self, items=[], total_count=0): + def __init__(self, items=None, total_count=0): #: total count of items that exist on the server. This is useful when #: paginating through a large list of objects. diff --git a/SoftLayer/utils.py b/SoftLayer/utils.py index bef1e935b..d4218ee93 100644 --- a/SoftLayer/utils.py +++ b/SoftLayer/utils.py @@ -212,6 +212,14 @@ def is_ready(instance, pending=False): def clean_string(string): + """Returns a string with all newline and other whitespace garbage removed. + + Mostly this method is used to print out objectMasks that have a lot of extra whitespace + in them because making compact masks in python means they don't look nice in the IDE. + + :param string: The string to clean. + :returns string: A string without extra whitespace + """ if string is None: return '' else: From 549460f7a973e89ec3b44ea28c8668077c2fa36c Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Wed, 1 Aug 2018 16:59:48 -0500 Subject: [PATCH 6/7] finishing touches --- SoftLayer/utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/SoftLayer/utils.py b/SoftLayer/utils.py index d4218ee93..131c681f1 100644 --- a/SoftLayer/utils.py +++ b/SoftLayer/utils.py @@ -213,12 +213,12 @@ def is_ready(instance, pending=False): def clean_string(string): """Returns a string with all newline and other whitespace garbage removed. - - Mostly this method is used to print out objectMasks that have a lot of extra whitespace + + Mostly this method is used to print out objectMasks that have a lot of extra whitespace in them because making compact masks in python means they don't look nice in the IDE. - + :param string: The string to clean. - :returns string: A string without extra whitespace + :returns string: A string without extra whitespace. """ if string is None: return '' From 7ba0fa09d61169468f8abae5183ba17fedfde042 Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Thu, 2 Aug 2018 14:50:50 -0500 Subject: [PATCH 7/7] fixed a typo --- SoftLayer/API.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SoftLayer/API.py b/SoftLayer/API.py index ff7f917f5..3fbab72b6 100644 --- a/SoftLayer/API.py +++ b/SoftLayer/API.py @@ -215,7 +215,7 @@ def call(self, service, method, *args, **kwargs): """ if kwargs.pop('iter', False): # Most of the codebase assumes a non-generator will be returned, so casting to list - # keeps thsoe sections working + # keeps those sections working return list(self.iter_call(service, method, *args, **kwargs)) invalid_kwargs = set(kwargs.keys()) - VALID_CALL_ARGS