Made iteration more efficient and simplified code.

cloudbridge/cloud/base.py

@@ -2,6 +2,7 @@
 Implementation of common methods across cloud providers.
 """
 
+import itertools
 import logging
 import time
 
@@ -164,6 +165,7 @@ class BaseResultList(ResultList):
 
     def __init__(
             self, is_truncated, marker, supports_total, total=None, data=None):
+        # call list constructor
         super(BaseResultList, self).__init__(data or [])
         self._marker = marker
         self._is_truncated = is_truncated
@@ -187,6 +189,62 @@ class BaseResultList(ResultList):
         return self._total
 
 
+class ServerPagedResultList(BaseResultList):
+    """
+    This is a convenience class that extends the :class:`BaseResultList` class
+    and provides a server side implementation of paging. It is meant for use by
+    provider developers and is not meant for direct use by end-users.
+    This class can be used to wrap a partial result list when an operation
+    supports server side paging.
+    """
+
+    @property
+    def supports_server_paging(self):
+        return True
+
+    @property
+    def data(self):
+        raise NotImplementedError(
+            "ServerPagedResultLists do not support the data property")
+
+
+class ClientPagedResultList(BaseResultList):
+    """
+    This is a convenience class that extends the :class:`BaseResultList` class
+    and provides a client side implementation of paging. It is meant for use by
+    provider developers and is not meant for direct use by end-users.
+    This class can be used to wrap a full result list when an operation does
+    not support server side paging. This class will then provide a paged view
+    of the full result set entirely on the client side.
+    """
+
+    def __init__(self, provider, objects, limit=None, marker=None):
+        self._objects = objects
+        limit = limit or provider.config.result_limit
+        total_size = len(objects)
+        if marker:
+            from_marker = itertools.dropwhile(
+                lambda obj: not obj.id == marker, objects)
+            # skip one past the marker
+            next(from_marker, None)
+            objects = list(from_marker)
+        is_truncated = len(objects) > limit
+        results = list(itertools.islice(objects, limit))
+        super(ClientPagedResultList, self).__init__(
+            is_truncated,
+            results[-1].id if is_truncated else None,
+            True, total=total_size,
+            data=results)
+
+    @property
+    def supports_server_paging(self):
+        return False
+
+    @property
+    def data(self):
+        return self._objects
+
+
 class BasePageableObjectMixin(PageableObjectMixin):
     """
     A mixin to provide iteration capability for a class
@@ -194,15 +252,20 @@ class BasePageableObjectMixin(PageableObjectMixin):
     """
 
     def __iter__(self):
-        more_results = True
         marker = None
 
-        while more_results:
-            result_list = self.list(marker=marker)
+        result_list = self.list(marker=marker)
+        if result_list.supports_server_paging:
             for result in result_list:
                 yield result
-            marker = result_list.marker
-            more_results = result_list.is_truncated
+            while result_list.is_truncated:
+                result_list = self.list(marker=marker)
+                for result in result_list:
+                    yield result
+                marker = result_list.marker
+        else:
+            for result in result_list.data:
+                yield result
 
 
 class BaseInstanceType(InstanceType):

cloudbridge/cloud/helpers.py

@@ -1,27 +0,0 @@
-"""
-Helper functions
-"""
-import itertools
-from cloudbridge.cloud.base import BaseResultList
-
-
-def to_result_list(provider, objects, limit, marker):
-    """
-    Converts a list of objects to a ResultList, applying paging based on limit
-    and marker. This method is only intended for use by cloud operations which
-    do not natively support paging, since it's somewhat inefficient.
-    """
-    limit = limit or provider.config.result_limit
-    total_size = len(objects)
-    if marker:
-        from_marker = itertools.dropwhile(
-            lambda obj: not obj.id == marker, objects)
-        # skip one past the marker
-        next(from_marker, None)
-        objects = list(from_marker)
-    is_truncated = len(objects) > limit
-    results = list(itertools.islice(objects, limit))
-    return BaseResultList(is_truncated,
-                          results[-1].id if is_truncated else None,
-                          True, total=total_size,
-                          data=results)

cloudbridge/cloud/interfaces/resources.py

@@ -304,6 +304,19 @@ class ResultList(list):
         """
         pass
 
