rambleraptor commented on code in PR #3454:
URL: https://github.com/apache/iceberg-python/pull/3454#discussion_r3391477215


##########
pyiceberg/typedef.py:
##########
@@ -211,3 +213,95 @@ def __hash__(self) -> int:
 
 TableVersion: TypeAlias = Literal[1, 2, 3]
 ViewVersion: TypeAlias = Literal[1]
+
+
+class PaginationList(list[T]):
+    """A list that lazily fetches subsequent pages from a paginated API.
+
+    The first page is pre-loaded on construction.  Subsequent pages are only
+    fetched when the caller iterates past items already in memory.  Operations
+    that require the complete result set — ``len()``, ``in``, slicing,
+    ``repr()`` — trigger a full fetch of all remaining pages.
+
+    Args:
+        first_page: Items from the first API response.
+        next_page_token: Pagination token returned with the first response,
+            or ``None`` if no further pages exist.
+        fetch_next_page: Callable that accepts a page token and returns a
+            tuple of ``(items, next_page_token_or_None)``.
+    """
+
+    def __init__(
+        self,
+        first_page: list[T],
+        next_page_token: str | None,
+        fetch_next_page: Callable[[str], tuple[list[T], str | None]],

Review Comment:
   Can we add the return type on this Callable? Just want to make sure that 
future paginated methods are able to adhere to this
   
   (This looks great and this is a very big nit)



##########
tests/integration/test_rest_catalog.py:
##########
@@ -110,3 +112,42 @@ def test_load_view_with_table_ident(
     assert catalog.table_exists(table_identifier)
     with pytest.raises(NoSuchViewError):
         catalog.load_view(table_identifier)
+
+
[email protected]
+def test_list_tables_returns_pagination_list(table_schema_nested: Schema, 
database_name: str, table_list: list[str]) -> None:

Review Comment:
   Love this!
   
   Again, another big nit. Could we see how many HTTP calls are being made? 
We're assuming that the REST Fixture is giving the proper number of pages with 
`rest-page-size`, but if that's not true, then this could hide issues.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to