Просмотр исходного кода

Make the transport page size configurable, as an AWS setting

The page size was a bare module constant, which made it the odd one out:
default_result_limit sits beside it in the same call and is configurable,
as are the wait timeout and interval.

Read it from an aws_page_size config value, defaulting to 500. It is
deliberately an AWS setting rather than one on the shared Configuration:
it only means anything where the provider walks pages itself, and boto3's
paginator is the only one that does. GCP, Azure and OpenStack each return
a single page plus a continuation token and let the caller drive, so page
size and result limit coincide there by design and there is nothing for a
larger page to amplify. Putting it on the abstract Configuration would
have promised a cross-provider setting that three of four providers
cannot honour.

Clamp to the declared minimum as well as the maximum. A configured value
is no safer than a default: several EC2 describes require at least 5 and
DescribeRouteTables permits at most 100, and falling outside either is a
hard InvalidParameterValue rather than a clamp. Tests cover a configured
value being used, and being pulled back to both bounds.
Nuwan Goonasekera 7 часов назад
Родитель
Сommit
e9202fc887
3 измененных файлов с 70 добавлено и 39 удалено
  1. 16 4
      CHANGELOG.rst
  2. 29 19
      cloudbridge/providers/aws/helpers.py
  3. 25 16
      tests/test_aws_pagination.py

+ 16 - 4
CHANGELOG.rst

@@ -10,10 +10,22 @@
   tiny increments: the same filtered ``describe_images`` took 977.6s at a
   page size of 5 and 10.0s at 1000, for one result either way. ``MaxItems``
   still bounds what the caller receives; ``PageSize`` is now a full page,
-  clamped to whatever ceiling the service model declares for the operation
-  (``DescribeRouteTables`` allows 100 where most allow more, and exceeding a
-  ceiling is a hard ``InvalidParameterValue``). Since ``DEFAULT_RESULT_LIMIT``
-  is 50, every paginated AWS call was affected, not just filtered searches.
+  clamped to whatever bounds the service model declares for the operation
+  (``DescribeRouteTables`` permits 100 where most permit more, several require
+  at least 5, and falling outside them is a hard ``InvalidParameterValue``).
+  Since ``DEFAULT_RESULT_LIMIT`` is 50, every paginated AWS call was affected,
+  not just filtered searches.
+
+## Enhancements
+* **New ``aws_page_size`` configuration value.** How many records to request
+  from AWS per call while satisfying a list method, defaulting to 500. It is
+  a transport setting, distinct from ``default_result_limit``, which bounds
+  how many results the caller receives; the two used to be the same number.
+  It is clamped to what the service permits for the call in hand, so a value
+  outside those bounds is adjusted rather than rejected. AWS-specific
+  because it only means anything where the provider walks pages itself:
+  GCP, Azure and OpenStack each return a single page plus a continuation
+  token and let the caller drive.
 * **``AWSImageService.find`` no longer scans every public image to run its
   tag search.** ``find(label=...)`` issues two ``describe_images`` calls, one
   filtered on ``name`` and one on ``tag:Name``, and neither was scoped by

+ 29 - 19
cloudbridge/providers/aws/helpers.py

@@ -25,12 +25,16 @@ log = logging.getLogger(__name__)
 
 T = TypeVar("T")
 
-# How many items to ask the service for per request, independent of how many
-# the caller wants back. Small enough to sit inside every EC2 describe's
-# documented ceiling that the service model does not declare (DescribeVolumes
-# is the tightest of those, at 500), large enough that a sparse filtered scan
-# does not turn into thousands of round trips. Operations that do declare a
-# ceiling are clamped to it - see BotoEC2Service._page_size.
+# How many records to request per call while satisfying a list method,
+# overridable per provider with the ``aws_page_size`` config value. This is a
+# transport setting and is deliberately not ``default_result_limit``, which
+# bounds how many results the caller receives - see
+# BotoGenericService._page_size for why conflating them is expensive.
+#
+# AWS-specific because it only means anything where the provider walks pages
+# itself. GCP, Azure and OpenStack each return a single page plus a
+# continuation token and let the caller drive, so there is nothing for a page
+# size to amplify; boto3's paginator is the odd one out.
 DEFAULT_PAGE_SIZE = 500
 
 
@@ -161,7 +165,7 @@ class BotoGenericService(object):
         else:
             return None
 
-    def _page_size(self, client: Any, list_op: str, limit: int) -> int:
+    def _page_size(self, client: Any, list_op: str) -> int:
         """
         Transport page size for a paginated call.
 
@@ -174,15 +178,16 @@ class BotoGenericService(object):
         bounding - at worst one page more data is fetched than was wanted,
         and a limit larger than a page is served over several requests.
 
-        EC2's per-operation ceilings differ - ``DescribeRouteTables`` allows
-        100 where most allow 1000 - and exceeding one is a hard
-        ``InvalidParameterValue`` rather than a clamp, so defer to whatever
-        the service model declares for this operation.
+        The size comes from the ``aws_page_size`` config value so it can be
+        tuned per provider, then is clamped to whatever the service model
+        declares for this operation. EC2's bounds differ per call -
+        ``DescribeRouteTables`` permits 100 where most permit 1000, and
+        several require at least 5 - and falling outside them is a hard
+        ``InvalidParameterValue`` rather than a clamp, so neither the default
+        nor a configured value is passed through unchecked.
         """