+    @abstractproperty
+    def supports_server_paging(self):
+        """
+        Indicates whether this ResultList supports client side paging or server
+        side paging. If server side paging is not supported, the data property
+        provides direct access to all available data.
+        """
+        pass
+
+    @abstractproperty
+    def data(self):
+        pass
+
 
 class InstanceState(object):
 

cloudbridge/cloud/providers/aws/resources.py

@@ -7,7 +7,6 @@ from boto.exception import EC2ResponseError
 from boto.s3.key import Key
 from retrying import retry
 
-from cloudbridge.cloud import helpers as cbhelpers
 from cloudbridge.cloud.base import BaseBucket
 from cloudbridge.cloud.base import BaseBucketObject
 from cloudbridge.cloud.base import BaseInstance
@@ -20,6 +19,7 @@ from cloudbridge.cloud.base import BaseSecurityGroup
 from cloudbridge.cloud.base import BaseSecurityGroupRule
 from cloudbridge.cloud.base import BaseSnapshot
 from cloudbridge.cloud.base import BaseVolume
+from cloudbridge.cloud.base import ClientPagedResultList
 from cloudbridge.cloud.interfaces.resources import InstanceState
 from cloudbridge.cloud.interfaces.resources import MachineImageState
 from cloudbridge.cloud.interfaces.resources import SnapshotState
@@ -663,8 +663,8 @@ class AWSBucket(BaseBucket):
         """
         objects = [AWSBucketObject(self._provider, obj)
                    for obj in self._bucket.list()]
-        return cbhelpers.to_result_list(self._provider, objects, limit,
-                                        marker)
+        return ClientPagedResultList(self._provider, objects,
+                                     limit=limit, marker=marker)
 
     def delete(self, delete_contents=False):
         """

cloudbridge/cloud/providers/aws/services.py

@@ -8,7 +8,6 @@ from boto.ec2.blockdevicemapping import BlockDeviceType
 from boto.exception import EC2ResponseError
 import requests
 
-from cloudbridge.cloud import helpers as cbhelpers
 from cloudbridge.cloud.base import BaseBlockStoreService
 from cloudbridge.cloud.base import BaseComputeService
 from cloudbridge.cloud.base import BaseImageService
@@ -18,11 +17,12 @@ from cloudbridge.cloud.base import BaseKeyPairService
 from cloudbridge.cloud.base import BaseLaunchConfig
 from cloudbridge.cloud.base import BaseObjectStoreService
 from cloudbridge.cloud.base import BaseRegionService
-from cloudbridge.cloud.base import BaseResultList
 from cloudbridge.cloud.base import BaseSecurityGroupService
 from cloudbridge.cloud.base import BaseSecurityService
 from cloudbridge.cloud.base import BaseSnapshotService
 from cloudbridge.cloud.base import BaseVolumeService
+from cloudbridge.cloud.base import ClientPagedResultList
+from cloudbridge.cloud.base import ServerPagedResultList
 from cloudbridge.cloud.interfaces.resources \
     import InvalidConfigurationException
 from cloudbridge.cloud.interfaces.resources import InstanceType
@@ -89,8 +89,8 @@ class AWSKeyPairService(BaseKeyPairService):
         """
         key_pairs = [AWSKeyPair(self.provider, kp)
                      for kp in self.provider.ec2_conn.get_all_key_pairs()]
-        return cbhelpers.to_result_list(self.provider, key_pairs, limit,
-                                        marker)
+        return ClientPagedResultList(self.provider, key_pairs,
+                                     limit=limit, marker=marker)
 
     def find(self, name):
         """
@@ -134,7 +134,8 @@ class AWSSecurityGroupService(BaseSecurityGroupService):
         sgs = [AWSSecurityGroup(self.provider, sg)
                for sg in self.provider.ec2_conn.get_all_security_groups()]
 
