Repository: boto/botocore
Pull request #645 (2015-09-04 20:08:23)

Paginator docs
Add docstrings to paginator's ``paginate`` method.  It also renames the paginator class returned from client to a class that falls in line with what is in the documentation e.g. ``S3.Paginator.list_objects``.

```py
In [1]: import botocore.session

In [2]: session = botocore.session.get_session()

In [3]: client = session.create_client('s3')

In [4]: paginator = client.get_paginator('list_objects')

In [5]: paginator.__class__.__name__
Out[5]: 'S3.Paginator.list_objects'

In [6]: help(paginator.paginate)
```

I realize that I have a lot of PRs out right now and many depend on the other. This is the order in which how I would like to see them reviewed/merged:
1) https://github.com/boto/botocore/pull/642
2) https://github.com/boto/botocore/pull/643
3) This one
4) https://github.com/boto/boto3/pull/239

Or you can just review this one, which accounts for 1 through 3.

cc @jamesls @mtdowling @rayluo

jamesls: I primarily have the same question as the waiter doc PR.  I'm not sure if we want to create a new class on every single call to get_paginator.  I know do that for `session.create_client`, but I would envision a paginator/waiter within a client to be called much more than a `create_client` call.
kyleknap: Doing some performance testing with the paginators as well. Here are some preliminary results in terms of speed. I used this script:
```py
import time
import botocore.session

session = botocore.session.get_session()
s3 = session.create_client('s3')

total = 0
iterations = 5
for i in range(iterations):
    start_time = time.time()
    s3.get_paginator('list_objects')
    end_time = time.time()
    diff = end_time - start_time
    print('%s round instantiation: %s seconds' % (i, diff))
    total += diff

average = total/iterations
print('Average %f seconds' % average)
```

For the develop branch, I got these results:
```
0 round instantiation: 0.00303792953491 seconds
1 round instantiation: 3.79085540771e-05 seconds
2 round instantiation: 2.40802764893e-05 seconds
3 round instantiation: 3.19480895996e-05 seconds
4 round instantiation: 4.10079956055e-05 seconds
Average 0.000635 seconds
```

For my PR I got:
```
0 round instantiation: 0.0030620098114 seconds
1 round instantiation: 8.98838043213e-05 seconds
2 round instantiation: 6.29425048828e-05 seconds
3 round instantiation: 6.69956207275e-05 seconds
4 round instantiation: 6.00814819336e-05 seconds
Average 0.000668 seconds
```

So in terms of speed it looks like it was only affected by a little.
kyleknap: So I ran some memory leak tests similiarly to what I did for the waiter docstring pull request. Here is my results for s3 ListObjects paginator.

I ran each test five times and took the average difference between the end memory usage and the beginning memory usage (i.e. ``end - start``):

develop ``test_create_single_paginator_memory_constant``: 4.46 MB
develop ``test_create_memory_paginators_in_loop``: 1.63 MB

PR ``test_create_single_paginator_memory_constant``: 4.04 MB
PR ``test_create_memory_paginators_in_loop``: 1.93 MB

Surprisingly the memory leakage decreased for one of the tests using my PR. Similarly to the Waiter docstring PR, I do not think we need to cache the Paginator class. Thoughts?

Patch:
diff --git a/botocore/client.py b/botocore/client.py
index 995fb776b..df65c3ae4 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -28,6 +28,7 @@ from botocore.signers import RequestSigner
 from botocore.utils import CachedProperty
 from botocore.utils import get_service_module_name
 from botocore.docs.docstring import ClientMethodDocstring
+from botocore.docs.docstring import PaginatorDocstring
 
 
 logger = logging.getLogger(__name__)
@@ -398,9 +399,36 @@ class BaseClient(object):
             raise OperationNotPageableError(operation_name=operation_name)
         else:
             actual_operation_name = self._PY_TO_OP_NAME[operation_name]
-            paginator = Paginator(
+
+            # Create a new paginate method that will serve as a proxy to
+            # the underlying Paginator.paginate method. This is needed to
+            # attach a docstring to the method.
+            def paginate(self, **kwargs):
+                return Paginator.paginate(self, **kwargs)
+
+            paginator_config = self._cache['page_config'][
+                actual_operation_name]
+            # Add the docstring for the paginate method.
+            paginate.__doc__ = PaginatorDocstring(
+                paginator_name=actual_operation_name,
+                event_emitter=self.meta.events,
+                service_model=self.meta.service_model,
+                paginator_config=paginator_config,
+                include_signature=False
+            )
+
+            # Rename the paginator class based on the type of paginator.
+            paginator_class_name = str('%s.Paginator.%s' % (
+                get_service_module_name(self.meta.service_model),
+                actual_operation_name))
+
+            # Create the new paginator class
+            documented_paginator_cls = type(
+                paginator_class_name, (Paginator,), {'paginate': paginate})
+
+            paginator = documented_paginator_cls(
                 getattr(self, operation_name),
-                self._cache['page_config'][actual_operation_name])
+                paginator_config)
             return paginator
 
     def can_paginate(self, operation_name):
diff --git a/botocore/docs/docstring.py b/botocore/docs/docstring.py
index de07e83e3..5146b79d6 100644
--- a/botocore/docs/docstring.py
+++ b/botocore/docs/docstring.py
@@ -12,6 +12,7 @@
 # language governing permissions and limitations under the License.
 from botocore.docs.method import document_model_driven_method
 from botocore.docs.waiter import document_wait_method
+from botocore.docs.paginator import document_paginate_method
 from botocore.docs.bcdoc.restdoc import DocumentStructure
 
 
@@ -88,3 +89,8 @@ class ClientMethodDocstring(LazyLoadedDocstring):
 class WaiterDocstring(LazyLoadedDocstring):
     def _write_docstring(self, *args, **kwargs):
         document_wait_method(*args, **kwargs)
+
+
+class PaginatorDocstring(LazyLoadedDocstring):
+    def _write_docstring(self, *args, **kwargs):
+        document_paginate_method(*args, **kwargs)
diff --git a/botocore/docs/paginator.py b/botocore/docs/paginator.py
index 08adb674c..13e0bb226 100644
--- a/botocore/docs/paginator.py
+++ b/botocore/docs/paginator.py
@@ -13,6 +13,7 @@
 from botocore import xform_name
 from botocore.compat import OrderedDict
 from botocore.docs.utils import DocumentedShape
+from botocore.utils import get_service_module_name
 from botocore.docs.method import document_model_driven_method
 
 
@@ -38,8 +39,7 @@ class PaginatorDocumenter(object):
         for paginator_name in paginator_names:
             section.style.li(
                 ':py:class:`%s.Paginator.%s`' % (
-                    self._client.__class__.__name__,
-                    xform_name(paginator_name)))
+                    self._client.__class__.__name__, paginator_name))
             self._add_paginator(section, paginator_name)
 
     def _add_paginator(self, section, paginator_name):
@@ -48,7 +48,7 @@ class PaginatorDocumenter(object):
         # Docment the paginator class
         section.style.start_sphinx_py_class(
             class_name='%s.Paginator.%s' % (
-                self._client.__class__.__name__, xform_name(paginator_name)))
+                self._client.__class__.__name__, paginator_name))
         section.style.start_codeblock()
         section.style.new_line()
 
@@ -59,93 +59,118 @@ class PaginatorDocumenter(object):
         )
         section.style.end_codeblock()
         section.style.new_line()
-        self._add_paginate_method(section, paginator_name)
-
-    def _add_paginate_method(self, section, paginator_name):
         # Get the pagination model for the particular paginator.
         paginator_config = self._service_paginator_model.get_paginator(
             paginator_name)
+        document_paginate_method(
+            section=section,
+            paginator_name=paginator_name,
+            event_emitter=self._client.meta.events,
+            service_model=self._client.meta.service_model,
+            paginator_config=paginator_config
+        )
 
