Skip to content

Commit d0fd744

Browse files
authored
Improvements for credentials refresh under high load (#658)
1 parent 92187c9 commit d0fd744

File tree

2 files changed

+35
-8
lines changed

2 files changed

+35
-8
lines changed

Diff for: gcsfs/credentials.py

+32-8
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import textwrap
66
import threading
77
import warnings
8+
from datetime import datetime
89

910
import google.auth as gauth
1011
import google.auth.compute_engine
@@ -16,6 +17,8 @@
1617
from google.oauth2.credentials import Credentials
1718
from google_auth_oauthlib.flow import InstalledAppFlow
1819

20+
from gcsfs.retry import HttpError
21+
1922
logger = logging.getLogger("gcsfs.credentials")
2023

2124
tfile = os.path.join(os.path.expanduser("~"), ".gcs_tokens")
@@ -99,7 +102,6 @@ def _connect_cloud(self):
99102
raise ValueError("Invalid gcloud credentials") from error
100103

101104
def _connect_cache(self):
102-
103105
if len(self.tokens) == 0:
104106
raise ValueError("No cached tokens")
105107

@@ -167,19 +169,41 @@ def _connect_token(self, token):
167169
if self.credentials.valid:
168170
self.credentials.apply(self.heads)
169171

170-
def maybe_refresh(self):
171-
# this uses requests and is blocking
172+
def _credentials_valid(self, refresh_buffer):
173+
return (
174+
self.credentials.valid
175+
# In addition to checking current validity, we ensure that there is
176+
# not a near-future expiry to avoid errors when expiration hits.
177+
and self.credentials.expiry
178+
and (self.credentials.expiry - datetime.utcnow()).total_seconds()
179+
> refresh_buffer
180+
)
181+
182+
def maybe_refresh(self, refresh_buffer=300):
183+
"""
184+
Check and refresh credentials if needed
185+
"""
172186
if self.credentials is None:
173187
return # anon
174-
if self.credentials.valid:
175-
return # still good
188+
189+
if self._credentials_valid(refresh_buffer):
190+
return # still good, with buffer
191+
176192
with requests.Session() as session:
177193
req = Request(session)
178194
with self.lock:
179-
if self.credentials.valid:
180-
return # repeat to avoid race (but don't want lock in common case)
195+
if self._credentials_valid(refresh_buffer):
196+
return # repeat check to avoid race conditions
197+
181198
logger.debug("GCS refresh")
182-
self.credentials.refresh(req)
199+
try:
200+
self.credentials.refresh(req)
201+
except gauth.exceptions.RefreshError as error:
202+
# Re-raise as HttpError with a 401 code and the expected message
203+
raise HttpError(
204+
{"code": 401, "message": "Invalid Credentials"}
205+
) from error
206+
183207
# https://github.com/fsspec/filesystem_spec/issues/565
184208
self.credentials.apply(self.heads)
185209

Diff for: gcsfs/retry.py

+3
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ def is_retriable(exception):
7171
"""Returns True if this exception is retriable."""
7272

7373
if isinstance(exception, HttpError):
74+
# Add 401 to retriable errors when it's an auth expiration issue
75+
if exception.code == 401 and "Invalid Credentials" in str(exception.message):
76+
return True
7477
return exception.code in errs
7578

7679
return isinstance(exception, RETRIABLE_EXCEPTIONS)

0 commit comments

Comments
 (0)