Repository: planetlabs/planet-client-python
Pull request #622 (2022-06-30 00:53:46)

limit option: remove None as a possibility, value of zero means no limit


Patch:
diff --git a/planet/cli/options.py b/planet/cli/options.py
index 408cfed..d3057f4 100644
--- a/planet/cli/options.py
+++ b/planet/cli/options.py
@@ -19,7 +19,7 @@ limit = click.option(
     type=int,
     default=100,
     show_default=True,
-    help="""Maximum number of results to return. A value of zero or None means
-    all results (a potentially large number) are returned.""")
+    help="""Maximum number of results to return. When set to 0, no maximum is
+        applied.""")
 
 pretty = click.option('--pretty', is_flag=True, help='Format JSON output.')
diff --git a/planet/clients/data.py b/planet/clients/data.py
index e4547d5..38d6fa5 100644
--- a/planet/clients/data.py
+++ b/planet/clients/data.py
@@ -97,14 +97,12 @@ class DataClient:
         """
         return await self._session.request(request)
 
-    async def quick_search(
-            self,
-            item_types: typing.List[str],
-            search_filter: dict,
-            name: str = None,
-            sort: str = None,
-            limit: typing.Union[int,
-                                None] = 100) -> typing.AsyncIterator[dict]:
+    async def quick_search(self,
+                           item_types: typing.List[str],
+                           search_filter: dict,
+                           name: str = None,
+                           sort: str = None,
+                           limit: int = 100) -> typing.AsyncIterator[dict]:
         """Execute a quick search.
 
         Quick searches are saved for a short period of time (~month). The
@@ -141,7 +139,8 @@ class DataClient:
             sort: Field and direction to order results by. Valid options are
             given in SEARCH_SORT.
             name: The name of the saved search.
-            limit: Maximum number of items to return.
+            limit: Maximum number of results to return. When set to 0, no
+                maximum is applied.
 
         Returns:
             Returns an iterator over all items matching the search.
@@ -151,10 +150,6 @@ class DataClient:
         """
         url = f'{self._base_url}/quick-search'
 
-        # Set no limit
-        if limit == 0:
-            limit = None
-
         # TODO: validate item_types
         request_json = {'filter': search_filter, 'item_types': item_types}
         if name:
@@ -238,12 +233,10 @@ class DataClient:
         response = await self._do_request(request)
         return response.json()
 
-    async def list_searches(
-            self,
-            sort: str = 'created desc',
-            search_type: str = 'any',
-            limit: typing.Union[int,
-                                None] = 100) -> typing.AsyncIterator[dict]:
+    async def list_searches(self,
+                            sort: str = 'created desc',
+                            search_type: str = 'any',
+                            limit: int = 100) -> typing.AsyncIterator[dict]:
         """List all saved searches available to the authenticated user.
 
         NOTE: the term 'saved' is overloaded here. We want to list saved
@@ -254,7 +247,8 @@ class DataClient:
         Parameters:
             sort: Field and direction to order results by.
             search_type: Search type filter.
-            limit: Maximum number of items to return.
+            limit: Maximum number of results to return. When set to 0, no
+                maximum is applied.
 
         Returns:
             An iterator over all searches that match filter.
@@ -309,16 +303,15 @@ class DataClient:
         resp = await self._do_request(req)
         return resp.json()
 
-    async def run_search(
-            self,
-            search_id: str,
-            limit: typing.Union[int,
-                                None] = 100) -> typing.AsyncIterator[dict]:
+    async def run_search(self,
+                         search_id: str,
+                         limit: int = 100) -> typing.AsyncIterator[dict]:
         """Execute a saved search.
 
         Parameters:
             search_id: Stored search identifier.
-            limit: Maximum number of items to return.
+            limit: Maximum number of results to return. When set to 0, no
+                maximum is applied.
 
         Returns:
             Returns an iterator over all items matching the search.
@@ -518,7 +511,8 @@ class DataClient:
         Parameters:
             asset: Description of the asset.
             delay: Time (in seconds) between polls.
-            max_attempts: Maximum number of polls. Set to zero for no limit.
+            max_attempts: Maximum number of polls. When set to 0, no limit
+                is applied.
             callback: Function that handles state progress updates.
 
         Returns:
diff --git a/planet/clients/orders.py b/planet/clients/orders.py
index db062f0..19da607 100644
--- a/planet/clients/orders.py
+++ b/planet/clients/orders.py
@@ -476,16 +476,15 @@ class OrdersClient:
 
         return current_state
 
-    async def list_orders(
-            self,
-            state: str = None,
-            limit: typing.Union[int,
-                                None] = 100) -> typing.AsyncIterator[dict]:
+    async def list_orders(self,
+                          state: str = None,
+                          limit: int = 100) -> typing.AsyncIterator[dict]:
         """Get all order requests.
 
         Parameters:
             state: Filter orders to given state.
-            limit: Limit orders to given limit.
+            limit: Maximum number of results to return. When set to 0, no
+                maximum is applied.
 
         Returns:
             User orders that match the query