-        # Retrieve the operation model of the underlying operation.
-        operation_model = self._client.meta.service_model.operation_model(
-            paginator_name)
 
-        # Add representations of the request and response parameters
-        # we want to include in the description of the paginate method.
-        # These are parameters we expose via the botocore interface.
-        pagination_config_members = OrderedDict()
+def document_paginate_method(section, paginator_name, event_emitter,
+                             service_model, paginator_config,
+                             include_signature=True):
+    """Documents the paginate method of a paginator
+
+    :param section: The section to write to
+
+    :param paginator_name: The name of the paginator. It is snake cased.
+
+    :param event_emitter: The event emitter to use to emit events
+
+    :param service_model: The service model
+
+    :param paginator_config: The paginator config associated to a particular
+        paginator.
 
-        pagination_config_members['MaxItems'] = DocumentedShape(
-            name='MaxItems', type_name='integer',
+    :param include_signature: Whether or not to include the signature.
+        It is useful for generating docstrings.
+    """
+    # Retrieve the operation model of the underlying operation.
+    operation_model = service_model.operation_model(
+        paginator_name)
+
+    # Add representations of the request and response parameters
+    # we want to include in the description of the paginate method.
+    # These are parameters we expose via the botocore interface.
+    pagination_config_members = OrderedDict()
+
+    pagination_config_members['MaxItems'] = DocumentedShape(
+        name='MaxItems', type_name='integer',
+        documentation=(
+            '<p>The total number of items to return. If the total '
+            'number of items available is more than the value '
+            'specified in max-items then a <code>NextToken</code> '
+            'will be provided in the output that you can use to '
+            'resume pagination.</p>'))
+
+    pagination_config_members['PageSize'] = DocumentedShape(
+        name='PageSize', type_name='integer',
+        documentation='<p>The size of each page.<p>')
+
+    pagination_config_members['StartingToken'] = DocumentedShape(
+        name='StartingToken', type_name='string',
+        documentation=(
+            '<p>A token to specify where to start paginating. '
+            'This is the <code>NextToken</code> from a previous '
+            'response.</p>'))
+
+    botocore_pagination_params = [
+        DocumentedShape(
+            name='PaginationConfig', type_name='structure',
             documentation=(
-                '<p>The total number of items to return. If the total '
-                'number of items available is more than the value '
-                'specified in max-items then a <code>NextToken</code> '
-                'will be provided in the output that you can use to '
-                'resume pagination.</p>'))
-
-        pagination_config_members['PageSize'] = DocumentedShape(
-            name='PageSize', type_name='integer',
-            documentation='<p>The size of each page.<p>')
-
-        pagination_config_members['StartingToken'] = DocumentedShape(
-            name='StartingToken', type_name='string',
+                '<p>A dictionary that provides parameters to control '
+                'pagination.</p>'),
+            members=pagination_config_members)
+    ]
+
+    botocore_pagination_response_params = [
+        DocumentedShape(
+            name='NextToken', type_name='string',
             documentation=(
-                '<p>A token to specify where to start paginating. '
-                'This is the <code>NextToken</code> from a previous '
-                'response.</p>'))
-
-        botocore_pagination_params = [
-            DocumentedShape(
-                name='PaginationConfig', type_name='structure',
-                documentation=(
-                    '<p>A dictionary that provides parameters to control '
-                    'pagination.</p>'),
-                members=pagination_config_members)
-        ]
-
-        botocore_pagination_response_params = [
-            DocumentedShape(
-                name='NextToken', type_name='string',
-                documentation=(
-                    '<p>A token to resume pagination.</p>'))
-        ]
-
-        service_pagination_params = []
-
-        # Add the normal input token of the method to a list
-        # of input paramters that we wish to hide since we expose our own.
-        if isinstance(paginator_config['input_token'], list):
-            service_pagination_params += paginator_config['input_token']
-        else:
-            service_pagination_params.append(paginator_config['input_token'])
-
-        # Hide the limit key in the documentation.
-        if paginator_config.get('limit_key', None):
-            service_pagination_params.append(paginator_config['limit_key'])
-
-        # Hide the output tokens in the documentation.
-        service_pagination_response_params = []
-        if isinstance(paginator_config['output_token'], list):
-            service_pagination_response_params += paginator_config[
-                'output_token']
-        else:
-            service_pagination_response_params.append(paginator_config[
-                'output_token'])
-
-        paginate_description = (
-            'Creates an iterator that will paginate through responses '
-            'from :py:meth:`{0}.Client.{1}`.'.format(
-                self._client.__class__.__name__, xform_name(paginator_name))
-        )
-
-        document_model_driven_method(
-            section, 'paginate', operation_model,
-            event_emitter=self._client.meta.events,
-            method_description=paginate_description,
-            example_prefix='response_iterator = paginator.paginate',
-            include_input=botocore_pagination_params,
-            include_output=botocore_pagination_response_params,
-            exclude_input=service_pagination_params,
-            exclude_output=service_pagination_response_params
-        )
+                '<p>A token to resume pagination.</p>'))
+    ]
+
+    service_pagination_params = []
+
+    # Add the normal input token of the method to a list
+    # of input paramters that we wish to hide since we expose our own.
+    if isinstance(paginator_config['input_token'], list):
+        service_pagination_params += paginator_config['input_token']
+    else:
+        service_pagination_params.append(paginator_config['input_token'])
+
+    # Hide the limit key in the documentation.
+    if paginator_config.get('limit_key', None):
+        service_pagination_params.append(paginator_config['limit_key'])
+
+    # Hide the output tokens in the documentation.
+    service_pagination_response_params = []
+    if isinstance(paginator_config['output_token'], list):
+        service_pagination_response_params += paginator_config[
+            'output_token']
+    else:
+        service_pagination_response_params.append(paginator_config[
+            'output_token'])
+
+    paginate_description = (
+        'Creates an iterator that will paginate through responses '
+        'from :py:meth:`{0}.Client.{1}`.'.format(
+            get_service_module_name(service_model), xform_name(paginator_name))
+    )
+
+    document_model_driven_method(
+        section, 'paginate', operation_model,
+        event_emitter=event_emitter,
+        method_description=paginate_description,
+        example_prefix='response_iterator = paginator.paginate',
+        include_input=botocore_pagination_params,
+        include_output=botocore_pagination_response_params,
+        exclude_input=service_pagination_params,
+        exclude_output=service_pagination_response_params,
+        include_signature=include_signature
+    )


