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

Breaking change in minor version? #828

Open
SHxKM opened this issue Jun 11, 2022 · 15 comments
Open

Breaking change in minor version? #828

SHxKM opened this issue Jun 11, 2022 · 15 comments
Labels

Comments

@SHxKM
Copy link

SHxKM commented Jun 11, 2022

Hi,

Super glad to see this maintained after a long halt.

I've noticed after upgrading from 2.4.4. to 2.19.0 that some endpoints like search no longer work when the Spotipy object is initialized with a auth param.

My old code (now returns an error):

credentials = oauth2.SpotifyClientCredentials(
        client_id=SPOTIFY_CLIENT_ID,
        client_secret=SPOTIFY_SECRET_ID,
    )
token = credentials.get_access_token()
spotipy_obj = spotipy.Spotify(auth=token)
artist_on_spotify = spotipy_obj.search(q="X", type="artist", limit=1)

The new code:

spotipy_obj = spotipy.Spotify(client_credentials_manager=credentials)
artist_on_spotify = spotipy_obj.search(q="X", type="artist", limit=1)

Meanwhile, user-related endpoints continue to work with token.

Is this expected? I couldn't find anything in the release notes regarding this. Maybe I didn't look hard enough.

Also: am I correct in saying that the cache system is only for the application's access token, not those of the application's users?

@SHxKM SHxKM added the question label Jun 11, 2022
@Peter-Schorn
Copy link
Contributor

Peter-Schorn commented Jun 13, 2022

My old code (now returns an error):

credentials = oauth2.SpotifyClientCredentials(
       client_id=SPOTIFY_CLIENT_ID,
       client_secret=SPOTIFY_SECRET_ID,
   )
token = credentials.get_access_token()
spotipy_obj = spotipy.Spotify(auth=token)
artist_on_spotify = spotipy_obj.search(q="X", type="artist", limit=1)

This will always return an error after an hour when the access token expires. The refresh token has not been saved. Therefore, this pattern is never recommended.

Follow the pattern shown on the README:

Without user authentication

import spotipy
from spotipy.oauth2 import SpotifyClientCredentials

sp = spotipy.Spotify(auth_manager=SpotifyClientCredentials(client_id="YOUR_APP_CLIENT_ID",
                                                           client_secret="YOUR_APP_CLIENT_SECRET"))

results = sp.search(q='weezer', limit=20)
for idx, track in enumerate(results['tracks']['items']):
    print(idx, track['name'])

@SHxKM
Copy link
Author

SHxKM commented Jun 13, 2022

@Peter-Schorn that code is just a simplified example. I’m not complaining that it’s changed I just want to know if it indeed was. Because it did work with 2.4.4.

@Peter-Schorn
Copy link
Contributor

Your code is flawed for any version as already described.

@SHxKM
Copy link
Author

SHxKM commented Jun 13, 2022

Your code is flawed for any version as already described.

You can repeat yourself being condescending all you want. The question remains: a code that works (even for a split second) in 2.4.4 doesn’t work at all in 2.19.0, is this breaking change intended? If you are uninterested in answering that, don’t.

I didn’t ask for a code review. I asked if something broke between versions.

@Peter-Schorn
Copy link
Contributor

Don't know.

@stephanebruckert
Copy link
Member

stephanebruckert commented Jun 13, 2022

It should probably still work and looks like a bug. It's really weird that it works for a user endpoint but not for a non-user endpoint.

Can you please share the error with stack trace and any warning spotipy might be raising?

@SHxKM
Copy link
Author

SHxKM commented Jun 15, 2022

Hi @stephanebruckert, thank you. With 2.19.0 here's the exception of type spotipy.client.SpotifyException:

http status: 400, code:-1 - https://api.spotify.com/v1/search?q=Roped+Off+Problem+%26+Boogie+artist%3AThe+Game&limit=50&offset=0&type=track&market=us:
 Only valid bearer authentication supported, reason: None

As I said, with 2.4.4 this works fine.

@stephanebruckert
Copy link
Member

stephanebruckert commented Jun 15, 2022

@SHxKM in your "old" code can you try to replace

token = credentials.get_access_token()

with

token = credentials.get_access_token(as_dict=False)

and let me know if it helps?

@SHxKM
Copy link
Author

SHxKM commented Jun 15, 2022

@SHxKM in your "old" code can you try to replace

token = credentials.get_access_token()

with

token = credentials.get_access_token(as_dict=False)

and let me know if it helps?

Yes, as_dict=False solves the issue. Can you elaborate on this please?

@stephanebruckert
Copy link
Member

Since 2.4.4, was added a new parameter as_dict.

https://github.com/plamere/spotipy/blob/06551cd0553d6e030f3f4ee7da5fac12c424bac7/spotipy/oauth2.py#L213

Looks like it should be False by default instead of True for things to remain backward-compatible

@SHxKM
Copy link
Author

SHxKM commented Jun 15, 2022

Since 2.4.4, was added a new parameter as_dict.

https://github.com/plamere/spotipy/blob/06551cd0553d6e030f3f4ee7da5fac12c424bac7/spotipy/oauth2.py#L213

Looks like it should be False by default instead of True for things to remain backward-compatible

Thank you for taking the time to look into this. I guess for now if I don't find additional issues I'll just do:

spotipy_obj = spotipy.Spotify(client_credentials_manager=credentials)
artist_on_spotify = spotipy_obj.search(q="X", type="artist", limit=1)

By the way, is the spotipy.Spotify object designed to a be a singleton? Because in some parts of my code I repetitively initialize it en-masse, and I'm wondering if that's what's leading to high memory usage.

@stephanebruckert
Copy link
Member

By the way, is the spotipy.Spotify object designed to a be a singleton?

It looks like it is, I would try to initialise it once and pass it where you need. Particularly since the tokens auto-refresh in newer versions!

@SHxKM
Copy link
Author

SHxKM commented Jun 15, 2022

By the way, is the spotipy.Spotify object designed to a be a singleton?

It looks like it is, I would try to initialise it once and pass it where you need. Particularly since the tokens auto-refresh in newer versions!

Got it. I wonder if that'll mess things up when I'm authenticating both on the app-level and the-user level. But that's for another issue. Thanks a lot @stephanebruckert!

@SHxKM SHxKM closed this as completed Jun 15, 2022
@SHxKM
Copy link
Author

SHxKM commented Jun 16, 2022

@stephanebruckert one thing we didn't discuss is why it didn't affect user-level endpoints? (those that require user authorization) Do you know the answer?

@SHxKM SHxKM reopened this Jun 16, 2022
@Peter-Schorn
Copy link
Contributor

Got it. I wonder if that'll mess things up when I'm authenticating both on the app-level and the-user level. But that's for another issue.

Spotify is not a singleton, but you should generally only be using one instance of it in your program, especially if you've only authenticated with one user. Do not authenticate separately for different endpoints. That's a waste of your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants