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

Fix crash for unknown resource, add Podcast & Episode resources #134

Merged
merged 7 commits into from
Aug 20, 2020

Conversation

hithomasmorelli
Copy link
Contributor

@hithomasmorelli hithomasmorelli commented Aug 18, 2020

Fixes #132

Have so far created an empty Podcast class and linked it into Client.
Am planning to also take the opportunity to integrate some Podcast API functionality, and add a failsafe to _process_json to prevent a new "type" from causing a crash in the future (by just instantiating a Resource)

  • Include tests for bug fix and new functionality*
  • Updated documentation for new feature (docstrings)

(*a test for the failsafe would be a test that the program doesn't crash upon finding an unknown "type" field - not sure on the best way to implement this)

 - created a (so far empty) Podcast(Resource) class
 - import it in client.py and add it to Client.objects_types as "podcast"
 - update test cassette files (with the new podcast data)
 - updates existing tests (to check for the now-changed cassette data)
If a "type" is unknown, it instead instantiates the general class `Resource`
@hithomasmorelli hithomasmorelli changed the title Stop the program crashing at podcasts (or at any new API item) Stop the program crashing at podcasts (or at any new API item), add Podcast class Aug 18, 2020
@hithomasmorelli hithomasmorelli changed the title Stop the program crashing at podcasts (or at any new API item), add Podcast class Stop the program crashing at podcasts (or at any new API item), add Podcast & Episode classes Aug 19, 2020
@hithomasmorelli hithomasmorelli marked this pull request as ready for review August 19, 2020 13:16
@hithomasmorelli
Copy link
Contributor Author

Not sure how to achieve 100% coverage on this - how would we test for an unexpected resource?(!)

@browniebroke
Copy link
Owner

We could write a test specificly for the Client._process_json or Client.get_object method in here:

https://github.com/browniebroke/deezer-python/blob/master/tests/test_client.py

If we write a test for _process_json, we don't need to mock anything, but we're testing a bit implementation detail.

If we do it at the get_object level, we would need to record a cassette from a simple real resource and edit it to use a replace the resource name by something that doesn't exist.

@hithomasmorelli
Copy link
Contributor Author

I think Client._process_json would be better, as that way a cassette couldn't be accidentally overwritten.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Aug 20, 2020

Sourcery Code Quality Report (beta)

Merging this PR leaves code quality unchanged.

Quality metrics Before After Change
Complexity 0.33 0.33 0.00
Method Length 27.24 27.47 0.23 🔴
Quality 9.30 9.30 0.00
Other metrics Before After Change
Lines 1035 1140 105
Changed files Quality Before Quality After Quality Change
deezer/client.py 9.08 9.11 0.03 🔵
deezer/resources.py 9.59 9.60 0.01 🔵
tests/test_client.py 9.33 9.30 -0.03 🔴
tests/test_resources.py 8.94 8.93 -0.01 🔴

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Overall Recommendation
tests/test_client.py TestClient.test_process_json_types 0 139.43 6.99 Split out functionality

Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Let us know what you think of it by mentioning @sourcery-ai in a comment.

@browniebroke browniebroke added the enhancement New feature or request label Aug 20, 2020
@browniebroke browniebroke merged commit 04b3058 into browniebroke:master Aug 20, 2020
@browniebroke
Copy link
Owner

Thanks for the great work! 🎉

@browniebroke
Copy link
Owner

@all-contributors please add @hithomasmorelli for bug, code, ideas

@allcontributors
Copy link
Contributor

@browniebroke

I've put up a pull request to add @hithomasmorelli! 🎉

@browniebroke browniebroke changed the title Stop the program crashing at podcasts (or at any new API item), add Podcast & Episode classes Fix crash for unknown resource ,add Podcast & Episode resources Aug 20, 2020
@browniebroke browniebroke changed the title Fix crash for unknown resource ,add Podcast & Episode resources Fix crash for unknown resource, add Podcast & Episode resources Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

client.get_chart fails
2 participants