feat: implement vended credential refresh by gabeiglio · Pull Request #3751 · apache/iceberg-python

gabeiglio · 2026-08-04T11:51:54Z

Third time's a charm (prev PR closed due to staleness) :) Closes #3506

Rationale for this change

Implement automatic vended credential refresh in PyIceberg.

Are these changes tested?

Yes,

catalog/test_credential_provider.py test the VendedCredential class
test_rest.py tests the attach logic and overall refresh feature
io/test_credential_provider.py tests caching and locking

Are there any user-facing changes?

Yes, users will have the option to set the refresh credentials configurations in the catalog

gabeiglio · 2026-08-05T10:42:01Z

@kevinjqliu I opened this PR as the previous one was closed for being stale. Here I've hopefully addressed your comments!

CredentialsProvider now accepts a refresh_fn (passed in by the catalog) to fetch new credentials when needed.
FileIO now caches filesystems by credentials too, so a cache hit can no longer return a filesystem built with expired credentials.
CredentialsProvider owns the credential resolution logic, and the longest-prefix-match against storage credentials.
CredentialsProvider exposes a per-scheme hook so refresh logic can be implemented individually per scheme (currently only implemented for S3).

dossett · 2026-08-13T20:41:05Z

+1 (non-binding). GCS support would be pretty to add to this.

rambleraptor

I've got a couple concerns around refresh intervals + pickling.

Thanks a lot for getting this started! Excited to get this merged in.

rambleraptor · 2026-08-18T21:08:00Z

            catalog=self,
        )

+    def _attach_credentials_provider(


Can we rename this to _attach_credentials_provider_to_io?

rambleraptor · 2026-08-18T21:40:32Z

+        longest-prefix matching against the freshly vended credentials.
+        """
+        if storage_credentials and property_as_bool(self.properties, REFRESH_CREDENTIALS_ENABLED, False):
+            io.set_credentials_provider(


This breaks pickling support on FileIO. We can't pickle lambdas.

We have some tests for pickling FileIO, but none of them break because this is behind REFRESH_CREDENTIALS_ENABLE

Good catch, this is a real blocker for this. If we want to keep the concern of refreshing the credentials by calling the catalog.load_credentials then we cannot reliable pickle this object. Specially this we would need to pickle AuthManagers and its locks and it would become a very fragile and prone to break.

I bellieve we should re-visit allowing the credential provider make the API call itself so there is no reference to any lambdas or catalog object dependency

cc: @kevinjqliu thoughts?

What if we just drop the lock when pickling? When the object is unpickled, it's a brand new instance anyways. It shouldn't have the same lock to begin with.

Dropping and recreating the lock makes sense. My concern is that the refresh lambda still captures the catalog instance, and the catalog contains additional state like sessions, auth managers, and possibly other locks that may not be pickleable.

So we’d still need to make the refresh call independent of the catalog or reconstruct it after unpickling/

rambleraptor · 2026-08-18T21:45:50Z

+
+    def _can_refresh(self, location: str) -> bool:
+        scheme = urlparse(location).scheme
+        refresh_by_scheme = self._needs_refresh_by_scheme.get(scheme)


If the TTL on a S3 token is < 300s (the default value), we'll attempt to refresh on every call. That's a lot of possibly unnecessary refreshes.

It seems like we should track when the last refresh was and use that information to determine when we should next refresh.

Java does something similar in scheduleCredentialRefresh

IIUC your comment, we wont refresh on every call, since the lock will hold until the refresh completes and all other threads will not reach the load_credentials

rambleraptor · 2026-08-18T21:48:33Z

+        The refresh callback returns the full LoadCredentialsResponse so the provider can re-run
+        longest-prefix matching against the freshly vended credentials.
+        """
+        if storage_credentials and property_as_bool(self.properties, REFRESH_CREDENTIALS_ENABLED, False):


We should check that the server has access to the LoadCredentials endpoint. Otherwise, it'll probably throw an error.

(If you're setting this property, your catalog probably has the proper endpoints, but we should verify anyways)

the self._load_credentials() method performs the check

dossett · 2026-08-19T17:11:25Z

+    return best_match.config if best_match else {}
+
+
+class CredentialsProvider:


I think this class ultimately needs to less S3-specific and delegate some functionality into scheme-specific handlers. I don't think that has to be part of this PR though, it could be part of a following that adds support for other schemes, such as GCS.

Overall, I agree. I think we'll have a better idea of the shape of this class once we implement GCS.

gabeiglio · 2026-08-26T09:04:36Z

Thanks for the reviews! I will make the changes this week been AFK

rambleraptor · 2026-09-21T21:21:07Z

@gabeiglio super excited for this! Anything I can do to help out?

mrcnc · 2026-09-25T21:20:26Z

I'm excited for this too! I have some long-running PyIceberg readers and will be happy to use this when it lands 🚀

github-actions · 2026-10-26T00:29:52Z

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

dossett · 2026-10-26T13:19:10Z

@gabeiglio if you're not able to continue with this, I'd be happy to give it a shot.

gabeiglio · 2026-10-26T13:25:11Z

sorry about the silence here, @dossett feel free to take this further!

dossett · 2026-10-26T13:29:00Z

@gabeiglio Thank you! would you mind giving me write access to this PR branch? That would let me keep everything together here. But no worries, if you'd rather me open a new PR.

gabeiglio · 2026-10-26T13:46:50Z

done sent the invite to my fork

kevinjqliu · 2026-10-26T16:15:57Z

this got dropped from my radar... i'll take a look!

dossett · 2026-10-26T20:44:28Z

I pushed a few commits, hopefully broken up in a sensible way.

My local integration testing confirms this is working for GCS. I can't easily live test S3 at the moment.

ef1018e4 - address serializability concerns
dad1f8dd - refactors the original approach a bit and includes GCS support directly in this PR
1931e168 - wires in the refresh framework to PyArrow and fsspec operations
49a26ded - documentation

dossett · 2026-10-27T13:15:21Z

@kevinjqliu @rambleraptor feedback welcome on the changes

rambleraptor

Thanks so much for your work on this! I've got a few comments for you. Happy to take another review.

My biggest concern is just making sure that we don't break existing users with this.

rambleraptor · 2026-10-27T20:06:49Z

+        if (expiration := self.expiration_ms(properties)) is not None:
+            for key in self.required_properties:
+                if not properties.get(key):
+                    raise ValueError(f"Missing vended credential property: {key}")


This is being called on every credential, even if refresh is off.

It doesn't appear that this happens in Java. Even if it did, this would potentially break users who are using credentials currently with refresh off. We should make sure that this only works for refresh, since the property acts as a feature flag for us automatically.

rambleraptor · 2026-10-27T20:17:02Z

+                    now_ms = int(time() * 1000)
+                    states = _credential_states(response.storage_credentials, now_ms)
+                    refreshed = _matching_state(states, location)
+                    if refreshed is None:


If the refresh fails, this will immediately throw a ValueError.

Here's the rough idea of how Java handles this:

Credential due for refresh, but hasn't expired yet

Refreshing threw an error of some kind

If the old credential is still valid, we should use it and not immediately surface the failed refresh to the user

In this situation, we should suppress the exception, log a warning, and use the old credential.

rambleraptor · 2026-10-27T20:18:26Z

+Retained file handles resolve current credentials when opened or created again. An already-open native stream keeps its original credentials;
+its lifetime should fit within the credential lifetime. External engines that manage their own storage clients must manage their own credential refresh.
+
+FileIO instances remain pickleable with refresh enabled. Deserialization performs no network requests; catalog access and authentication


I'm not sure if we need to keep this last paragraph. This seems like a code behavior and not super useful for users.

rambleraptor · 2026-10-27T20:20:07Z

+            result.storage_credentials,
+            location,
+            identifier=self.table_identifier,
+            existing_io=self.io,


Can you plumb through the plan_id as well? We'll want that to eventually get over to the load_credentials endpoint

dossett · 2026-10-27T21:23:04Z

@rambleraptor Thanks very much! Your comments make sense and I pushed some commits to address the separately (for readability)

gabeiglio force-pushed the vended-credentials-provider branch from 350dc82 to 010d6bc Compare August 5, 2026 10:20

gabeiglio marked this pull request as ready for review August 5, 2026 10:41

rambleraptor suggested changes Aug 18, 2026

View reviewed changes

dossett reviewed Aug 19, 2026

View reviewed changes

Fokko self-requested a review September 1, 2026 18:29

github-actions Bot added the stale label Oct 26, 2026

feat: implement vended credential refresh

dc32974

dossett force-pushed the vended-credentials-provider branch from 010d6bc to dc32974 Compare October 26, 2026 14:17

dossett added 4 commits October 26, 2026 09:25

Make REST credential refresh survive FileIO serialization

ef1018e

Share credential refresh scheduling across S3 and GCS

dad1f8d

Refresh vended credentials across S3 and GCS file operations

1931e16

Document S3 and GCS vended credential refresh

49a26de

kevinjqliu self-requested a review October 26, 2026 16:15

github-actions Bot removed the stale label Oct 27, 2026

rambleraptor suggested changes Oct 27, 2026

View reviewed changes

dossett added 2 commits October 27, 2026 15:52

Preserve credential configuration when refresh is disabled

b495575

Reuse valid credentials when an early refresh fails

ed38456

dossett added 2 commits October 27, 2026 16:05

Preserve scan plan context when refreshing credentials

7762e27

Keep credential refresh docs focused on worker requirements

072a909

		return best_match.config if best_match else {}


		class CredentialsProvider:

Conversation

gabeiglio commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

Are these changes tested?

Are there any user-facing changes?

Uh oh!

gabeiglio commented Aug 5, 2026

Uh oh!

dossett commented Aug 13, 2026

Uh oh!

rambleraptor left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabeiglio commented Aug 26, 2026

Uh oh!

rambleraptor commented Sep 21, 2026

Uh oh!

mrcnc commented Sep 25, 2026

Uh oh!

github-actions Bot commented Oct 26, 2026

Uh oh!

dossett commented Oct 26, 2026

Uh oh!

gabeiglio commented Oct 26, 2026

Uh oh!

dossett commented Oct 26, 2026

Uh oh!

gabeiglio commented Oct 26, 2026

Uh oh!

kevinjqliu commented Oct 26, 2026

Uh oh!

dossett commented Oct 26, 2026

Uh oh!

dossett commented Oct 27, 2026

Uh oh!

rambleraptor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dossett commented Oct 27, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

gabeiglio commented Aug 4, 2026 •

edited

Loading

rambleraptor left a comment •

edited

Loading