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

Choosing Serializer from Configuration for Storage Providers #895

Closed
armona opened this issue Oct 10, 2015 · 9 comments
Closed

Choosing Serializer from Configuration for Storage Providers #895

armona opened this issue Oct 10, 2015 · 9 comments

Comments

@armona
Copy link

armona commented Oct 10, 2015

This is a suggestion to add a feature to the storage provider sub-system. Both the Azure storage provider and some others community created ones (e.g. RedisStorageProvider) use Json as a defualt. Currently there is no 'clean' way to override this behavior by configuration.

My suggestion - add custom property serializer/deserializer to the storage provide attribute as so:
[StorageProvider(ProviderName="AzureStore", Serializer="MsgPack")]

Some consideration have to be made about implementing this feature. Writing a special serialization class will probably alter the IStorageProvider and might have issues with backward-compatibility, although this could worked around.

Also, consider adding built-in compression support.

@gabikliot
Copy link
Contributor

Good idea. An alternative way to implement it is via configuration. Azure Table storage provider already accepts a boolean flag to use json or not. We could change it to accept serializer name, if we need to support multiple serializers.

@armona
Copy link
Author

armona commented Oct 11, 2015

Thanks for the reply, would you mind if I craft a pull request? would you consider the suggestion useful for upcoming versions?

@sergeybykov
Copy link
Contributor

I agree with @gabikliot - config in general is a more flexible place for such things, so that one doesn't have to change code in order to change serializer. Unless you meant that sgPack" in your example above is just a reference to a config value or something

@armona
Copy link
Author

armona commented Oct 12, 2015

@sergeybykov - yes that is what I meant. With current implementation every IStorageProvider will have to implement the same logic if it would like to support different serializer other than the default (same holds true for compression/decompression). I was thinking this part (that is the serialization logic) could be generalized for all storage providers.

@sergeybykov @gabikliot - Would you reckon this enhancement useful?

@veikkoeeva
Copy link
Contributor

I wonder if versioning should be thought too. It is true that some formats allow for better story when deserializing different versions. One solution would be to add an explicit counter for version and have code that checks that and chooses the serializer with that information. This explicit information could be too much data, though. I wonder if it would work to define a list of deserializers in preference order and pick the first that can deserialize the data. Or better yet, allow one to plug in a Func in which one can do whatever logic is needed to check the version. This Func just would have an appropriate signature (which state class type, data stream etc.).

@sergeybykov
Copy link
Contributor

@armona Sorry, looks like I somehow missed your question here. Yes, this would definitely be a useful enhancement. A PR would we greatly appreciated.

@veikkoeeva I think we could generalize is by passing optional parameters string to the serializer, which could include version information. #953 already started down the path of dynamically discovering the right serializer to use, although the multiple serializer story is incomplete yet.

@DixonDs
Copy link
Contributor

DixonDs commented May 25, 2017

@sergeybykov I find this request very useful and I could try to implement it at some point, but I just wonder, since it is quite old issue, is there any new design considerations related to this issue that I should be aware of e.g. maybe providing the serializer type name through configuration is not wanted anymore, those kinds of things.

@sergeybykov
Copy link
Contributor

We are moving fast towards more explicit ways of configuring providers and persistence. So this idea might be outdated soon. Tagging @jdom and @jason-bragg, as they've been exploring the new avenues.

@sergeybykov
Copy link
Contributor

#3738 makes provider configuration completely open. So this is no longer an issue.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants