Repository navigation
fix: page backward in auto_paging_iter after a before cursor #745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| Generic, | ||
| Iterator, | ||
| List, | ||
| Literal, | ||
| Optional, | ||
| TypeVar, | ||
| ) | ||
|
|
@@ -20,6 +21,9 @@ | |
|
|
||
| T = TypeVar("T", bound=Deserializable) | ||
|
|
||
| PaginationDirection = Literal["forward", "backward"] | ||
| """Direction a page paginates in: ``forward`` follows ``after``, ``backward`` follows ``before``.""" | ||
|
|
||
|
|
||
| @dataclass(slots=True) | ||
| class ListMetadata: | ||
|
|
@@ -42,6 +46,7 @@ class SyncPage(Generic[T]): | |
| _fetch_page: Optional[Callable[..., "SyncPage[T]"]] = field( | ||
| default=None, repr=False | ||
| ) | ||
| _direction: PaginationDirection = field(default="forward", repr=False) | ||
|
|
||
| @property | ||
| def before(self) -> Optional[str]: | ||
|
|
@@ -58,15 +63,27 @@ def has_more(self) -> bool: | |
| return self.after is not None | ||
|
|
||
| def auto_paging_iter(self) -> Iterator[T]: | ||
| """Iterate through all items across all pages.""" | ||
| """Iterate through all items across all pages. | ||
|
|
||
| Follows the page's direction: forward pages keep fetching with | ||
| ``after``; a page first requested with a ``before`` cursor keeps | ||
| fetching with ``before`` and yields each page's items reversed. | ||
| """ | ||
| page = self | ||
| backward = page._direction == "backward" | ||
| while True: | ||
| yield from page.data | ||
| items = reversed(page.data) if backward else page.data | ||
| yield from items | ||
| if not page.data: | ||
| break | ||
| if not page.has_more() or page._fetch_page is None: | ||
| break | ||
| page = page._fetch_page(after=page.after) | ||
| if backward: | ||
| if page.before is None or page._fetch_page is None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Knowledge Base Used: Pagination and shared types Prompt To Fix With AIThis is a comment left during a code review.
Path: src/workos/_pagination.py
Line: 80
Comment:
**Events pagination stops early** `list_events(before=...)` returns pagination metadata with only an `after` cursor. This new backward branch requires `page.before`, so it reverses the first page and stops without fetching further results. The async iterator has the same behavior.
**Knowledge Base Used:** [Pagination and shared types](https://app.greptile.com/workos/-/custom-context/knowledge-base/workos/workos-python/-/docs/pagination-and-shared-types.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||
| break | ||
| page = page._fetch_page(before=page.before) | ||
| else: | ||
| if not page.has_more() or page._fetch_page is None: | ||
| break | ||
| page = page._fetch_page(after=page.after) | ||
|
|
||
| def __iter__(self) -> Iterator[T]: | ||
| """Iterate through all items across all pages.""" | ||
|
|
@@ -82,6 +99,7 @@ class AsyncPage(Generic[T]): | |
| _fetch_page: Optional[Callable[..., Awaitable["AsyncPage[T]"]]] = field( | ||
| default=None, repr=False | ||
| ) | ||
| _direction: PaginationDirection = field(default="forward", repr=False) | ||
|
|
||
| @property | ||
| def before(self) -> Optional[str]: | ||
|
|
@@ -98,16 +116,28 @@ def has_more(self) -> bool: | |
| return self.after is not None | ||
|
|
||
| async def auto_paging_iter(self) -> AsyncIterator[T]: | ||
| """Iterate through all items across all pages.""" | ||
| """Iterate through all items across all pages. | ||
|
|
||
| Follows the page's direction: forward pages keep fetching with | ||
| ``after``; a page first requested with a ``before`` cursor keeps | ||
| fetching with ``before`` and yields each page's items reversed. | ||
| """ | ||
| page = self | ||
| backward = page._direction == "backward" | ||
| while True: | ||
| for item in page.data: | ||
| items = reversed(page.data) if backward else page.data | ||
| for item in items: | ||
| yield item | ||
| if not page.data: | ||
| break | ||
| if not page.has_more() or page._fetch_page is None: | ||
| break | ||
| page = await page._fetch_page(after=page.after) | ||
| if backward: | ||
| if page.before is None or page._fetch_page is None: | ||
| break | ||
| page = await page._fetch_page(before=page.before) | ||
| else: | ||
| if not page.has_more() or page._fetch_page is None: | ||
| break | ||
| page = await page._fetch_page(after=page.after) | ||
|
|
||
| def __aiter__(self) -> AsyncIterator[T]: | ||
| """Iterate through all items across all pages.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
order="normal"with abeforecursor, results are documented as descending even thoughbeforefetches older records. This reversal yields each page in ascending order instead, so sync and async iteration no longer preserve the requested order.Knowledge Base Used: Pagination and shared types
Prompt To Fix With AI