-
Notifications
You must be signed in to change notification settings - Fork 448
Comments
Support storage-credentials in REST catalog LoadTableResult#3042
Open
rcjverhoef wants to merge 3 commits intoapache:main from
Open
Support storage-credentials in REST catalog LoadTableResult #3042rcjverhoef wants to merge 3 commits intoapache:main from
rcjverhoef wants to merge 3 commits intoapache:main from
Conversation
@rcjverhoef
rcjverhoef
force-pushed
the
rcjverhoef/storage-creds-support
branch
from
February 13, 2026 14:47
2864ab7 to
b2cd702
Compare
Fokko
pushed a commit
that referenced
this pull request
Feb 15, 2026
# Rationale for this change While reviewing some open PRs #3041, #3042, I noticed CI kept failing on the Python 3.13 job in the hive tests. Turns out [CPython 3.13.12 ](https://docs.python.org/3.13/whatsnew/changelog.html#id3)was just released and included a change python/cpython#142651 which made `Mock.call_count` thread-safe by deriving it from `len(call_args_list)`. This broke our hive test, which was resetting the counter with `mock.call_count = 0` directly. This switches to use reset_mock(), which properly clears all the internal call tracking state. ## Are these changes tested? `make test` passes ## Are there any user-facing changes? no
The Iceberg REST spec's LoadTableResult includes a storage-credentials field for vended credentials (prefix-scoped temporary STS tokens). PyIceberg was only reading the config field and silently dropping storage-credentials, so vended credentials never reached the FileIO. Per the spec: "Clients must first check whether the respective credentials exist in the storage-credentials field before checking the config for credentials." This adds: - storage_credentials field to TableResponse - Longest-prefix credential resolution (mirroring Java's S3FileIO) - Merging resolved credentials into FileIO with highest precedence Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rcjverhoef
rcjverhoef
force-pushed
the
rcjverhoef/storage-creds-support
branch
from
February 23, 2026 19:46
b3dab54 to
bc9bacb
Compare
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.
Rationale for this change
Vended credentials don't work with REST catalogs following latests specs. Per latest spec:
This is missing in pyiceberg. REST catalogs that don't expose a config field are hence not compatible.
Are these changes tested?
Yes, ran locally against my REST catalog implementation, added tests.
Are there any user-facing changes?
No API changes. REST catalogs that return vended credentials via storage-credentials will now work correctly.