Test patch:
diff --git a/tests/cmd-runner b/tests/cmd-runner
index 3b1cb8869..7723952c7 100644
--- a/tests/cmd-runner
+++ b/tests/cmd-runner
@@ -78,6 +78,7 @@ class CommandRunner(object):
         self._session = botocore.session.get_session()
         self._clients = []
         self._waiters = []
+        self._paginators = []
 
     def run(self):
         while True:
@@ -134,6 +135,27 @@ class CommandRunner(object):
         # Frees the memory associated with all of the waiters.
         self._waiters = []
 
+    def _do_create_paginator(self, args):
+        # Args:
+        # [0] client name used in its instantiation
+        # [1] paginator name used in its instantiation
+        client = self._session.create_client(args[0])
+        self._paginators.append(client.get_paginator(args[1]))
+
+    def _do_create_multiple_paginators(self, args):
+        # Args:
+        # [0] num_paginators - Number of paginators to create in a loop
+        # [1] client name used in its instantiation
+        # [2] paginator name used in its instantiation
+        num_paginators = args[0]
+        client = self._session.create_client(args[1])
+        for _ in range(int(num_paginators)):
+            self._paginators.append(client.get_paginator(args[2]))
+
+    def _do_free_paginators(self, args):
+        # Frees the memory associated with all of the waiters.
+        self._paginators = []
+
     def _do_make_aws_request(self, args):
         # Create a client and make a number of AWS requests.
         # Args:
