Fix Paginator dropping results on empty token pages#818
Open
hectorcast-db wants to merge 1 commit into
Open
Conversation
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
force-pushed
the
deco-27280-paginator-empty-page
branch
from
2f28576 to
136e5a0
Compare
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
force-pushed
the
deco-27280-paginator-empty-page
branch
from
136e5a0 to
b2f9743
Compare
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
force-pushed
the
deco-27280-paginator-empty-page
branch
from
b2f9743 to
06008a3
Compare
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
changed the title
The Paginator stopped iterating at the first empty page even when the
response carried a non-empty next_page_token, silently dropping results
on later pages. Token-paginated list methods (e.g. tables().list()) can
return empty intermediate pages with a valid token, so results could be
under-returned.
Add two explicit pagination factories:
- Paginator.newTokenPagination: iterate until nextPageFn returns null;
empty pages are skipped, not treated as the end of results.
- Paginator.newOffsetPagination: stop on the first empty page (offset,
SCIM and legacy SQL APIs have no token and rely on this).
The existing Paginator constructor is kept and deprecated (it retains its
previous offset/limit behavior), so this is not a breaking change.
Regenerate all *API.java list methods to the appropriate factory. Update
hand-written APIs: SharesExtAPI -> newTokenPagination; SCIM
Users/Groups/ServicePrincipals (and Account* variants) -> newOffsetPagination.
Signed-off-by: Hector Castejon Diaz <hector.castejon@databricks.com>
hectorcast-db
force-pushed
the
deco-27280-paginator-empty-page
branch
from
06008a3 to
4ee7901
Compare
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Contributor
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix
Paginatorsilently dropping results when a token-paginated response returns an empty page that still carries anext_page_token. Adds explicitnewTokenPagination/newOffsetPaginationfactories and regenerates all list methods to use them. Backward compatible — the existing constructor is kept and deprecated.Why
Token-paginated endpoints (e.g.
tables().list()) may return empty intermediate pages with a validnext_page_token— the only reliable end-of-results signal is the token's absence.Paginatorstopped at the first empty page, so these calls under-returned results. Offset/limit APIs (SCIM, legacy SQL) have no token and do end on an empty page, so the fix is strategy-aware (a blanket "skip empty pages" would loop those forever).What changed
Paginator.newTokenPagination(...)— page untilnextPageFnreturns null; empty pages are skipped.Paginator.newOffsetPagination(...)— stop on the first empty page (previous behavior).Paginatorconstructor is kept and@Deprecated, retaining its prior offset/limit behavior — no breaking change.*API.java(91 token / 34 offset). Hand-written:SharesExtAPI→ token; SCIMUsers/Groups/ServicePrincipals+Account*→ offset.How is this tested?
PaginatorTest: token pagination walks past empty leading/intermediate pages; offset pagination still stops on the first empty page.mvn clean compileover the full regenerated SDK (3,902 sources) passes.