Token is not getting refreshed after its expired after around 1 hour
Closed this issue ยท 17 comments
Issue: I've been executing a script to fetch all the objects from cloud storage. There are around a lot of objects present in the bucket.
Observation: The script fetches half of the objects in around 1 hr and then raises the below-mentioned error.
Traceback:
Traceback (most recent call last):
File "~/aiogoogle/models.py", line 341, in _next_page_generator
prev_res = await sess.send(next_req, full_res=True)
File "~/aiogoogle/sessions/aiohttp_session.py", line 183, in send
results = await schedule_tasks()
File "~/aiogoogle/sessions/aiohttp_session.py", line 175, in schedule_tasks
return await asyncio.gather(*tasks, return_exceptions=False)
File "~/aiogoogle/sessions/aiohttp_session.py", line 157, in get_response
response.raise_for_status()
File "~/aiogoogle/models.py", line 453, in raise_for_status
raise AuthError(msg=self.reason, req=self.req, res=self)
aiogoogle.excs.AuthError:
Unauthorized
Content:
{'code': 401,
'errors': [{'domain': 'global',
'location': 'Authorization',
'locationType': 'header',
'message': 'Invalid Credentials',
'reason': 'authError'}],
'message': 'Invalid Credentials'}
Request URL:
https://storage.googleapis.com/storage/v1/b/....
Question:
Is there any other way to reset the access token present in the session_factory's send() method? or am I missing a step?
code snippet:
blobs_full_request = await google_client.as_service_account(
storage_client.objects.list(bucket=BUCKET),
full_res=True,
)
async for blob_page in blobs_full_request:
yield blob_page
Possible issue: When pagination starts, if there are more pages to go through, the package fails to look for the validity of the access_token.
Hey, thanks for the detailed ticket!
Possible issue: When pagination starts, if there are more pages to go through, the package fails to look for the validity of the access_token.
Yes, this indeed seems to be the case.
I'll be grateful if you can create a PR with a fix ๐ and I'll go ahead and create a new release once you open the PR and it's merged.
Thanks!
Hey, @omarryhan @praveen-elastic I've raised a PR which fixes this issue for service-accounts.
@omarryhan can you please go ahead and release this, meanwhile I'll raise subsequent PRs for other auth modes.
Thank you so much @neel004! I've just released v4.4.0
p.s. I think right now we're refreshing tokens on every next page request, ideally, we should check if the access token is expired first to avoid the extra network call. We probably need to work on the Auth.ServiceAccountManager.refresh method. But I guess this isn't urgent, hence why I merged and released it.
Thanks for the quick merge, I'll raise a PR once I have proper fix ready, till then we can keep the issue open.
Hey @omarryhan , on a totally different note, is there a way to get the object data in binary or encoded form? I'm encountering
this error when fetching object content using get call with alt=media as a query param.
Traceback (most recent call last):
File "/lib/python3.10/site-packages/aiogoogle/sessions/aiohttp_session.py", line 70, in resolve_response
json = await response.json()
File "/lib/python3.10/site-packages/aiohttp/client_reqrep.py", line 1104, in json
raise ContentTypeError(
aiohttp.client_exceptions.ContentTypeError: 0, message='Attempt to decode JSON with unexpected mimetype: application/msword', url=URL('https://storage.googleapis.com/storage/v1/b/{bucket}/o/{file_name}.doc?alt=media')
I think right now we're refreshing tokens on every next page request, ideally, we should check if the access token is expired first to avoid the extra network call.
@omarryhan
we do call refresh method for service account but it indeed checks the validity before going for new token[here].
Thinking about the authentication with ApiKeyManager, should the package raise the error it encounters for expired token or you want it to be handled differently?
do let me know your thoughts on this.
Ah makes sense.
I'm not sure what the current behaviour is. Also, I was under the impression that API keys never expire?
Also, I was under the impression that API keys never expire?
Ohh, Got it ๐๐ป .
I guess it's safe to close this issue now? :)
meanwhile I'll raise subsequent PRs for other auth modes.
I'm working on implementation for other authentication methods, as planned before. Once done we can close the issue ๐.
Right! My bad. Thank you for your contributions and work!
Hello @omarryhan, began work on this. Could you could assist me with this tiny problem?
According to the code for the as user
oauth2 flow, the creds will only be used for one request.
Can't we refresh the tokens for the next API calls using the configured refresh token
from the user creds
?
OR is it because we don't store the user creds
that are supplied immediately to as user
?
Sure, happy to help.
The purpose of giving the option to pass user creds for only one request is for applications/library users who are handling user creds of multiple users. It would be inconvenient if the Aiogoogle class stores the user creds in the instance's state if it will change with every call anyway.
I recall that the problem was only with pagination, no? If you set user creds as an attribute in the aiogoogle instance, then it should automatically check if the access token is expired and refresh. That only doesn't happen in pagination, right?
If you set user creds as an attribute in the aiogoogle instance, then it should automatically check if the access token is expired and refresh. That only doesn't happen in pagination, right?
@omarryhan Yes, This is the case for individual requests.
But the concern arises when the API call takes more than an hour to complete the API request(pagination..) and the token gets expires. When this happens the token gets refreshed at this point, and for refreshing the tokens the user_creds
can't be available anyhow.
So, as a solution to it, I was thinking of introducing the user_creds
as an instance variable to Oauth2Manager.
I'm happy to take any suggestion for this, Do let me know if any thing seems confusing.
Thanks for the clarification @neel004!
I think we should avoid setting user_creds to auth manager if the user didn't explicitly do so when instantiating the auth manager, otherwise, it will be useless to allow one off user_creds requests.
If it will be easy, maybe we can somehow pass the user_creds object like we pass the auth manager?
Got your concern about allowing one-off user_creds ๐๐ป .
If it will be easy, maybe we can somehow pass the user_creds object like we pass the auth manager?
๐ I'll do that.