Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated property usage #628

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

dbalabka
Copy link

@dbalabka dbalabka commented Jun 27, 2024

In the scope of #627, I found that gcsfs uses a deprecated approach to check the token validity. Using self.credentials.token_state == google.auth.credentials.TokenState.FRESH allows to refresh the token a bit earlier using a threshold:

# The smallest MDS cache used by this library stores tokens until 4 minutes from
# expiry.
REFRESH_THRESHOLD = datetime.timedelta(minutes=3, seconds=45)

Important! This change requires v2.26.0

Now, a token might have 3 states:

Tracks the state of a token.
FRESH: The token is valid. It is not expired or close to expired, or the token has no expiry.
STALE: The token is close to expired, and should be refreshed. The token can be used normally.
INVALID: The token is expired or invalid. The token cannot be used for a normal operation.

In a nutshell, FRESH means that a token has more than 3 minutes and 45 seconds till the real expiration time.

@martindurant
Copy link
Member

This change requires v2.26.0

Since that version is pretty new, we should support both ways rather than force everyone to upgrade. Can you please link where the new behaviour was introduced upstream, please (or a link to TokenState's doc) ?

@dbalabka
Copy link
Author

dbalabka commented Jun 27, 2024

@martindurant , here is a PR: googleapis/google-auth-library-python#1368

I'm not sure they have documentation yet. Also, let's skip this change for now unless it is required. I assumed it would fix my issue with a token refresh (see #627), but it seems it is not related. Hence, we can close this PR unmerged

@martindurant
Copy link
Member

let's skip this change for now unless it is required

I'm happy to leave it open for others to see, and we'll probably need it eventually.

@dbalabka dbalabka changed the title Remove deprecated poperty usage Remove deprecated property usage Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants