OpenSearch migration for Alegre, including dedicated migration tasks in ECS#364
Open
sonoransun wants to merge 27 commits intodevelopfrom
Open
OpenSearch migration for Alegre, including dedicated migration tasks in ECS#364sonoransun wants to merge 27 commits intodevelopfrom
sonoransun wants to merge 27 commits intodevelopfrom
Conversation
added 14 commits
October 26, 2023 11:57
…eparation for merge.
skyemeedan
reviewed
Dec 5, 2023
Contributor
skyemeedan
left a comment
There was a problem hiding this comment.
Wow, this amazing, huge amount of work!
- do we actually need to change the name of the model? ie 'ELASTICSEARCH_SIMILARITY' could still be used to describe the 'model' (indexing structure) even if it is hosted in OpenSearch?
- If we are changing the name we use to refer to doing a lookup using Open/ElasticSearch full text indexes, maybe we should change it to something like 'FULLTEXT_INDEX' so that it describes the kind of index (vs the hosting technology)
Contributor
Author
for sanity sake, I wanted to keep things consistent once we migrate. I did wonder about naming this something more generic, like
|
computermacgyver
requested changes
Jan 11, 2024
Contributor
computermacgyver
left a comment
There was a problem hiding this comment.
We need to decide exactly the scope of the changes here.
- We definitely want to shift to using the Opensearch docker image
- Do we want to rename all the secrets? E.g., change
ELASTICSEARCH_URLtoOPENSEARCH_URL? I don't have a strong preference. We just have to make sure not only the.env_files but also the actual secrets in AWS get changed if we do so. - I do NOT think we should change the Alegre API. The alegre API currently has a model called
elasticsearch. If we change the name of that model, it will break the Alegre API and all current uses of it across Check, Timpani, etc. We would also need to migrate existing data to rename the model, which is painful. Per my comments in this review, I think the Alegre API should not change. This should be a change internal to Alegre and not something that breaks the API for services external to Alegre.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This change migrates Alegre over to OpenSearch with limited backwards compatibility for the legacy model name
elasticsearchfor certain queires.References:
https://meedan.atlassian.net/browse/DEVOPS-517
https://meedan.atlassian.net/browse/CV2-3255
How has this been tested?
This has been tested extensively in QA to verify the new migration task process, and the updating opensearch bindings, and the configuration and naming changes.
Have you considered secure coding practices when writing this code?
I am mostly concerned with breaking existing behavior. I tried to cover as much as possible, and will see how automated tests look when triggered for this pull request.