Coder Social home page Coder Social logo

Comments (17)

omarryhan avatar omarryhan commented on June 12, 2024 2

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!

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024 1

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.

from aiogoogle.

omarryhan avatar omarryhan commented on June 12, 2024 1

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.

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024 1

Thanks for the quick merge, I'll raise a PR once I have proper fix ready, till then we can keep the issue open.

from aiogoogle.

omarryhan avatar omarryhan commented on June 12, 2024 1

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?

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024

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

from aiogoogle.

omarryhan avatar omarryhan commented on June 12, 2024

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.

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024

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.

from aiogoogle.

omarryhan avatar omarryhan commented on June 12, 2024

Ah makes sense.

I'm not sure what the current behaviour is. Also, I was under the impression that API keys never expire?

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024

Also, I was under the impression that API keys never expire?

Ohh, Got it 👍🏻 .

from aiogoogle.

omarryhan avatar omarryhan commented on June 12, 2024

I guess it's safe to close this issue now? :)

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024

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 😃.

from aiogoogle.

omarryhan avatar omarryhan commented on June 12, 2024

Right! My bad. Thank you for your contributions and work!

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024

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?

from aiogoogle.

omarryhan avatar omarryhan commented on June 12, 2024

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?

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024

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.

from aiogoogle.

neel004 avatar neel004 commented on June 12, 2024

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.

from aiogoogle.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.