-        return cbhelpers.to_result_list(self.provider, sgs, limit, marker)
+        return ClientPagedResultList(self.provider, sgs,
+                                     limit=limit, marker=marker)
 
     def create(self, name, description):
         """
@@ -246,13 +247,12 @@ class AWSVolumeService(BaseVolumeService):
         List all volumes.
         """
         filtr = awshelpers.to_filter(self.provider, limit, marker)
-        vols = self.provider.ec2_conn.get_all_volumes(filters=filtr)
-        results = BaseResultList(vols.is_truncated,
-                                 vols.next_token,
-                                 False)
-        for vol in vols:
-            results.append(AWSVolume(self.provider, vol))
-        return results
+        aws_vols = self.provider.ec2_conn.get_all_volumes(filters=filtr)
+        cb_vols = [AWSVolume(self.provider, vol) for vol in aws_vols]
+        return ServerPagedResultList(aws_vols.is_truncated,
+                                     aws_vols.next_token,
+                                     False,
+                                     data=cb_vols)
 
     def create(self, name, size, zone, snapshot=None, description=None):
         """
@@ -297,8 +297,8 @@ class AWSSnapshotService(BaseSnapshotService):
         """
         snaps = [AWSSnapshot(self.provider, snap)
                  for snap in self.provider.ec2_conn.get_all_snapshots()]
-        return cbhelpers.to_result_list(self.provider, snaps, limit,
-                                        marker)
+        return ClientPagedResultList(self.provider, snaps,
+                                     limit=limit, marker=marker)
 
     def create(self, name, volume, description=None):
         """
@@ -345,8 +345,8 @@ class AWSObjectStoreService(BaseObjectStoreService):
         """
         buckets = [AWSBucket(self.provider, bucket)
                    for bucket in self.provider.s3_conn.get_all_buckets()]
-        return cbhelpers.to_result_list(self.provider, buckets, limit,
-                                        marker)
+        return ClientPagedResultList(self.provider, buckets,
+                                     limit=limit, marker=marker)
 
     def create(self, name, location=None):
         """
@@ -389,8 +389,8 @@ class AWSImageService(BaseImageService):
         """
         images = [AWSMachineImage(self.provider, image)
                   for image in self.provider.ec2_conn.get_all_images()]
-        return cbhelpers.to_result_list(self.provider, images, limit,
-                                        marker)
+        return ClientPagedResultList(self.provider, images,
+                                     limit=limit, marker=marker)
 
 
 class AWSComputeService(BaseComputeService):
@@ -550,13 +550,12 @@ class AWSInstanceService(BaseInstanceService):
         reservations = self.provider.ec2_conn.get_all_reservations(
             max_results=limit,
             next_token=marker)
-        results = BaseResultList(reservations.is_truncated,
-                                 reservations.next_token,
-                                 False)
-        for res in reservations:
-            for inst in res.instances:
-                results.append(AWSInstance(self.provider, inst))
-        return results
+        instances = [AWSInstance(self.provider, inst)
+                     for res in reservations
+                     for inst in res.instances]
+        return ServerPagedResultList(reservations.is_truncated,
+                                     reservations.next_token,
+                                     False, data=instances)
 
 AWS_INSTANCE_DATA_DEFAULT_URL = "https://swift.rc.nectar.org.au:8888/v1/" \
                                 "AUTH_377/cloud-bridge/aws/instance_data.json"
@@ -579,8 +578,8 @@ class AWSInstanceTypesService(BaseInstanceTypesService):
     def list(self, limit=None, marker=None):
         inst_types = [AWSInstanceType(self.provider, inst_type)
                       for inst_type in self.instance_data]
-        return cbhelpers.to_result_list(self.provider, inst_types, limit,
-                                        marker)
+        return ClientPagedResultList(self.provider, inst_types,
+                                     limit=limit, marker=marker)
 
 
 class AWSRegionService(BaseRegionService):
@@ -600,5 +599,5 @@ class AWSRegionService(BaseRegionService):
 
         regions = [AWSRegion(self.provider, region)
                    for region in self.provider.ec2_conn.get_all_regions()]
-        return cbhelpers.to_result_list(self.provider, regions, limit,
-                                        marker)
+        return ClientPagedResultList(self.provider, regions,
+                                     limit=limit, marker=marker)