-        # Deliberately not max(limit, ...): a caller asking for more than a
-        # page still gets it, over several requests, rather than having an
-        # oversized limit pushed at a service that would reject it.
-        page_size = DEFAULT_PAGE_SIZE
+        page_size = int(self.provider._get_config_value(
+            'aws_page_size', DEFAULT_PAGE_SIZE))
         api_name = client.meta.method_to_api_mapping.get(list_op)
         if not api_name:
             return page_size
@@ -191,9 +196,14 @@ class BotoGenericService(object):
         max_results = input_shape.members.get('MaxResults') if input_shape \
             else None
         # Not every operation declares bounds, even where the documentation
-        # gives them; fall back to a size known to be within all of them.
-        ceiling = max_results.metadata.get('max') if max_results else None
-        return min(page_size, ceiling) if ceiling else page_size
+        # gives them; the default is within all of the undeclared ones.
+        bounds = max_results.metadata if max_results else {}
+        ceiling, floor = bounds.get('max'), bounds.get('min')
+        if ceiling:
+            page_size = min(page_size, ceiling)
+        if floor:
+            page_size = max(page_size, floor)
+        return page_size
 
     def _get_list_operation(self) -> str:
         """
@@ -239,7 +249,7 @@ class BotoGenericService(object):
         if limit:
             PaginationConfig = {
                 'MaxItems': limit,
-                'PageSize': self._page_size(client, list_op, limit)}
+                'PageSize': self._page_size(client, list_op)}
 
         if marker:
             PaginationConfig.update({'StartingToken': marker})

+ 25 - 16
tests/test_aws_pagination.py

@@ -77,33 +77,42 @@ class AWSPaginationTestCase(unittest.TestCase):
         page = self.provider.storage.volumes.list()
         self.assertLessEqual(len(page), RESULT_LIMIT)
 
+    def _page_size(self, list_op):
+        # pylint:disable=protected-access
+        return self.provider.storage.volumes.svc._page_size(
+            self.provider.ec2_conn.meta.client, list_op)
+
     def test_transport_page_size_is_independent_of_the_result_limit(self):
         # The regression this guards: PageSize tracking the result limit is
         # what made a sparse scan pathological.
-        # pylint:disable=protected-access
-        svc = self.provider.storage.volumes.svc
-        client = self.provider.ec2_conn.meta.client
-        page_size = svc._page_size(client, 'describe_volumes', RESULT_LIMIT)
+        page_size = self._page_size('describe_volumes')
         self.assertEqual(page_size, DEFAULT_PAGE_SIZE)
         self.assertNotEqual(page_size, RESULT_LIMIT)
 
     def test_page_size_is_clamped_to_the_operations_ceiling(self):
         # DescribeRouteTables allows 100 where most EC2 describes allow more,
         # and exceeding a ceiling is a hard InvalidParameterValue.
-        # pylint:disable=protected-access
-        svc = self.provider.storage.volumes.svc
-        client = self.provider.ec2_conn.meta.client
-        self.assertEqual(
-            svc._page_size(client, 'describe_route_tables', RESULT_LIMIT),
-            100)
+        self.assertEqual(self._page_size('describe_route_tables'), 100)
 
     def test_page_size_survives_an_unknown_operation(self):
-        # pylint:disable=protected-access
-        svc = self.provider.storage.volumes.svc
-        client = self.provider.ec2_conn.meta.client
-        self.assertEqual(
-            svc._page_size(client, 'not_an_operation', RESULT_LIMIT),
-            DEFAULT_PAGE_SIZE)
+        self.assertEqual(self._page_size('not_an_operation'),
+                         DEFAULT_PAGE_SIZE)
+
+    def test_configured_page_size_is_used(self):
+        self.provider.config['aws_page_size'] = 250
+        self.assertEqual(self._page_size('describe_volumes'), 250)
+
+    def test_configured_page_size_is_clamped_to_the_ceiling(self):
+        # A configured value is no safer than the default: DescribeRouteTables
+        # would reject anything above 100 outright.
+        self.provider.config['aws_page_size'] = 900
+        self.assertEqual(self._page_size('describe_route_tables'), 100)
+
+    def test_configured_page_size_is_raised_to_the_floor(self):
+        # Several EC2 describes require at least 5, so a smaller configured
+        # value would be rejected rather than honoured.
+        self.provider.config['aws_page_size'] = 1
+        self.assertEqual(self._page_size('describe_vpcs'), 5)
 
 
 if __name__ == "__main__":