Skip to content

Conversation

thrau
Copy link
Member

@thrau thrau commented Mar 2, 2023

This PR uses the persistence API to integrate community services into the persistence lifecycle. It's "basic" because it doesn't fully add restore functionality for every services.

  • added accept_state_visitor methods to community services that have an asset directory
  • added the SAVE_STRATEGY variable used by the persistence plugin to the config
  • I was able to add the restore functionality for opensearch (tested manually), but not for others. Those will be part of follow-up PRs once we have a good persistence testing setup.
  • I also snuck in the two parameters to the opensearch create_domain method that were missing after the last ASF API update /cc @alexrashed

@thrau thrau requested a review from giograno March 2, 2023 18:21
@thrau thrau requested a review from alexrashed as a code owner March 2, 2023 18:21
@thrau thrau temporarily deployed to localstack-ext-tests March 2, 2023 18:22 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Mar 2, 2023

LocalStack integration with Pro

       3 files  ±0         3 suites  ±0   1h 31m 26s ⏱️ - 9m 11s
1 778 tests ±0  1 399 ✔️ ±0  379 💤 ±0  0 ±0 
2 504 runs  ±0  1 775 ✔️ ±0  729 💤 ±0  0 ±0 

Results for commit 84cf54b. ± Comparison against base commit 881018e.

♻️ This comment has been updated with latest results.

@coveralls
Copy link

coveralls commented Mar 2, 2023

Coverage Status

Coverage: 85.068% (-0.05%) from 85.116% when pulling 84cf54b on persistence-plugin-prep into 881018e on master.

@thrau thrau temporarily deployed to localstack-ext-tests March 2, 2023 19:51 — with GitHub Actions Inactive
@dominikschubert dominikschubert self-requested a review March 3, 2023 12:53
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny issue with the create_cluster call, otherwise LGTM! 🚀

LOG.info(f"Restoring domain {domain_name} in region {region}.")
try:
preferred_port = None
if config.OPENSEARCH_ENDPOINT_STRATEGY == "port":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting problem actually that might bite us in a few other places as well when a feature flag changes between different persistence-enabled runs 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thrau thrau temporarily deployed to localstack-ext-tests March 5, 2023 19:40 — with GitHub Actions Inactive
@thrau thrau force-pushed the persistence-plugin-prep branch from 38ccc0a to 16691a7 Compare March 5, 2023 20:57
@thrau thrau temporarily deployed to localstack-ext-tests March 5, 2023 20:58 — with GitHub Actions Inactive
@thrau thrau temporarily deployed to localstack-ext-tests March 5, 2023 21:00 — with GitHub Actions Inactive
@thrau thrau merged commit 629518a into master Mar 5, 2023
@thrau thrau deleted the persistence-plugin-prep branch March 5, 2023 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants