Fix Paginator dropping results on empty pages (DECO-27280)#819
Closed
renaudhartert-db wants to merge 1 commit into
Closed
Fix Paginator dropping results on empty pages (DECO-27280)#819renaudhartert-db wants to merge 1 commit into
renaudhartert-db wants to merge 1 commit into
Conversation
The Paginator stopped iterating on the first page that returned zero items, even when that page carried a non-empty next_page_token. Token- paginated calls such as tables().list(...) could silently return no results when leading pages were empty but more results lived on later pages. flipNextPage now continues past empty pages and terminates only when nextPageFn returns null (or the response is null), establishing the contract: nextPageFn returns null exactly when there are no more pages. Offset-based pagination (the SCIM IAM list APIs and redash 1-based list APIs) previously relied on the Paginator stopping on an empty page and never returned null, so the offset lambdas are updated to return null on an empty page; otherwise they would loop forever. These generated files are produced by the SDK codegen template, which is updated in databricks-eng/universe-dev#11878; the edits here are reproduced on the next regeneration. Adds PaginatorTest covering empty leading/interleaved pages, offset termination, forward-progress fetch counts, the absence of a built-in loop guard, and error propagation. Co-authored-by: Isaac
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. |
Contributor
|
Please ensure that the NEXT_CHANGELOG.md file is updated with any relevant changes. |
Contributor
Author
|
Closing as duplicate: #818 |
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
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.
The Paginator stopped iterating on the first page that returned zero items, even when that page carried a non-empty next_page_token. As a result, token-paginated calls such as tables().list(...) could silently return no results when the leading pages were empty but the real rows lived on later pages. This was reported in DECO-27280, where the Python SDK walked 99 pages (97 of them empty but with valid tokens) and returned 23 tables, while the Java SDK gave up on the first empty page and returned nothing.
flipNextPage now continues past empty pages and terminates only when nextPageFn returns null (or the response is null). This establishes a single contract for all pagination styles: nextPageFn returns null exactly when there are no more pages.
Offset-based pagination needed a matching change. The SCIM IAM list APIs and the redash 1-based list APIs relied on the Paginator stopping as soon as a page was empty, and their nextPageFn never returned null. Under the new behavior they would loop forever, because the offset (or page number) does not advance on a zero-item page. Their lambdas now return null on an empty page so they terminate explicitly. These files are generated by the SDK codegen template; the template is fixed in databricks-eng/universe-dev#11878, so the next regeneration reproduces the same guards. The generated edits are included here so this change is correct and mergeable on its own, in either order relative to the codegen change.
Adds PaginatorTest, which had no coverage before. It exercises empty leading and interleaved pages, offset-style termination, per-page fetch counts (forward progress with no redundant fetches), the fact that the loop has no built-in cap and depends entirely on nextPageFn returning null, error propagation through the loop, and the null-initial-request path used by legacy empty-request APIs.
This pull request and its description were written by Isaac.