Merge pull request #1015 from allmightyspiff/1006

allmightyspiff · web-flow · commit fe8c6a8125e6 · 2018-08-06T14:38:06.000-05:00
Pagination for vs list and others
diff --git 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 <TEST>`
+
+* `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="#<ISSUENUMBER> <whatever you did>`
+* `git push origin <issueBranch>`
+* create pull request
+
+
+
+
diff --git 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 those sections working
+            return list(self.iter_call(service, method, *args, **kwargs))
 
         invalid_kwargs = set(kwargs.keys()) - VALID_CALL_ARGS
         if invalid_kwargs:
@@ -267,55 +269,51 @@ 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
+        # Set to make unit tests, which call this function directly, play nice.
         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)
+        result_count = 0
+        keep_looping = True
 
-            # It looks like we ran out results
-            if not results:
-                break
+        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
                 result_count += 1
 
-            offset += chunk
+            # Got less results than requested, we are at the end
+            if len(results) < limit:
+                keep_looping = False
+            # Got all the needed items
+            if result_count >= results.total_count:
+                keep_looping = False
+
+            offset += limit
 
-            if len(results) < chunk:
-                break
+        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
@@ -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/securitygroup/list.py b/SoftLayer/CLI/securitygroup/list.py
@@ -16,16 +16,20 @@
 @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)
 
     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'],
diff --git 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,11 +81,11 @@ 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
-
     for guest in guests:
         table.add_row([value or formatting.blank()
                        for value in columns.row(guest)])
diff --git 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
@@ -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
@@ -477,11 +477,10 @@ 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,18 +513,20 @@ 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):
         """List security group rules associated with a security group.
 
         :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
@@ -158,8 +158,8 @@ 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/testing/__init__.py b/SoftLayer/testing/__init__.py
@@ -19,7 +19,6 @@
 from SoftLayer.CLI import environment
 from SoftLayer.testing import xmlrpc
 
-
 FIXTURE_PATH = os.path.abspath(os.path.join(__file__, '..', '..', 'fixtures'))
 
 
diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py
@@ -120,16 +120,25 @@ def __init__(self):
         #: Exception any exceptions that got caught
         self.exception = None
 
+    def __repr__(self):
+        """Prints out what this call is all about"""
+        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=None, 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 +450,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
@@ -209,3 +209,18 @@ def is_ready(instance, pending=False):
     if instance.get('provisionDate') and not reloading and not outstanding:
         return True
     return 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:
+        return " ".join(string.split())
diff --git a/tests/CLI/modules/vs_tests.py b/tests/CLI/modules/vs_tests.py
@@ -404,7 +404,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
diff --git a/tests/managers/block_tests.py b/tests/managers/block_tests.py
diff --git a/tests/managers/hardware_tests.py b/tests/managers/hardware_tests.py
diff --git a/tests/managers/ordering_tests.py b/tests/managers/ordering_tests.py
