-
Notifications
You must be signed in to change notification settings - Fork 50
Fix invalid UUID breaking related images query #766
Conversation
Signed-off-by: Olga Bulat <obulat@gmail.com>
API Developer Docs Preview: Ready https://wordpress.github.io/openverse-api/_preview/766 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good fix. We should still tighten up the lookup regex for good measure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. +1 for what Dhruv said. Also, do you know why something that looks like a valid UUID (follows the pattern) doesn't cause the 500? Why doesn't it throw an IndexError as well like the same-length-but-not-UUID-pattern string does when queried for?
@sarayourfriend, actually, that's not the case, but it took me some time to figure it out :) |
Ah!! I forgot that UUID had rules 😅 Good catch! Sorry for the false lead 😱 |
It was actually a great opportunity to learn more about what's happening when we search for something 👍 |
Fixes
Related to #729 by @AetherUnbound
Description
This PR uses @krysal 's suggestion to add an IndexError handling for when there are no
hits
in the search controller:openverse-api/api/catalog/api/controllers/search_controller.py
Line 344 in 7414174
Testing Instructions
On main, if you go to
http://localhost:8000/v1/images/000000000000000000000000000000000000/related/
, you would get a 500 error.On this branch, you should get a 404.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin