spotipy-dev / spotipy

A light weight Python library for the Spotify Web API
http://spotipy.readthedocs.org
MIT License
4.97k stars 956 forks source link

Invalidate cache if the credentials change (or the credentials become invalid) #1149

Open vitorbaptista opened 1 month ago

vitorbaptista commented 1 month ago

Is your feature request related to a problem? Please describe.

Spotify blocked one of my API keys. I noticed this when I was running a simple Spotipy example (just searching an artist name), and Spotipy failed after retrying 3 times. I changed the credentials on my Spotipy script, but the failure was the same. After ~2 hours of debugging, I realized that Spotipy was using the .cache with the old key so, after deleting this file, it used my new key and everything worked.

Describe the solution you'd like

I see two potential solutions:

  1. Include a hash of the credentials in the cache. If this hash changes, invalidate the cache. This feels correct.
  2. Invalidate the cache if the credentials become invalid. This is already done for timeouts, but AFAIK it isn't done in the case where the API key is blocked by Spotify.

Describe alternatives you've considered

It wasn't clear that the issue was with my credentials. I just ran Spotipy and it took a while to run (because it was retrying 3x), and then failed with a 429 Too Many Requests error. A clearer error message would have made my life easier.

dieser-niko commented 1 month ago

Your first potential solution would be effective and quite simple to implement. While reading up to this point, I was thinking of something similar.

I'll start working on a new CacheHandler, this shouldn't be too hard.

dieser-niko commented 1 month ago

So currently there's something similar, but for usernames. Basically the username is appended to the end of the filename.

I'll probably go down a similar route, but reduce the hash length to 7 chars.

I also thought about hijacking the username parameter as it's marked as deprecated, but that just feels wrong if this is supposed to be official :)

dieser-niko commented 1 month ago

After some internal discussions with @stephanebruckert, we came to the conclusion that this change should be kept for the v3 release, as it would be somewhat difficult and unstable to implement a solution that is also backwards compatible. Also, this problem does not only affect the CacheFileHandler, but pretty much any other cache handler that has some kind of name attribute.