omarryhan/aiogoogle

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')

Hey @neel004, if you pass a download_file parameter it will save the binary in a file. If you want to stream the binary response, you can use the pipe_to param, you can find some examples in this issue #98.

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.