-
Notifications
You must be signed in to change notification settings - Fork 204
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
Run API and Ingestion Server tests with all relevant ES versions #2832
Conversation
This will also need the required checks changed in the infra repo with the new names. I'll push a commit with changes to ingestion and api so that those steps actually run. |
The most recent CI run confirms that this runs (and that the changes in #2777 should be okay (I think): https://github.com/WordPress/openverse/actions/runs/5862236561/job/15893876650?pr=2832#step:5:19 Notice that it pulls ES version 7. And here it pulls version 8: https://github.com/WordPress/openverse/actions/runs/5862236561/job/15893876557?pr=2832#step:5:19 |
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.
The CI runs prove that this is working.
I've asked inline, too, but do we really need changes to the steps that just "imitate" a CI step, both for the ingestion and the API (the ones that just echo something and don't do anything)?
|
||
steps: | ||
- name: Pass | ||
run: echo 'Ingestion server tests are skipped because ingestion server is unchanged.' |
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.
Do we really need to update this step since it's not really doing anything? It's just the "Pass" step.
* Run API and Ingestion Server tests with all relevant ES versions * Use better names and add bypass jobs * Force CI to run for API and ingestion
* Remove `body` from the index requests * Run API and Ingestion Server tests with all relevant ES versions (#2832) * Run API and Ingestion Server tests with all relevant ES versions * Use better names and add bypass jobs * Force CI to run for API and ingestion --------- Co-authored-by: sarayourfriend <[email protected]>
Fixes
Based on #2777 and related to the comment here: #2777 (review)
Description
Because production is currently stuck on ES 7 until we've figured out why ES 8 caused the queries to take so long, we need to test all our code against both ES versions. Until we've fully transitioned to ES 8 we need to support both versions.
Testing Instructions
Check the new CI tasks and confirm they've run as expected. Locally, run
OPENVERSE_ES_VER=7 just api/test
locally to test as an example that the environment variable works as expected.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin