CB-261: Autheticate requests to Spotify Web API #123
Conversation
default_config.py
Outdated
| @@ -59,6 +59,10 @@ | |||
| MUSICBRAINZ_CLIENT_ID = "" | |||
| MUSICBRAINZ_CLIENT_SECRET = "" | |||
|
|
|||
| # Spotify | |||
| SPOTIFY_CLIENT_ID = "EXAMPLE_CLIENT_ID" | |||
| SPOTIFY_CLIENT_SECRET = "EXAMPLE_CLIENT_SECRET" | |||
There was a problem hiding this comment.
Just set those to be empty strings.
| ).json() | ||
| access_token = access_token.get('access_token') | ||
| if not access_token: | ||
| raise Exception("Could not fetch access token for Spotify") |
There was a problem hiding this comment.
Try not to use Exception class. This makes it very inconvenient to capture since having except Exception is typically a bad idea. It's better to subclass it with something specific to this module/package.
|
|
||
|
|
||
| def get_spotify(url): | ||
| """Make a GET request to Spotify Web API. |
There was a problem hiding this comment.
This is not very accurate. It currently makes a GET request to whatever URL is provided, not just from spotify.com. One thing you can do here is instead accept a path with query string and append it to the base URL of the Spotify API.
|
|
||
| BASE_URL = "https://api.spotify.com/v1" | ||
|
|
||
|
|
||
| def fetch_access_token(): |
There was a problem hiding this comment.
Make sure to prefix functions that are not meant to be used outside of the module with _.
| url = BASE_URL + query | ||
| headers = {"Authorization": f"Bearer {access_token}"} | ||
|
|
||
| result = requests.get(f"{url}", headers=headers) |
There was a problem hiding this comment.
we should probably try being consistent in our format strings, whether this is %, .format, or f"" - my opinion is that having too many isn't a good thing. If we want to slowly move from one style to another, that's OK (I'm not sure if this has already been talked about)
In this specific case, do we actually need the format string? we can just do requests.get(url)
There was a problem hiding this comment.
You are right, we don't need it.
And no, we haven't discussed this. I think f-strings are new in Python 3, so we couldn't have used it before. This inconsistency might be partially my fault since at the beginning I used % and then started switching to .format since it allows to specify placeholder names explicitly.
Added function
fetch_access_tokenfor authenticating requests to Spotify Web API. Since, CB doesn't need access to spotify users' private data, I've usedClient Credential Flowas the authorization flow requiring only the client credentials for fetching access token.