openverse-api icon indicating copy to clipboard operation
openverse-api copied to clipboard

Remove `get_api_exception` and replace with built in DRF exceptions

Open sarayourfriend opened this issue 3 years ago • 2 comments

Description

The bespoke get_api_exception function recreates built in DRF functionality and makes searching for particular exceptions more difficult. Instead of searching for raise NotFound or raise <built-in-DRF-exception>, the typical way to find code that explicitly raises exceptions, you'd have to know about the get_api_exception function and search for usages with the specific HTTP status code (which has to be hand written by the user each time, meaning they have to remember what the status code is).

It also obscures stack traces because all errors created by get_api_exception have the same name (only solvable by further meta programming using the type constructor instead of a closed class): https://sentry.io/share/issue/abf49297eb2749e889eff15e4d37e151/

In short: we can eliminate code by removing this, be more in line with default DRF functionality and the community.

  1. Replace all usages of get_api_exception function with the appropriate built-in DRF exception from rest_framework.exceptions. For any that are following the try: queryset.get(); except Model.DoesNotExist: ... pattern, replace it with get_object_or_404.
  2. Remove the get_api_exception function

sarayourfriend avatar Aug 02 '22 13:08 sarayourfriend

Hello @sarayourfriend, I am new to open source and I would like to work on this.

AbhiYHub avatar Aug 02 '22 21:08 AbhiYHub

Hi @AbhiYHub! That's great, thank you for your willingness to contribute :slightly_smiling_face: I've assigned the issue to you. If you have any questions here or run into issues during implementation, please feel free to ping me or @WordPress/openverse-maintainers for help.

sarayourfriend avatar Aug 02 '22 22:08 sarayourfriend

Hello @sarayourfriend !! I am looking to start my journey in Open Source projects. Could you please assign this issue to me?

abhi2296 avatar Sep 10 '22 13:09 abhi2296

Hello @sarayourfriend and @dhruvkb, Apologies for the delay. I have solved the issue and also ran the test cases. All the test cases have passed successfully. I would request if you can reassign me the task so that I could submit the solution.

AbhiYHub avatar Oct 07 '22 19:10 AbhiYHub

@AbhiYHub please push your solution as a PR. Thanks!

dhruvkb avatar Oct 07 '22 19:10 dhruvkb