Repository: meltano/sdk
Pull request #1918 (2023-08-18 17:29:35)

feat(taps): End RESTStream pagination if an empty page is received


<!-- readthedocs-preview meltano-sdk start -->
----
:books: Documentation preview :books:: https://meltano-sdk--1918.org.readthedocs.build/en/1918/

<!-- readthedocs-preview meltano-sdk end -->

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/meltano/sdk/pull/1918?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano) Report
> Merging [#1918](https://app.codecov.io/gh/meltano/sdk/pull/1918?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano) (5ff76ed) into [main](https://app.codecov.io/gh/meltano/sdk/commit/efca24f7a439d934f6655eb08cb99d2cbee5e3a7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano) (efca24f) will **decrease** coverage by `0.03%`.
> Report is 1 commits behind head on main.
> The diff coverage is `62.50%`.

```diff
@@            Coverage Diff             @@
##             main    #1918      +/-   ##
==========================================
- Coverage   86.95%   86.93%   -0.03%     
==========================================
  Files          59       59              
  Lines        5084     5097      +13     
  Branches      823      824       +1     
==========================================
+ Hits         4421     4431      +10     
- Misses        466      469       +3     
  Partials      197      197              
```


| [Files Changed](https://app.codecov.io/gh/meltano/sdk/pull/1918?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano) | Coverage Δ | |
|---|---|---|
| [singer\_sdk/streams/rest.py](https://app.codecov.io/gh/meltano/sdk/pull/1918?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano#diff-c2luZ2VyX3Nkay9zdHJlYW1zL3Jlc3QucHk=) | `85.53% <62.50%> (-1.31%)` | :arrow_down: |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/meltano/sdk/pull/1918/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano)

codspeed-hq[bot]: ## [CodSpeed Performance Report](https://codspeed.io/meltano/sdk/branches/feat/short-circuit-no-records-in-page)

### Merging #1918 will **not alter performance**

<sub>Comparing <code>feat/short-circuit-no-records-in-page</code> (7a2fbf7) with <code>main</code> (80e31a2)</sub>



### Summary

`✅ 1` untouched benchmarks






Patch:
diff --git a/docs/guides/pagination-classes.md b/docs/guides/pagination-classes.md
index 89771242..c77ef579 100644
--- a/docs/guides/pagination-classes.md
+++ b/docs/guides/pagination-classes.md
@@ -83,11 +83,6 @@ class can be used to handle this pattern.
 
 from singer_sdk.pagination import BaseOffsetPaginator
 
-class MyPaginator(BaseOffsetPaginator):
-    def has_more(self, response):
-        data = response.json()
-        return data.get("has_more", False)
-
 
 class MyStream(RESTStream):
     def get_new_paginator(self):
diff --git a/singer_sdk/pagination.py b/singer_sdk/pagination.py
index 23874076..5bf55ca4 100644
--- a/singer_sdk/pagination.py
+++ b/singer_sdk/pagination.py
@@ -337,19 +337,6 @@ class SimpleHeaderPaginator(BaseAPIPaginator[t.Optional[str]]):
 class BasePageNumberPaginator(BaseAPIPaginator[int], metaclass=ABCMeta):
     """Paginator class for APIs that use page number."""
 
-    @abstractmethod
-    def has_more(self, response: Response) -> bool:
-        """Override this method to check if the endpoint has any pages left.
-
-        Args:
-            response: API response object.
-
-        Returns:
-            Boolean flag used to indicate if the endpoint has more pages.
-
-        """
-        ...
-
     def get_next(self, response: Response) -> int | None:  # noqa: ARG002
         """Get the next page number.
 
@@ -383,18 +370,6 @@ class BaseOffsetPaginator(BaseAPIPaginator[int], metaclass=ABCMeta):
         super().__init__(start_value, *args, **kwargs)
         self._page_size = page_size
 
-    @abstractmethod
-    def has_more(self, response: Response) -> bool:
-        """Override this method to check if the endpoint has any pages left.
-
-        Args:
-            response: API response object.
-
-        Returns:
-            Boolean flag used to indicate if the endpoint has more pages.
-        """
-        ...
-
     def get_next(self, response: Response) -> int | None:  # noqa: ARG002
         """Get the next page offset.
 
diff --git a/singer_sdk/streams/rest.py b/singer_sdk/streams/rest.py
index f8dbeadc..a9cfa568 100644
--- a/singer_sdk/streams/rest.py
+++ b/singer_sdk/streams/rest.py
@@ -384,6 +384,7 @@ class RESTStream(Stream, t.Generic[_TToken], metaclass=abc.ABCMeta):
         """
         paginator = self.get_new_paginator()
         decorated_request = self.request_decorator(self._request)
+        pages = 0
 
         with metrics.http_request_counter(self.name, self.path) as request_counter:
             request_counter.context = context
@@ -396,7 +397,19 @@ class RESTStream(Stream, t.Generic[_TToken], metaclass=abc.ABCMeta):
                 resp = decorated_request(prepared_request, context)
                 request_counter.increment()
                 self.update_sync_costs(prepared_request, resp, context)
-                yield from self.parse_response(resp)
+                records = iter(self.parse_response(resp))
+                try:
+                    first_record = next(records)
+                except StopIteration:
+                    self.logger.info(
+                        "Pagination stopped after %d pages because no records were "
+                        "found in the last response",
+                        pages,
+                    )
+                    break
+                yield first_record
+                yield from records
+                pages += 1
 
                 paginator.advance(resp)
 


Test patch:
diff --git a/tests/core/rest/test_pagination.py b/tests/core/rest/test_pagination.py
index 09e9d04b..6ef34f13 100644
--- a/tests/core/rest/test_pagination.py
+++ b/tests/core/rest/test_pagination.py
@@ -4,9 +4,10 @@ from __future__ import annotations
 
 import json
 import typing as t
+from urllib.parse import parse_qs, urlparse
 
 import pytest
-from requests import Response
+from requests import PreparedRequest, Response
 
 from singer_sdk.helpers.jsonpath import extract_jsonpath
 from singer_sdk.pagination import (
@@ -20,6 +21,10 @@ from singer_sdk.pagination import (
     SinglePagePaginator,
     first,
 )
+from singer_sdk.streams.rest import RESTStream
+
+if t.TYPE_CHECKING:
+    from singer_sdk.tap_base import Tap
 
 
 def test_paginator_base_missing_implementation():
@@ -47,26 +52,6 @@ def test_single_page_paginator():
     assert paginator.count == 1
 
 
-def test_paginator_page_number_missing_implementation():
-    """Validate that `BasePageNumberPaginator` implementation requires `has_more`."""
-
-    with pytest.raises(
-        TypeError,
-        match="Can't instantiate abstract class .* '?has_more'?",
-    ):
-        BasePageNumberPaginator(1)
-
-
-def test_paginator_offset_missing_implementation():
-    """Validate that `BaseOffsetPaginator` implementation requires `has_more`."""
-
-    with pytest.raises(
-        TypeError,
-        match="Can't instantiate abstract class .* '?has_more'?",
-    ):
-        BaseOffsetPaginator(0, 100)
-
-
 def test_paginator_hateoas_missing_implementation():
     """Validate that `BaseHATEOASPaginator` implementation requires `get_next_url`."""
 
@@ -352,3 +337,63 @@ def test_paginator_custom_hateoas():
     paginator.advance(response)
     assert paginator.finished
     assert paginator.count == 3
+
+
+def test_break_pagination(tap: Tap, caplog: pytest.LogCaptureFixture):
+    class MyAPIStream(RESTStream[int]):
+        """My API stream."""
+
+        name = "my-api-stream"
+        url_base = "https://my.api.test"
+        path = "/path/to/resource"
+        schema = {"type": "object", "properties": {"id": {"type": "integer"}}}  # noqa: RUF012
+
+        def parse_response(self, response: Response) -> t.Iterable[dict]:
+            return response.json()
+
+        def get_new_paginator(self) -> BasePageNumberPaginator:
+            return BasePageNumberPaginator(1)
+
+        def get_url_params(
+            self,
+            context: dict | None,  # noqa: ARG002
+            next_page_token: int | None,
+        ) -> dict[str, t.Any] | str:
+            params = {}
+            if next_page_token:
+                params["page"] = next_page_token
+            return params
+
+        def _request(
+            self,
+            prepared_request: PreparedRequest,
+            context: dict | None,  # noqa: ARG002
+        ) -> Response:
+            r = Response()
+            r.status_code = 200
+
+            parsed = urlparse(prepared_request.url)
+            query = parse_qs(parsed.query)
+
+            if query.get("page", ["1"]) == ["1"]:
+                r._content = json.dumps(
+                    [
+                        {"id": 1},
+                        {"id": 2},
+                    ]
+                ).encode()
+            else:
+                r._content = json.dumps([]).encode()
+
+            return r
+
+    stream = MyAPIStream(tap=tap)
+
+    records_iter = stream.request_records(context=None)
+
+    next(records_iter)
+    next(records_iter)
+    with pytest.raises(StopIteration):
+        next(records_iter)
+
+    assert "Pagination stopped after 1 pages" in caplog.text