diff --git a/tests/functional/leak/test_resource_leaks.py b/tests/functional/leak/test_resource_leaks.py
index bc1a7b20b..fad21c0ec 100644
--- a/tests/functional/leak/test_resource_leaks.py
+++ b/tests/functional/leak/test_resource_leaks.py
@@ -82,3 +82,29 @@ class TestDoesNotLeakMemory(BaseClientDriverTest):
         self.record_memory()
         start, end = self.memory_samples
         self.assertTrue((end - start) < self.MAX_GROWTH_BYTES, (end - start))
+
+    def test_create_single_paginator_memory_constant(self):
+        self.cmd('create_paginator', 's3', 'list_objects')
+        self.cmd('free_paginators')
+        self.record_memory()
+        for _ in range(100):
+            self.cmd('create_paginator', 's3', 'list_objects')
+            self.cmd('free_paginators')
+        self.record_memory()
+        start, end = self.memory_samples
+        self.assertTrue((end - start) < self.MAX_GROWTH_BYTES, (end - start))
+
+    def test_create_memory_paginators_in_loop(self):
+        # See ``test_create_memory_clients_in_loop`` to understand why
+        # paginators are first initialized and then freed. Same reason applies.
+        self.cmd('create_multiple_paginators', '200', 's3', 'list_objects')
+        self.cmd('free_paginators')
+        self.record_memory()
+        # 500 waiters in batches of 50.
+        for _ in range(10):
+            self.cmd(
+                'create_multiple_paginators', '50', 's3', 'list_objects')
+            self.cmd('free_paginators')
+        self.record_memory()
+        start, end = self.memory_samples
+        self.assertTrue((end - start) < self.MAX_GROWTH_BYTES, (end - start))
diff --git a/tests/unit/docs/test_docstring.py b/tests/unit/docs/test_docstring.py
index 0f3073c74..01adb4af3 100644
--- a/tests/unit/docs/test_docstring.py
+++ b/tests/unit/docs/test_docstring.py
@@ -14,6 +14,7 @@ from tests import unittest, mock
 from botocore.docs.docstring import LazyLoadedDocstring
 from botocore.docs.docstring import ClientMethodDocstring
 from botocore.docs.docstring import WaiterDocstring
+from botocore.docs.docstring import PaginatorDocstring
 
 
 class MockedLazyLoadedDocstring(LazyLoadedDocstring):
@@ -83,3 +84,13 @@ class TestWaiterDocstring(unittest.TestCase):
             docstring = WaiterDocstring()
             str(docstring)
             self.assertTrue(mock_writer.called)
+
+
+class TestPaginatorDocstring(unittest.TestCase):
+    def test_use_correct_docstring_writer(self):
+        with mock.patch(
+                'botocore.docs.docstring'
+                '.document_paginate_method') as mock_writer:
+            docstring = PaginatorDocstring()
+            str(docstring)
+            self.assertTrue(mock_writer.called)
diff --git a/tests/unit/docs/test_paginator.py b/tests/unit/docs/test_paginator.py
index f2673e0fc..76e116c20 100644
--- a/tests/unit/docs/test_paginator.py
+++ b/tests/unit/docs/test_paginator.py
@@ -35,8 +35,8 @@ class TestPaginatorDocumenter(BaseDocsTest):
             'Paginators',
             '==========',
             'The available paginators are:',
-            '* :py:class:`MyService.Paginator.sample_operation`',
-            '.. py:class:: MyService.Paginator.sample_operation',
+            '* :py:class:`MyService.Paginator.SampleOperation`',
+            '.. py:class:: MyService.Paginator.SampleOperation',
             '  ::',
             '    paginator = client.get_paginator(\'sample_operation\')',
             '  .. py:method:: paginate(**kwargs)',
diff --git a/tests/unit/docs/test_service.py b/tests/unit/docs/test_service.py
index 99891bee5..51451f601 100644
--- a/tests/unit/docs/test_service.py
+++ b/tests/unit/docs/test_service.py
@@ -49,7 +49,7 @@ class TestServiceDocumenter(BaseDocsTest):
             '==========',
             'Paginators',
             '==========',
-            '.. py:class:: MyService.Paginator.sample_operation',
+            '.. py:class:: MyService.Paginator.SampleOperation',
             '  .. py:method:: paginate(**kwargs)',
             '=======',
             'Waiters',
diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py
index 38b42bcfa..33c97790f 100644
--- a/tests/unit/test_client.py
+++ b/tests/unit/test_client.py
@@ -723,6 +723,88 @@ class TestAutoGeneratedClient(unittest.TestCase):
         # but we can at least make sure it looks like a paginator.
         self.assertTrue(hasattr(paginator, 'paginate'))
 
+    def test_paginator_class_name_from_client(self):
+        pagination_config = {
+            'pagination': {
+                'TestOperation': {
+                    "input_token": "Marker",
+                    "output_token": "Marker",
+                    "more_results": "IsTruncated",
+                    "limit_key": "MaxItems",
+                    "result_key": "Users"
+                }
+            }
+        }
+        self.loader.load_service_model.side_effect = [
+            self.service_description,
+            pagination_config
+        ]
+        creator = self.create_client_creator()
+        service_client = creator.create_client('myservice', 'us-west-2')
+        paginator = service_client.get_paginator('test_operation')
+        self.assertEqual(
+            paginator.__class__.__name__,
+            'MyService.Paginator.TestOperation')
+
+    def test_paginator_help_from_client(self):
+        pagination_config = {
+            'pagination': {
+                'TestOperation': {
+                    "input_token": "Marker",
+                    "output_token": "Marker",
+                    "more_results": "IsTruncated",
+                    "limit_key": "MaxItems",
+                    "result_key": "Users"
+                }
+            }
+        }
+        self.loader.load_service_model.side_effect = [
+            self.service_description,
+            pagination_config
+        ]
+        creator = self.create_client_creator()
+        service_client = creator.create_client('myservice', 'us-west-2')
+        paginator = service_client.get_paginator('test_operation')
+        with mock.patch('sys.stdout', six.StringIO()) as mock_stdout:
+            help(paginator.paginate)
+        contents = mock_stdout.getvalue()
+        lines = [
+            ('    Creates an iterator that will paginate through responses '
+             'from :py:meth:`MyService.Client.test_operation`.'),
+            '    **Request Syntax** ',
+            '    ::',
+            '      response_iterator = paginator.paginate(',
+            "          Foo='string',",
+            "          Bar='string',",
+            '          PaginationConfig={',
+            "              'MaxItems': 123,",
+            "              'PageSize': 123,",
+            "              'StartingToken': 'string'",
+            '          }',
+            '      )',
+            '    :type Foo: string',
+            '    :param Foo: **[REQUIRED]** Documents Foo',
+            '    :type Bar: string',
+            '    :param Bar: Documents Bar',
+            '    :type PaginationConfig: dict',
+            '    :param PaginationConfig: ',
+            ('      A dictionary that provides parameters to control '
+             'pagination.'),
+            '      - **MaxItems** *(integer) --* ',
+            ('        The total number of items to return. If the total '
+             'number of items available is more than the value specified '
+             'in max-items then a ``NextToken`` will be provided in the '
+             'output that you can use to resume pagination.'),
+            '      - **PageSize** *(integer) --* ',
+            '        The size of each page.',
+            '      - **StartingToken** *(string) --* ',
+            ('        A token to specify where to start paginating. This is '
+             'the ``NextToken`` from a previous response.'),
+            '    :returns: None',
+        ]
+        for line in lines:
+            self.assertIn(line, contents)
+
     def test_can_set_credentials_in_client_init(self):
         creator = self.create_client_creator()
         credentials = Credentials(