cloudbridge/cloud/providers/openstack/helpers.py

@@ -2,26 +2,35 @@
 Helper functions
 """
 import itertools
-from cloudbridge.cloud.base import BaseResultList
+from cloudbridge.cloud.base import ServerPagedResultList
 
 
-def to_result_list(provider, limit, list_func):
+def os_result_limit(provider, requested_limit):
+    """
+    Calculates the limit for openstack.
+    """
+    limit = requested_limit or provider.config.result_limit
+    # fetch one more than the limit to help with paging.
+    # i.e. if length(objects) is one more than the limit,
+    # we know that the object has another page of results,
+    # so we always request one extra record.
+    return limit + 1
+
+
+def to_server_paged_list(provider, objects, limit):
     """
     A convenience function for wrapping a list of openstack native objects in
-    a BaseResultList. The list_func is called with a custom limit to fetch the
+    a ServerPagedResultList. OpenStack
     initial list of objects. Thereafter, the return list is wrapped in a
     BaseResultList, enabling extra properties like
     `is_truncated` and `marker` to be accessed.
     """
     limit = limit or provider.config.result_limit
-    # Fetch one more than the limit, so we can
-    # detect whether is_truncated=True
-    objects = list_func(limit + 1)
     is_truncated = len(objects) > limit
-    next_token = objects[-2].id if is_truncated else None
-    results = BaseResultList(is_truncated,
-                             next_token,
-                             False)
+    next_token = objects[limit].id if is_truncated else None
+    results = ServerPagedResultList(is_truncated,
+                                    next_token,
+                                    False)
     for obj in itertools.islice(objects, limit):
         results.append(obj)
     return results

cloudbridge/cloud/providers/openstack/resources.py

@@ -722,17 +722,16 @@ class OpenStackBucket(BaseBucket):
         :rtype: BucketObject
         :return: List of all available BucketObjects within this bucket.
         """
-        def _list_container_objects(nlimit):
-            _, object_list = self._provider.swift.get_container(
-                self.name,
-                limit=nlimit, marker=marker)
-            return [OpenStackBucketObject(
-                self._provider, self, obj) for obj in object_list]
-
-        return oshelpers.to_result_list(
+        _, object_list = self._provider.swift.get_container(
+            self.name, limit=oshelpers.os_result_limit(self._provider, limit),
+            marker=marker)
+        cb_objects = [OpenStackBucketObject(
+            self._provider, self, obj) for obj in object_list]
+
+        return oshelpers.to_server_paged_list(
             self._provider,
-            limit,
-            _list_container_objects)
+            cb_objects,
+            limit)
 
     def delete(self, delete_contents=False):
         """

cloudbridge/cloud/providers/openstack/services.py

@@ -1,7 +1,6 @@
 """
 Services implemented by the OpenStack provider.
 """
-import itertools
 from cinderclient.exceptions import NotFound as CinderNotFound
 from novaclient.exceptions import NotFound as NovaNotFound
 
@@ -18,6 +17,7 @@ from cloudbridge.cloud.base import BaseSecurityGroupService
 from cloudbridge.cloud.base import BaseSecurityService
 from cloudbridge.cloud.base import BaseSnapshotService
 from cloudbridge.cloud.base import BaseVolumeService
+from cloudbridge.cloud.base import ClientPagedResultList
 from cloudbridge.cloud.interfaces.resources import InstanceType
 from cloudbridge.cloud.interfaces.resources import KeyPair
 from cloudbridge.cloud.interfaces.resources import MachineImage
@@ -81,20 +81,11 @@ class OpenStackKeyPairService(BaseKeyPairService):
         :return:  list of KeyPair objects
         """
 
-        # pylint:disable=unused-argument
-        def _list_key_pairs(nlimit):
-            keypairs = self.provider.nova.keypairs.list()
-            if marker:
-                keypairs = itertools.dropwhile(
-                    lambda kp: not kp.name == marker, keypairs)
-
-            return [OpenStackKeyPair(self.provider, kp)
-                    for kp in keypairs]
-
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            _list_key_pairs)
+        keypairs = self.provider.nova.keypairs.list()
+        results = [OpenStackKeyPair(self.provider, kp)
+                   for kp in keypairs]
+        return ClientPagedResultList(self.provider, results,
+                                     limit=limit, marker=marker)
 
     def find(self, name):
         """
@@ -136,20 +127,11 @@ class OpenStackSecurityGroupService(BaseSecurityGroupService):
         :return:  list of SecurityGroup objects
         """
 
-        # pylint:disable=unused-argument
-        def _list_security_groups(nlimit):
-            sgs = self.provider.nova.security_groups.list()
-            if marker:
-                sgs = itertools.dropwhile(
-                    lambda sg: not sg.name == marker, sgs)
-
-            return [OpenStackSecurityGroup(self.provider, sg)
-                    for sg in sgs]
+        sgs = [OpenStackSecurityGroup(self.provider, sg)
+               for sg in self.provider.nova.security_groups.list()]
 
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            _list_security_groups)
+        return ClientPagedResultList(self.provider, sgs,
+                                     limit=limit, marker=marker)
 
     def create(self, name, description):
         """
@@ -248,14 +230,13 @@ class OpenStackImageService(BaseImageService):
         """
         List all images.
         """
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            lambda nlimit:
-                [OpenStackMachineImage(self.provider, img)
-                 for img in self.provider.nova.images.list(
-                    limit=nlimit,
-                    marker=marker)])
+        cb_images = [
+            OpenStackMachineImage(self.provider, img)
+            for img in self.provider.nova.images.list(
+                limit=oshelpers.os_result_limit(self.provider, limit),
+                marker=marker)]
+
+        return oshelpers.to_server_paged_list(self.provider, cb_images, limit)
 
 
 class OpenStackInstanceTypesService(BaseInstanceTypesService):
@@ -264,14 +245,13 @@ class OpenStackInstanceTypesService(BaseInstanceTypesService):
         super(OpenStackInstanceTypesService, self).__init__(provider)
 
     def list(self, limit=None, marker=None):
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            lambda nlimit:
-                [OpenStackInstanceType(self.provider, obj)
-                 for obj in self.provider.nova.flavors.list(
-                    limit=nlimit,
-                    marker=marker)])
+        cb_itypes = [
+            OpenStackInstanceType(self.provider, obj)
+            for obj in self.provider.nova.flavors.list(
+                limit=oshelpers.os_result_limit(self.provider, limit),
+                marker=marker)]
+
+        return oshelpers.to_server_paged_list(self.provider, cb_itypes, limit)
 
 
 class OpenStackBlockStoreService(BaseBlockStoreService):
@@ -318,14 +298,13 @@ class OpenStackVolumeService(BaseVolumeService):
         """
         List all volumes.
         """
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            lambda nlimit:
-                [OpenStackVolume(self.provider, vol)
-                 for vol in self.provider.cinder.volumes.list(
-                    limit=nlimit,
-                    marker=marker)])
+        cb_vols = [
+            OpenStackVolume(self.provider, vol)
+            for vol in self.provider.cinder.volumes.list(
+                limit=oshelpers.os_result_limit(self.provider, limit),
+                marker=marker)]
+
+        return oshelpers.to_server_paged_list(self.provider, cb_vols, limit)
 
     def create(self, name, size, zone, snapshot=None, description=None):
         """
@@ -368,15 +347,13 @@ class OpenStackSnapshotService(BaseSnapshotService):
         """
         List all snapshot.
         """
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            lambda nlimit:
-                [OpenStackSnapshot(self.provider, snap) for
-                 snap in self.provider.cinder.volume_snapshots.list(
-                    search_opts={
-                        'limit': nlimit,
-                        'marker': marker})])
+        cb_snaps = [
+            OpenStackSnapshot(self.provider, snap) for
+            snap in self.provider.cinder.volume_snapshots.list(
+                search_opts={'limit': oshelpers.os_result_limit(self.provider,
+                                                                limit),
+                             'marker': marker})]
+        return oshelpers.to_server_paged_list(self.provider, cb_snaps, limit)
 
     def create(self, name, volume, description=None):
         """
@@ -419,17 +396,12 @@ class OpenStackObjectStoreService(BaseObjectStoreService):
         """
         List all containers.
         """
-        # pylint:disable=unused-argument
-        def _list_containers(nlimit):
-            _, container_list = self.provider.swift.get_account(
-                limit=nlimit, marker=marker)
-            return [OpenStackBucket(self.provider, c)
-                    for c in container_list]
-
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            _list_containers)
+        _, container_list = self.provider.swift.get_account(
+            limit=oshelpers.os_result_limit(self.provider, limit),
+            marker=marker)
+        cb_buckets = [OpenStackBucket(self.provider, c)
+                      for c in container_list]
+        return oshelpers.to_server_paged_list(self.provider, cb_buckets, limit)
 
     def create(self, name, location=None):
         """
