Sitelet https://web.archive.org/web/20220109171735/https://github.com/localstack/localstack/pull/4580
Skip to content
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

Pickle CloudFormationRegion classes for persistence #4580

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@dbramwell
Copy link

@dbramwell dbramwell commented Sep 11, 2021

Attempt to add persistence to the cloudformation implementation by pickling the CloudFormationRegion instances.

I'm unfamiliar with both pickle and the localstack codebase, so sorry if this is a woefully naive implementation. It does however work well in my simple testing with a cdk project.

I'm unsure of the best way to add tests for it, if the implementation seems appropriate then some guidance would be appreciated.

Should hopefully address issue 3191

@dbramwell
Copy link
Author

@dbramwell dbramwell commented Oct 5, 2021

Any thoughts on this? It makes it much easier to keep a clean dev environment when using cdklocal

@whummer
Copy link
Member

@whummer whummer commented Oct 14, 2021 •

Hi @dbramwell, thanks for this PR, and apologies for the delay on this one. We're currently in the process of preparing a major PR which will introduce a plugin system and entirely restructures the way that services are loaded and started up: #4648

I have some suggestions for slightly restructuring the changes in this PR - to make them a bit more generic/reusable, but also to ensure they don't collide with existing persistence mechanisms that are plugged in and used in other places.

Before we go into a more detailed review, we'd like to wait for the changes in #4648 to get merged (should happen in the next couple of days), then we'll have a clearer picture how this can fit into the new service lifecycle.. Thanks!

@dbramwell
Copy link
Author

@dbramwell dbramwell commented Oct 15, 2021

Awesome, thanks! Lazy loading sounds like a great idea. No rush

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants