Explicitly set the expiry time for raw token, and avoid retrying if it expires #716
Conversation
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
martindurant
reviewed
Dec 2, 2025
Comment on lines
+47
to
+48
| env_val = os.environ.get("FETCH_RAW_TOKEN_EXPIRY", "true").lower() | ||
| should_fetch_expiry = env_val not in ("false", "0", "off", "no") |
Member
There was a problem hiding this comment.
Why would we NOT attempt to refresh expired or expiring tokens?
Member
There was a problem hiding this comment.
OK, maybe I understand following your description in the PR. This would need careful documentation in the RTD prose pages.
martindurant
reviewed
Dec 2, 2025
| self.credentials.refresh(req) | ||
| except gauth.exceptions.RefreshError as error: | ||
| # There may be scenarios where this error is raised from the client side due | ||
| # to missing dependencies, especially when the client doesn't know how to refresh |
Member
There was a problem hiding this comment.
Do we need to add to our dependencies? That would be better.
Member
|
Please ping when you want me to look again |
Collaborator
|
/gcbrun |
Author
|
/gcbrun(112238b) |
1 similar comment
Collaborator
|
/gcbrun(112238b) |
Author
|
/gcbrun |
2 similar comments
Author
|
/gcbrun |
Collaborator
|
/gcbrun |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes #674
More detail about the solution can be found here - #674 (comment)
Important Note:
Short-lived raw tokens (with a lifespan of less than 5 minutes ) will not work. This is because both
google.oauth2andgcsfs/credentialsimplement a safety buffer to preemptively refresh tokens before they actually expire.google.oauth2: Uses a refresh buffer of 3 minutes 45 seconds, and consider them invalid if it's expiry is less than 3 minutes 45 seconds away.gcsfs: Uses a refresh buffer of 5 minutes, and consider it expired if expiry is less than 5 minutes far away.If a token's lifespan is shorter than these buffers, the libraries consider it effectively expired immediately. While standard credentials handle this by refreshing automatically, raw tokens cannot be refreshed, resulting in an authentication failure.
We could effectively pad the token expiry by 5 minutes to cancel out the buffer and let the token work until it completely expires. However, this will not fix the retry raw token issue: at the very edge of expiry, we might get a
401 Unauthorisedat any file using that credential, it would be hard to tell if that error is due to a raw token dying (fatal) or a general credential refresh (retriable), so keeping the buffer is actually the safer option.