@@ -496,10 +495,6 @@ class OrdersClient:
         """
         url = self._orders_url()
 
-        # Set no limit
-        if limit == 0:
-            limit = None
-
         if state:
             if state not in ORDER_STATE_SEQUENCE:
                 raise exceptions.ClientError(
diff --git a/planet/models.py b/planet/models.py
index 6006e69..5bc79b5 100644
--- a/planet/models.py
+++ b/planet/models.py
@@ -20,6 +20,7 @@ from pathlib import Path
 import random
 import re
 import string
+import typing
 
 import httpx
 from tqdm.asyncio import tqdm
@@ -268,28 +269,32 @@ def _get_random_filename(content_type=None):
 
 
 class Paged:
-    '''Asynchronous iterator over results in a paged resource from the Planet
-    server.
-    Each returned result is a json dict.
-
-    :param request: Open session connected to server
-    :type request: planet.api.http.ASession
-    :param do_request_fcn: Function for submitting a request. Takes as input
-        a planet.api.models.Request and returns planet.api.models.Response.
-    :type do_request_fcn: function
-    :param limit: Limit orders to given limit. Defaults to None
-    :type limit: int, optional
-    '''
+    """Asynchronous iterator over results in a paged resource.
+
+    Each returned result is a JSON dict.
+    """
     LINKS_KEY = '_links'
     NEXT_KEY = 'next'
     ITEMS_KEY = 'items'
 
-    def __init__(self, request, do_request_fcn, limit=None):
+    def __init__(self,
+                 request: Request,
+                 do_request_fcn: typing.Callable,
+                 limit: int = 0):
+        """
+        Parameters:
+            request: Request to send to server for first page.
+            do_request_fcn: Function for submitting a request. Must take in
+                Request object and return Response.
+            limit: Maximum number of results to return. When set to 0, no
+                maximum is applied.
+        """
         self.request = request
         self._do_request = do_request_fcn
 
-        self._pages = None
-        self._items = []
+        self._pages = self._get_pages()
+
+        self._items: typing.List[dict] = []
 
         self.i = 0
         self.limit = limit
@@ -301,23 +306,18 @@ class Paged:
     def __aiter__(self):
         return self
 
-    async def __anext__(self):
-        '''Asynchronous next.
-        :returns: next item as json
-        :rtype: dict
-        '''
+    async def __anext__(self) -> dict:
         # This was implemented because traversing _get_pages()
         # in an async generator was resulting in retrieving all the
         # pages, when the goal is to stop retrieval when the limit
         # is reached
-        if self.limit is not None and self.i >= self.limit:
+        if self.limit and self.i >= self.limit:
             raise StopAsyncIteration
 
         try:
             item = self._items.pop(0)
             self.i += 1
         except IndexError:
-            self._pages = self._pages or self._get_pages()
             page = await self._pages.__anext__()
             self._items = page[self.ITEMS_KEY]
             try:
@@ -328,7 +328,7 @@ class Paged:
 
         return item
 
-    async def _get_pages(self):
+    async def _get_pages(self) -> typing.AsyncGenerator:
         LOGGER.debug('getting first page')
         resp = await self._do_request(self.request)
         page = resp.json()


Test patch:
diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py
index 4ee7331..c77ff2b 100644
--- a/tests/unit/test_models.py
+++ b/tests/unit/test_models.py
@@ -168,18 +168,6 @@ async def test_StreamingBody_write_img(tmpdir, mocked_request, open_test_img):
     assert os.stat(filename).st_size == 527
 
 
-@pytest.fixture
-def get_pages():
-    p1 = {'_links': {'next': 'blah'}, 'items': [1, 2]}
-    p2 = {'_links': {}, 'items': [3, 4]}
-    responses = [mock_http_response(json=p1), mock_http_response(json=p2)]
-
-    async def do_get(req):
-        return responses.pop(0)
-
-    return do_get
-
-
 def test_StreamingBody_last_modified_emptyheader():
     '''This function tests the last_modified function for an empty header, by
     seeing if the last_modified is None.
@@ -225,6 +213,18 @@ def test_StreamingBody_last_modified_completeheader():
     assert output == expected
 
 
+@pytest.fixture
+def get_pages():
+    p1 = {'_links': {'next': 'blah'}, 'items': [1, 2]}
+    p2 = {'_links': {}, 'items': [3, 4]}
+    responses = [mock_http_response(json=p1), mock_http_response(json=p2)]
+
+    async def do_get(req):
+        return responses.pop(0)
+
+    return do_get
+
+
 @pytest.mark.asyncio
 async def test_Paged_iterator(get_pages):
     req = MagicMock()
@@ -233,14 +233,15 @@ async def test_Paged_iterator(get_pages):
 
 
 @pytest.mark.asyncio
-async def test_Paged_limit(get_pages):
+@pytest.mark.parametrize('limit, expected', [(0, [1, 2, 3, 4]), (1, [1])])
+async def test_Paged_limit(limit, expected, get_pages):
     req = MagicMock()
-    paged = models.Paged(req, get_pages, limit=3)
-    assert [1, 2, 3] == [i async for i in paged]
+    paged = models.Paged(req, get_pages, limit=limit)
+    assert [i async for i in paged] == expected
 
 
 @pytest.mark.asyncio
-async def test_break_page_cycle():
+async def test_Paged_break_page_cycle():
     """Check that we break out of a page cycle."""
 
     async def func(req):
@@ -255,3 +256,23 @@ async def test_break_page_cycle():
 
     with pytest.raises(PagingError):
         [item async for item in paged]
+
+
+@pytest.mark.skip('not implemented')
+@pytest.mark.asyncio
+async def test_Paged_first_page_error():
+    """Check that we identify an error thrown by the first page upon
+    initialization."""
+
+    async def func(req):
+        return mock_http_response(404,
+                                  json={
+                                      '_links': {
+                                          'next': 'blah'
+                                      },
+                                      'items': [1, 2]
+                                  })
+
+    req = MagicMock()
+    with pytest.raises(Exception):
+        models.Paged(req, func, limit=None)