-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add point_alias DAG and add alias params to create_new_es_index DAGs #3890
Conversation
…_new_es_index dag
* `point_alias` needs to have trigger_rule=TriggerRule.NONE_FAILED, because the previous step to remove existing alias my be skipped * however, this is a problem for DAGs that import the entire point_alias taskGroup and try to skip it using a branching operator. Because `point_alias` runs when NONE_FAILED, it will try to run even though the entire taskgroup has been skipped. * easiest solution is to have all the individual tasks in the point_alias group individually handle skipping if the appropriate params haven't been passed in
Full-stack documentation: https://docs.openverse.org/_preview/3890 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. Changed files 🔄: |
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 awesome and very straightforward! Local testing worked and made sense, just a few questions for you on it.
...eate_proportional_by_source_staging_index/create_proportional_by_source_staging_index_dag.py
Show resolved
Hide resolved
catalog/dags/elasticsearch_cluster/point_alias/point_alias_dag.py
Outdated
Show resolved
Hide resolved
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.
Fantastic! All the local tests work for me just as expected. I'm glad we were able to combine those point steps into a single action 🚀
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.
Works as expected! LGTM ✅
Fixes
Fixes #3493 by @stacimc
Description
This PR adds DAGs for pointing a
target_alias
at an existing elasticsearch index, with one DAG each for staging and production. Before the alias is applied, it will first be removed from the index to which it currently points (if applicable). There is also an option to delete this pre-existing index if desired. This was done by slightly extending the existingpoint_alias
taskgroup from thecreate_proportional_by_source_staging_index
DAG.The
point_alias
task group is then used in a separatepoint_<environment>_alias
DAG factory, and also added as a step to thecreate_new_<environment>_es_index
DAGs to add functionality to those DAGs as well.Testing Instructions
Test that
create_proportional_by_source_staging_index
DAGs still workaudio-50-percent-proportional-20240308t235423
and the aliasaudio-subset-by-source
.audio-subset-by-source
should have been removed from the index created in the previous step, and applied to a new index with a later timestamp.Test the new DAG
point_staging_alias
locally. Pass in an existing index and an alias that does not currently exist. Verify that the alias gets added to the selected index.remove_alias_from_existing_index
anddelete_old_index
steps will be skipped.remove_alias_from_existing_index
will not be skipped; the alias should be removed from the old index before being applied to the new one.delete_old_index
is still skipped.should_delete_old_index
. This time no tasks should be skipped, and the alias should not only be moved to the new index but the old index should be deleted entirely.Test create_new__es_index DAGs
create_new_staging_es_index
and do NOT pass in a target_alias. It should pass, and all thepoint_alias
steps should be skipped.remove_alias_from_existing_index
anddelete_old_index
steps are skippedshould_delete_old_index
enabled. Verify it is added to the new index, and the index created in step 3 is deleted.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin