Open deeglaze opened 6 months ago
An implementation with 100% test coverage https://go-review.git.corp.google.com/c/protobuf/+/582735
We talked about this in our team meeting and a major point of contention was that we were not able to find a proto standard on this (https://protobuf.dev/search/?q=protopath). Did we miss it? Would this implementation be unique to Go? If not, would it be compatible with the official C++ implementation? With the Java one?
Failing a standard or another argument, the team believes this might best be part of an external repository for the time being.
cc current team @znkr @lfolger @stapelberg
cc emeritus @neild @dsnet
The concept isn’t protopath but rather fieldmask. I do see a C++ FromString for stringpiece to fieldmask. I think though that indexing repeated fields or maps is not a basic fieldmask concept.
On Wed, May 8, 2024 at 11:56 Nicolas Hillegeer @.***> wrote:
We talked about this in our team meeting and a major point of contention was that we were not able to find a proto standard on this ( https://protobuf.dev/search/?q=protopath). Did we miss it? Would this implementation be unique to Go? If not, would it be compatible with the official C++ implementation? With the Java one?
— Reply to this email directly, view it on GitHub https://github.com/golang/protobuf/issues/1612#issuecomment-2101230600, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFL4HA4SHC36HNL6O2TH2DZBJYOPAVCNFSM6AAAAABHEAEAFGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBRGIZTANRQGA . You are receiving this because you authored the thread.Message ID: @.***>
The concept isn’t protopath but rather fieldmask. I do see a C++ FromString for stringpiece to fieldmask. I think though that indexing repeated fields or maps is not a basic fieldmask concept.
Link to the code for convenience: https://github.com/protocolbuffers/protobuf/blob/bc80135f10ca2ad6f307d4a2165cdaa112d68555/src/google/protobuf/util/field_mask_util.cc#L39-L46
FieldMaskUtil::FromString
amounts to just a string split, though, so it’s considerably less complex than the solution you’re offering.
In the meantime (since discussion with the team and now), I have had a use-case for something like this myself (visiting all fields specified by a number of runtime-provided field paths), so I’m in favor of merging this, but the ecosystem fit question is something we need to understand and answer first.
C++ Protobuf doesn’t seem to have a protopath package. What’s their equivalent?
In general, I like the feature. However, as we had issue in the past with consistency with the protobuf spec, we should make sure we align on the input format with the wider protobuf ecosystem before commiting to something.
I think it makes sense to file a feature request for the protobuf repo. They might be willing to agree to an optional feature that language runtimes could offer but don't have to.
For context: In the past Go protobuf offered a json encoding before the official protobuf spec defined the mapping between protobuf and json. When the protobuf spec defined it, it was incompatible with the one offered by Go which caused a lot of issues for users an maintainers. (This happened before my time on the team and I might not have the full picture. So take it with a grain of salt).
The concept isn’t protopath but rather fieldmask.
As @stapelberg says, FieldMask appears to be an official protobuf concept, documented at https://github.com/protocolbuffers/protobuf/blob/bc80135f10ca2ad6f307d4a2165cdaa112d68555/src/google/protobuf/fi eld_mask.proto
I looked around a little bit at (internal) related concepts, and found:
Given that you say "The concept isn’t protopath but rather fieldmask", I expected to find at least some reference to fieldmask in the CL, but can't find out. Can you tell me how these concepts relate?
I’ll be out of office until the 20th. Thanks for the discussion and broader context. I’ll get back to you.
On Fri, May 10, 2024 at 01:48 Nicolas Hillegeer @.***> wrote:
The concept isn’t protopath but rather fieldmask.
As @stapelberg https://github.com/stapelberg says, FieldMask appears to be an official protobuf concept, documented at https://github.com/protocolbuffers/protobuf/blob/bc80135f10ca2ad6f307d4a2165cdaa112d68555/src/google/protobuf/fi eld_mask.proto
I looked around a little bit at (internal) related concepts, and found:
- FieldPath: XPath-like access to protobuf fields; defines a spec for referencing a single, specific node within a Message instance, providing libraries to convert a string path to a parsed path that can be followed to yield the target node. Entries in repeated fields are treated as distinct paths.
- ProtoPath: Another XPath-like library, but with support for richer expressions, including referencing nodes by tag number, and notably treats a repeated field as a single path, allowing multiple nodes to be accessed with a single path. Wildcard paths are also supported. Supports matchers in C++.
Given that you say "The concept isn’t protopath but rather fieldmask", I expected to find at least some reference to fieldmask in the CL, but can't find out. Can you tell me how these concepts relate?
— Reply to this email directly, view it on GitHub https://github.com/golang/protobuf/issues/1612#issuecomment-2104208249, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFL4HBK5DAX3HFEBIMS7IDZBSCUPAVCNFSM6AAAAABHEAEAFGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBUGIYDQMRUHE . You are receiving this because you authored the thread.Message ID: @.***>
Is your feature request related to a problem? Please describe.
I'm publishing a signed serialized protocol buffer that represents a fair amount of metadata that humans need to understand. I have a CLI tool that allows humans to explore aspects of the protobuf by providing field paths into the message. The syntax for field accesses is already well-established as the string format of a protopath. It'd be great if string -> protopath were a supported part of the protopath library to make this an easier process for anyone offering protobuf exploration tools.
Given a path, there is also the protopath.Values representation that carries the projections of a message along the path that help writing a tool that provides the field path-masked message back to the requester. So, given a path and a message, give back the protopath.Values that represents the values of the message along the path, or return an error if the path isn't traversable for the given message.
Describe the solution you'd like A protopath.ParsePath function to translate the string form of a path (with optional "(message.name)" root) to the path representation, and a protopath.PathValues function to translate a path and message to a protopath.Values.
Describe alternatives you've considered I have the whole implementation implemented as part of my own project, but it's general enough and desired (given a TODO in protopath to add a parser) to be part of the general library. A similar API exists internally at Google for field masks from string paths following the same semantics, and there's not really a need for that functionality to be internal only.