-
Notifications
You must be signed in to change notification settings - Fork 53
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
Keeping unknown fields in structs around #42
Comments
There's a feature matrix here for how other serialization libraries handle this: Proto2 had it, but removed it in Proto3 (without any justification). I think it's a good idea to store unknown fields, lets you add fields to semi-opaque structs that are just passed around without having every single hop on the way be updated to understand the struct. |
This would be a huge help in developer productivity for our team. |
There's more discussion on the removal of uknown fields in proto3 here: protocolbuffers/protobuf#272 There still hasn't been a reason for removal given. I still think we should retail unknown fields. @mjcowan Can you expand more on your use case and how keeping unknown fields would help your team? |
My team gets objects that have gone through multiple hops, often with little or no modification (e.g. service1 only cares about fieldA, service2 only cares about fieldB, we care about all of them). Right now we just use JSON, but in the future we'd like to define an IDL for the data. If unknown fields weren't kept, then every time we added a new field we might need to update five or six places. |
Just an update, this is now fixed in proto3, I think we should do something similar to keep them around. |
Prashant brought this up in #41: When we run into unknown fields during deserialization, we keep them around in some sort of map and then include them in the serialized result. This would make extension and forwarding use cases possible where you parse the partial argument, act on it somehow, serialize it and forward it elsewhere without losing any information.
CC @kriskowal @prashantv
The text was updated successfully, but these errors were encountered: