WordPress / openverse-api

The Openverse API allows programmatic access to search for CC-licensed and public domain digital media.
https://api.openverse.engineering/v1
MIT License
77 stars 51 forks source link

Add ES healthchecks to `/healthcheck/` endpoint #1047

Closed sarayourfriend closed 1 year ago

sarayourfriend commented 1 year ago

Fixes

Fixes #14 by @obulat

Description

Adds Elasticsearch status checks to the /healthcheck/ endpoint when check_es is in the query params.

Note: I chose not to implement a serializer for the query params to avoid over-complicating the implementation of what is an otherwise very straightforward request to process. If this does happen to get more complex in the future, like if we add different check_* params, a serializer might then become appropriate.

I'm leaving this as a draft to allow discussion in the issue to decide whether this approach actually makes sense for our usage of the healthcheck endpoint.

Testing Instructions

Check out the unit tests and confirm they make sense. There's not really a good way to test this practically as far as I can tell unless you do something to brick your local ES cluster. I'm not sure how to do that so I don't have any advice for this.

Checklist

[best_practices]: https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines

Developer Certificate of Origin

Developer Certificate of Origin ``` Developer Certificate of Origin Version 1.1 Copyright (C) 2004, 2006 The Linux Foundation and its contributors. 1 Letterman Drive Suite D4700 San Francisco, CA, 94129 Everyone is permitted to copy and distribute verbatim copies of this license document, but changing it is not allowed. Developer's Certificate of Origin 1.1 By making a contribution to this project, I certify that: (a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or (b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or (c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it. (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. ```
github-actions[bot] commented 1 year ago

API Developer Docs Preview: Ready

https://wordpress.github.io/openverse-api/_preview/1047

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.

sarayourfriend commented 1 year ago

Somehow I completely forgot to try running the es_check locally :facepalm: Fixed in the latest commit and I'll undraft the PR now that we've got agreement on the approach. Thanks for the early review, @AetherUnbound

sarayourfriend commented 1 year ago

@AetherUnbound Did you get that right after recreate or maybe up from a down state? At least it means the endpoint is working :slightly_smiling_face:

sarayourfriend commented 1 year ago

@dhruvkb I get green on my local machine, though :thinking:

AetherUnbound commented 1 year ago

How strange! 😮 I spun the stack up almost 10 minutes ago and it's still showing yellow locally 🤔 I'll try to leave it on for a while longer to see 🤷🏼‍♀️

Edit: 45 minutes later and it is still yellow

dhruvkb commented 1 year ago

Earlier I was thinking it might be a difference between Docker Desktop on macOS and regular Docker on Linux. But with @AetherUnbound experiencing the same behaviour on Linux, I can't think of any logical explanation for this discrepancy.

sarayourfriend commented 1 year ago

Can you clarify @dhruvkb if this is a blocker for getting a second approval on this PR? From my perspective just because the local environment doesn't return "green" doesn't mean that the changes in this PR aren't working. The code only relays exactly what the ES instance reports back, nothing more.

If you think "yellow" status does not constitute an error state, we can change it to return 200 with a specific message in that case. Please let me know if there are changes you are expecting in this PR as it stands now.

sarayourfriend commented 1 year ago

I've manually verified that the labels exist and are correct, so I'm going to merge this using the admin workaround instead of trying to sort out why GitHub won't let me re-run the actions. They've been stalled indefinitely, it seems.

AetherUnbound commented 1 year ago

I've manually verified that the labels exist and are correct, so I'm going to merge this using the admin workaround instead of trying to sort out why GitHub won't let me re-run the actions. They've been stalled indefinitely, it seems.

I've been seeing this with PRs that were opened before we added that as a required check. It seems that just adding and removing a label is sufficient enough to trigger the jobs and pass the checks, FWIW!

AetherUnbound commented 1 year ago

Oops, swapped digits with #1074 from the CLI 😅