Allow user pluggable serializers and add a demonstration ProxyStore serializer #2842
Conversation
This file contains 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
This is in expectation of at least one more exception being added related to serialization as part of ongoing serializer plugin work.
…ggable-identifier
khk-globus
reviewed
Jul 21, 2023
khk-globus
reviewed
Jul 21, 2023
Comment on lines
+29
to
+31
| m = bytes(t.__module__, encoding="utf-8") | ||
| c = bytes(t.__name__, encoding="utf-8") | ||
| return m + b' ' + c |
There was a problem hiding this comment.
To be double-check, __name__ and not __qualname__ is intended?
There was a problem hiding this comment.
I think in the cases where __qualname__ and __name__ differ, this is often going to break: the module + name needs to be something importable.
One case that could work, a class defined directly inside another class without any enclosing function/closure, I would have to poke at the import side of things to see how that would work.
…roxystore test, as it cannot be imported when optional proxystore is not installed
khk-globus
reviewed
Jul 21, 2023
khk-globus
reviewed
Jul 21, 2023
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.
This PR rearranges the deserializer identifier handling so as to dynamically load unknown serializers. These should be identified by a module and class name (and that identifier will be generated automatically).
New serializers, implementing SerializerBase, can be implemented and registered for either the
codeordatapaths in serialization using the existingregister_method_for_*functions introduced in #2831This PR also introduces an example serializer using this API: it extends dill to use ProxyStore on certain objects determined by a user specified policy.
The API for reconfiguring serializers is rather awkward, as evidenced by one of the ProxyStore test case, but this PR does not attempt to make this any nicer.
Type of change