@@ -450,25 +422,17 @@ class OpenStackRegionService(BaseRegionService):
 
     def list(self, limit=None, marker=None):
 
-        # pylint:disable=unused-argument
-        def _list_regions(nlimit):
-            regions = (
-                endpoint.get('region') or endpoint.get('region_id')
-                for svc in self.provider.keystone.service_catalog.get_data()
-                for endpoint in svc.get('endpoints', [])
-            )
-            regions = (region for region in regions if region)
-            if marker:
-                regions = itertools.dropwhile(
-                    lambda region: not region == marker, regions)
-
-            return [OpenStackRegion(self.provider, region)
-                    for region in regions]
+        regions = (
+            endpoint.get('region') or endpoint.get('region_id')
+            for svc in self.provider.keystone.service_catalog.get_data()
+            for endpoint in svc.get('endpoints', [])
+        )
+        regions = (region for region in regions if region)
+        os_regions = [OpenStackRegion(self.provider, region)
+                      for region in regions]
 
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            _list_regions)
+        return ClientPagedResultList(self.provider, os_regions,
+                                     limit=limit, marker=marker)
 
 
 class OpenStackComputeService(BaseComputeService):
@@ -613,14 +577,12 @@ class OpenStackInstanceService(BaseInstanceService):
         """
         List all instances.
         """
-        return oshelpers.to_result_list(
-            self.provider,
-            limit,
-            lambda nlimit:
-                [OpenStackInstance(self.provider, inst)
-                 for inst in self.provider.nova.servers.list(
-                    limit=nlimit,
-                    marker=marker)])
+        cb_insts = [
+            OpenStackInstance(self.provider, inst)
+            for inst in self.provider.nova.servers.list(
+                limit=oshelpers.os_result_limit(self.provider, limit),
+                marker=marker)]
+        return oshelpers.to_server_paged_list(self.provider, cb_insts, limit)
 
     def get(self, instance_id):
         """

test/test_cloud_helpers.py

@@ -1,6 +1,6 @@
 import itertools
 
-from cloudbridge.cloud import helpers as cbhelpers
+from cloudbridge.cloud.base import ClientPagedResultList
 from test.helpers import ProviderTestBase
 
 
@@ -27,18 +27,18 @@ class CloudHelpersTestCase(ProviderTestBase):
                    DummyResult(4, "Four"),
                    ]
 
-        results = cbhelpers.to_result_list(self.provider, objects, 2, None)
+        results = ClientPagedResultList(self.provider, objects, 2, None)
         self.assertListEqual(results, list(itertools.islice(objects, 2)))
         self.assertEqual(results.marker, objects[1].id)
         self.assertTrue(results.supports_total)
         self.assertEqual(results.total_results, 4)
 
-        results = cbhelpers.to_result_list(self.provider, objects, 2, 2)
+        results = ClientPagedResultList(self.provider, objects, 2, 2)
         self.assertListEqual(results, list(itertools.islice(objects, 2, 4)))
         self.assertEqual(results.marker, None)
         self.assertTrue(results.supports_total)
         self.assertEqual(results.total_results, 4)
 
-        results = cbhelpers.to_result_list(self.provider, objects, 2, 3)
+        results = ClientPagedResultList(self.provider, objects, 2, 3)
         self.assertListEqual(results, list(itertools.islice(objects, 3, 4)))
         self.assertEqual(results.marker, None)