Skip to content

Lazy pagination iterator for catalog list methods#3416

Closed
GayathriSrividya wants to merge 2 commits into
apache:mainfrom
GayathriSrividya:fix/issue-3365-lazy-pagination-iterator
Closed

Lazy pagination iterator for catalog list methods#3416
GayathriSrividya wants to merge 2 commits into
apache:mainfrom
GayathriSrividya:fix/issue-3365-lazy-pagination-iterator

Conversation

@GayathriSrividya

@GayathriSrividya GayathriSrividya commented May 26, 2026

Copy link
Copy Markdown
Contributor

Closes #3365

This PR builds on @jayceslesar's earlier work in #2172 and has been restructured to credit the original author.

Changes

Commit 1 — by @jayceslesar (rebased from #2172):

  • Changed abstract method signatures: list_tables, list_namespaces, list_views return types from list[Identifier] to Iterator[Identifier]
  • All non-REST catalogs (SQL, DynamoDB, Glue, Hive, BigQuery, NoOp) return iter() of their results
  • CLI and output updated to work with iterators
  • Tests updated accordingly

Commit 2 — by @GayathriSrividya:

  • REST catalog uses true generators with per-page HTTP fetch helpers (_fetch_tables_page, _fetch_namespaces_page, _fetch_views_page) decorated with @retry so auth retry logic works correctly per page

@Fokko

Fokko commented May 26, 2026

Copy link
Copy Markdown
Contributor

@GayathriSrividya To credit the original author, it would be good to cherry-pick his work

@GayathriSrividya
GayathriSrividya force-pushed the fix/issue-3365-lazy-pagination-iterator branch from 7b6744e to 7e1538e Compare May 27, 2026 18:06
@GayathriSrividya

GayathriSrividya commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the CI failures with follow-up commits and pushed fixes to this PR branch.
All required checks are now passing on the latest commit (aa88377).
Ready for review. Thanks!

@rambleraptor

Copy link
Copy Markdown
Collaborator

@GayathriSrividya was this based off of #2172? If it was, we should cherry-pick in the commits so that the original author shows up in this PR as well.

jayceslesar and others added 2 commits May 28, 2026 07:52
Replace the collect-then-return approach with proper generator functions
that yield results page by page. Extract per-page fetch logic into
dedicated helper methods (_fetch_tables_page, _fetch_views_page,
_fetch_namespaces_page) decorated with @Retry so authentication retries
work correctly per page.

Co-authored-by: Yuya Ebihara <ebyhry@gmail.com>
@GayathriSrividya
GayathriSrividya force-pushed the fix/issue-3365-lazy-pagination-iterator branch from aa88377 to 54d8f64 Compare May 28, 2026 02:38
@GayathriSrividya

Copy link
Copy Markdown
Contributor Author

@GayathriSrividya To credit the original author, it would be good to cherry-pick his work

Thanks for checking on this. Yes, this PR is based on @jayceslesar’s earlier work in #2172.

I restructured the branch to preserve attribution clearly:

  • The original implementation commit is authored by @jayceslesar.
  • My follow-up commit contains the REST lazy pagination changes and updates needed to bring the work forward.

I also updated the PR description to explicitly mention that this builds on @jayceslesar’s work. Let me know if there’s a better way you’d prefer me to handle attribution here.

@GayathriSrividya

Copy link
Copy Markdown
Contributor Author

@GayathriSrividya was this based off of #2172? If it was, we should cherry-pick in the commits so that the original author shows up in this PR as well.

Hi @rambleraptor and @Fokko — yes, PR #3416 was based on @jayceslesar's work from #2172. I've restructured the branch to credit them properly:

Commit 1 (authored by @jayceslesar): Changes abstract method signatures and adds iter() wrapping for all non-REST catalogs (BigQuery, DynamoDB, Glue, Hive, NoOp, SQL)
Commit 2 (authored by me): Implements proper lazy pagination generators for the REST catalog with per-page @retry-decorated helpers
All checks are passing on the latest push. Ready for review. Thanks!

@github-actions

Copy link
Copy Markdown

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

@github-actions github-actions Bot added the stale label Jun 30, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions Bot closed this Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement iterator to lazily go through the paged response

4 participants