-
Notifications
You must be signed in to change notification settings - Fork 205
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
Normalize values across backends #48
Comments
Hi @colebrumley, thanks for reporting the issue and testing out the lib. Much appreciated. There are still inconsistencies, as of now we lag behind the least exhaustive client and API which is
Luckily
We can fix it now in |
Thanks @abronan, that's seems perfectly cromulent to me. In terms of the general approach going forward, is libkv going to stick with the least exhaustive model, or will there eventually be a minimal feature set required of backend drivers? Feel free to close this issue if you'd like, thanks for the answers! |
I think that once we get those subtleties fixed, the classic trio (consul/etcd/zookeeper) is going to be very similar in terms of features without all the convoluted workarounds to make it work. There is a chance to achieve a good abstraction set for those three. For the rest and if we add more backends, the possibility to vendor stores independently will allow you to choose which set of backends makes the more sense to support based on their features and call supported. I'll make sure I add a compatibility matrix to make this choice more obvious in the future and when I'm going to tag a first release very soon. Closing this one, feel free to open another issue if you have any more question! Thanks! :) |
I noticed that the backend drivers for etcd and consul (haven't looked at ZK or Bolt) behave differently with regard to key values on puts. Since Consul base64 encodes key values by default and etcd doesn't, etcd key values cannot contain many special characters. This defeats the purpose of an abstraction layer IMO, since as it stands now a user needs to write different code to use libkv with different backends.
I propose that libkv should normalize value inputs so that all backends behave identically. If there's extra processing that needs to be done to accommodate different backends, libkv should do that on behalf of the client. Either escaping unusable characters or b64 encoding the whole value on backends that don't do this automatically are my first thoughts, but I'm sure there are plenty of others.
The flip side of my argument is that normalized values for some backends would no longer be plaintext to non-libkv clients, which could cause trouble for some use cases. I still think this is preferable to the raw values that libkv uses now. Perhaps there could be a boolean 'sanitize' toggle in
libkv.NewStore()
to maintain backwards compatibility?Just wanted to drop this here to get opinions. If it makes sense and would be accepted, I'll write it up and do a PR.
The text was updated successfully, but these errors were encountered: