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

Refactored refresh token #408

Merged
merged 4 commits into from
Oct 25, 2018
Merged

Refactored refresh token #408

merged 4 commits into from
Oct 25, 2018

Conversation

manveerxyz
Copy link
Contributor

@manveerxyz manveerxyz commented Oct 25, 2018

I moved the whole refresh functionality to spotify_tools.py as it seemed like a better option versus having spotdl.py meddle with spotify_tools.spotify

EDIT: Solves Issue #237

@manveerxyz
Copy link
Contributor Author

Also, can someone try testing the refresh functionality on their machine, I keep getting:

'Spotify' object has no attribute 'get_cached_token'

Even though their documentation clearly has the function.

@ritiek
Copy link
Member

ritiek commented Oct 25, 2018

Ah crud, my bad. The methods get_cached_token and refresh_access_token are available only on spotipy.oauth2.SpotifyOAuth class and not on spotipy.oauth2.SpotifyClientCredentials class. Since, we don't need access to personal information of our token user, it doesn't make much sense to use spotipy.oauth2.SpotifyOAuth class for authorizing ourselves as it further requires verification of redirect url.

Let's keep the way it currently generates new token.

However, I like the idea of creating a new function in spotify_tools.py and be done with just calling something like spotify_tools.refresh_token() which overrides the previous spotify instance in spotify_tools.py. I'd accept this PR if you could just make this changes. :)

@ritiek ritiek mentioned this pull request Oct 25, 2018
6 tasks
Kept refresh_token() in spotify_tools.py
Copy link
Member

@ritiek ritiek left a comment

Choose a reason for hiding this comment

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

Merging this in. Thanks!

@ritiek ritiek merged commit 94dc27a into spotDL:master Oct 25, 2018
@ritiek ritiek added this to the v1.1.0 milestone Nov 13, 2018
@ritiek ritiek mentioned this pull request Nov 13, 2018
6 tasks
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.

None yet

2 participants