Added OAuth2 class, remaining API endpoints and some docs for contribution #159
Conversation
renamed all instances of client_access_token to access_token
modified OAuth2 to make it user-friendlier
added missing commas
This reverts commit d2e99bb.
changed 'all' scope type to tuple, fixed combining scopes (changed ',' to ' ')
added tests
added docs for contributing
|
@johnwmillr, this PR is ready to be merged. |
johnwmillr
left a comment
johnwmillr
left a comment
There was a problem hiding this comment.
I left a comment about using OAuth properly in the unit tests. Are there extra steps to go through to get OAuth working with the tests?
|
The OAuth2 can't really be tested since it requires user input. But we could use a mock request just to test that it returns the correct result. |
|
I added the tests for the |
johnwmillr
left a comment
johnwmillr
left a comment
There was a problem hiding this comment.
I added the tests for the
OAuth2class and theutilsfunctions. But now the tests require three more env vars:GENIUS_CLIENT_ID,GENIUS_CLIENT_SECRET,GENIUS_REDIRECT_URI.
Excellent, thank you! I have your OAuth2 tests working locally now and will add the new environment vars for Travis-CI.
| if response.status_code == 200: | ||
| return response.json()['response'] | ||
| res = response.json() | ||
| return res['response'] if "response" in res else res |
There was a problem hiding this comment.
I often prefer using the get method directly when a key may not be present, but what you've written is fine.
return res.get("response", res)There was a problem hiding this comment.
return res.get("response", res) is actually better. Please replace it with that. Thanks.
|
@allerter, I'm not sure why the commits from your PRs have been getting squashed, but I just now turned off the "Allow merge commits" option for this repository. Hopefully that resolves the issue. |
2 participants
This PR completes the support of LyricsGenius of all the API endpoints available. I've also added the OAuth2 class with its docs, examples, and tests. I double if anyone will be using the endpoints that require a user access token, but it's nice to have them available just in case.
I also added some text to
contributing.rstso that users who want to contribute can check the styling of their code before opening their PR.I think it's also important to note that I renamed all instances of
GENIUS_CLIENT_ACCESS_TOKENTOGENIUS_ACCESS_TOKENto avoid confusion since client access token is just one of the two kinds of tokens available: client access tokens and user tokens. The tests will also need a user token will all permissions token to be able to test the methods that are already implemented and the new ones that require a user access token with sufficient permission.