stark256-spec commented on PR #3454: URL: https://github.com/apache/iceberg-python/pull/3454#issuecomment-5463407216
Merged latest `main` — conflicts are resolved and the PR is mergeable again. The only conflict was a new module-level `logger` in `rest/__init__.py` landing next to the `_PageFetchResult` alias; kept both. Quick status recap so this is easy to land: - **Approach**: settled on the backward-compatible `PaginationList` (per @Fokko's call in June — avoids breaking `len()`/slicing/`in`/etc. ahead of a 1.0 break). - **@Fokko's four review comments** — move type to `typedef`, `TypeAlias` for the repeated tuple, function-style tests, and the pre-commit/D105 question — are all addressed. - **@rambleraptor's** main concern (list methods backed by CPython C code — `count`/`index`/`reversed`/`copy`/`+`/`*` and the reflected variants — silently truncating multi-page results) is implemented: each fetches all remaining pages before delegating to `list`, with 13 dedicated multi-page tests. Both follow-up nits (named `FetchNextPage` alias; HTTP-call-count assertions in the integration tests) are done. He approved on Jun 16. Verified locally after the merge: `tests/utils/test_pagination.py` (36 tests) and the pagination tests in `tests/catalog/test_rest.py` all pass. @Fokko — since rambleraptor isn't a committer, this just needs your sign-off to land. Would you be able to give it a final look when you get a chance? Happy to address anything else. Thanks! -- 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]
