Conversation
5b78353 to
3036ba1
Compare
whummer
left a comment
There was a problem hiding this comment.
Awesome set of changes @thrau ! 🚀 This gives us a nice flexibility to keep the generic API in this repo, and extend it elsewhere with plugins (via SnapshotPersistencePlugin, and the visitor pattern).
Was initially thinking whether introducing our own pickle module (with the same convenience functions pickle.loads, pickle.dumps etc) could cause some confusion if users are expecting to be interacting with the built-in pickle library. But then my second reaction was that it actually can be beneficial - this way we make it more explicit that our custom mechanism is being used. (It's also not uncommon to see things like import dill as pickle, which would be a similar drop-in replacement as we're defining here.) 👌 I guess we only need to ensure that we communicate to devs that raw pickle should never be used directly (which should not be the case anyway, as the core logic is encapsulated in the core runtime).
Love the direction this is taking - can't wait to see it in action! 🥳 🚀
3036ba1 to
0d34694
Compare
This PR extends the persistence API to prepare localstack core for the persistence extension.
Most noteworthy, the PR adds a wrapper around pickle/dill, and exposes a similar API that is composable and extensible.
EncoderandDecoderinstances (they give more flexibility and decouple higher-level logic like visitors from pickle), butPickleEncoderandPickleDecoderby defaultfrom localstack.state import pickle; pickle.dump(...)dill.Pickler.dispatch) with a mechanism that can also match subclasses (apply certain reducers for all subclasses rather than just the specific type)pickle.dumpscan now be used in the store police, as it will be extended dynamically with custom serializers by the persistence plugin, and the underlying system is used for both pods and snapshot persistence.