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

Oauth refresh on request #403

Merged
merged 11 commits into from
Jul 3, 2023

Conversation

zeitmeister
Copy link
Contributor

This fix should update so that for every request to ytmusic the code checks if the access token needs refreshing and refreshes it if that's the case.

However, the headers object are updated on every request, regardless if the token needs refreshing or not. This is probably not cpu heavy by any means but should perhaps be looked at in another iteration. or when i have more time...

@sigma67
Copy link
Owner

sigma67 commented Jun 25, 2023

You can easily fix this by extending __init__ so that we know if this is an is_oauth instance (set a new instance variable like is_browser_auth). Then simply check for the value of that variable. Also you could simply call load_headers directly since we know then that it's an oauth instance.

@zeitmeister
Copy link
Contributor Author

Thank you for your comments. I think i managed to come up with something that works better. I also updated the prepare_headers function in headers.py so that they use the same YTMusicOAuth instance.

Copy link
Owner

@sigma67 sigma67 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is pretty close to a good solution, although still some optimizations to be had

ytmusicapi/auth/headers.py Outdated Show resolved Hide resolved
ytmusicapi/ytmusic.py Outdated Show resolved Hide resolved
ytmusicapi/ytmusic.py Outdated Show resolved Hide resolved
ytmusicapi/ytmusic.py Outdated Show resolved Hide resolved
@zeitmeister
Copy link
Contributor Author

I hope that the last fix makes the test run. I also don't know why these commits with merges keeps coming. they seem to be from another fork or something. git wouldn't let me push if i didn't pull stuff that i didn't even change in the first place..

anyway, thanks again for your comments. I think they really improved the funcionality.

@sigma67
Copy link
Owner

sigma67 commented Jul 3, 2023

This does it. Thanks for the persistence. No worry about the commits, they get squashed anyway

@sigma67 sigma67 merged commit ef312d7 into sigma67:master Jul 3, 2023
sigma67 pushed a commit that referenced this pull request Jul 14, 2024
* updating token on every request if auth is oauth

* remove print line

* headers only updated if auth is oauth

* some refactoring

* dont know what happened here

* check for self.auth

---------

Co-authored-by: simon <[email protected]>
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