Repository navigation
Conversation
Explicitly request pagination for configured page sizes and preserve blank query values in SigV4 signing.
This branch has not been deployed
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.
Closes #4096
Rationale for this change
When
rest-page-sizeis set, the first list request sendspageSizewithoutpageToken. Per the REST spec, servers return all results in one response whenpageTokenis not set, so the page size is ignored, and some servers (e.g.tabulario/iceberg-rest:1.6.0) fail on the request.list_tables,list_viewsandlist_namespacesnow send an emptypageTokenon the first request whenrest-page-sizeis set:Requests without
rest-page-sizeare unchanged. The SigV4 adapter now passeskeep_blank_values=Truetoparse_qsl, so the signed query includespageToken=.Are these changes tested?
Yes. Tests in
tests/catalog/test_rest.pycover the empty initial token for all three list APIs, a two-pagelist_tablesrun, and blank query values reaching the SigV4 signer.Also tested manually against
tabulario/iceberg-rest:1.6.0:list_namespaces()withrest-page-size=1previously raisedNumberFormatExceptionand now returns all namespaces across pages.Are there any user-facing changes?
With
rest-page-sizeset, the first list request includespageToken=. Without it, nothing changes.