-
Notifications
You must be signed in to change notification settings - Fork 451
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
Act on Messages generically ("Reflection" support in some other languages) #1029
Comments
If the API here returned a decoded descriptor, then we could iterate over time with different ways of storing the data. For example, we could store it in TextFormat or a base64-encoded binary or ... |
As @tbkka mentioned, descriptors aren't all that interesting. What you need is what C++ (and other languages) add beyond that to then generically work on message. i.e. - given a descriptor parse data/query fields/serialize. Also, given generic The first case, could be done with just a As far as generating lite/full, there is movement with Google's protobuf team to move somewhat away from that sorta generation mode, as it causes problems - what is one dependency just needs lite, but something else needs full and they overlap in the messages they need this for, how does a build system generate things only once and correctly. As you mention the text data for Swift, we've debated moving this into separate files, so it builds to model it as additional modules that are optionally depended on to resolve the build issues. ps - the text data would likely go away if we had descriptors since the data would be in the descriptor. Anyway, changing the summary, since the issue isn't really exposing the |
If we had some means to get the embedded descriptors (even as just an "experimental" option of some sort), I think it would be a lot easier to get people to help build out the APIs to use them. |
Not sure I follow. As you mention, it isn't clear what data is useful. If we don't intend to build api that uses that format, what's the point of exposing it?
|
The concrete use case for me is gRPC reflection (i.e. the following issue: grpc/grpc-swift#768). gRPC reflection allows the service API to be interrogated and invocations to be made via a CLI (https://github.com/grpc/grpc/blob/master/doc/command_line_tool.md).
In order for the Swift gRPC codegen to offer reflection support, there has to be a standarised support for getting descriptors of protobuf types. While it's theoretically possible to lift the feature to gRPC, we're likely eventually going to bring descriptor support to swift-protobuf, so I'd much prefer this option to start with. That way other use cases such as constructing self describing messages (https://developers.google.com/protocol-buffers/docs/techniques#self-description) would also benefit. I'd argue it makes sense to decouple the ability to query descriptors from message reflection (a complex area of its own, since no standard approach to generic reflection exists in the language, yet). |
From the looks of https://developers.google.com/protocol-buffers/docs/techniques#self-description, they'd actually want the transitive set, which is a lot more than just have a message be able to return the descriptor for itself. This would require logic to go from a messages descriptor, up to the file's, and also collect all the transitive descriptors used as imports to provide all the support types. All that seems to point more in the direction of the grpc generation feature at the moment. Scoping the support where needed would also mean if there was a bug, it could be fix via one project instead of synchronized changes between two projects. |
@gmilos -- Does gRPC require DynamicMessage support (the ability to encode/decode/store/manipulate) messages without having generated code? Although it's not trivial, I don't think this is especially hard to implement. As a starting point, I would suggest trying to represent a DynamicMessage as:
Getting the MessageDescriptor will be interesting. Depending on the use case, someone might have a collection of MessageDescriptor objects already available, or they might have a collection of FileDescriptors (in which you can look up the MessageDescriptor) or they might have a FileDescriptorSet, or any combination of these. |
I thought @gmilos case was that they want grpc-swift to be able to return descriptors so the other tool could then query them and act on the data? i.e. - no reflection/generic message support, just a way to return descriptors from within grpc-swift. That's why I was thinking since it will take support within that library, it makes more sense to it to do all the work itself so even if SwiftProtobuf had the descriptors (and/or reflection support) it likely wouldn't directly match what was needed for what they are trying to support. |
grpc-swift would only need to return the descriptor information, specifically https://github.com/grpc/grpc-proto/blob/dd2dca318eb197b96b60f22297871fb1ed862800/grpc/reflection/v1/reflection.proto#L94 in response to either one of the following requests https://github.com/grpc/grpc-proto/blob/dd2dca318eb197b96b60f22297871fb1ed862800/grpc/reflection/v1/reflection.proto#L45-L55. @thomasvl, I see your point about possible API discrepancy, but if SwiftProtobuf had the ability to return descriptors, we'd try our hardest not to have to duplicate the message type descriptor metadata (since grpc-swift is generally oblivious about serialisation, in extreme case it might not even be protobuf). Of course we don't yet have descriptors in SwiftProtobuf so that's mostly moot point. |
The If we expose Message ->
So if we're talking all this at generation time, that's where it seems like grpc-swift could do exactly this right now, it already generates off the exact same data that SwiftProtobuf does (FileDescriptorSet/FileDescriptor). So why put the indirection in place now when SwiftProtobuf would be doing this specifically to the needs of grpc-swift? Yes, if/when SwiftProtobuf has some of the reflection apis, we might be able to expose the same information, but it feels odd to build all this out for the single use case of grpc-swift when grpc-swift generation is on the exact same apis that SwiftProtobuf would be trying to generate this data from. |
Possibly relevant: SE-0271 adds support for binary resources in Swift packages. Writing out the I also wonder if it would make sense to spin out the Descriptor support APIs as a separate library that people could incorporate if they had a use for those capabilities. |
I think we'll eventually do it, but even the Descriptor classes we have work fine for code generators, but without a way to bridge that to parsing/acting on messages/etc. I'm still struggling for what one would do with them. So use cases are likely important before we commit to that API in a larger space. |
I'm interested in this too. Is there any current best practice or known workaround for generating a |
swift-protobuf doesn't support descriptors currently (https://github.com/apple/swift-protobuf/blob/9df381f72ff22062080d434e9c2f68e71ee44298/Protos/google/protobuf/descriptor.proto). Specifically, it's not possible to query the descriptors for file/message/... at runtime. This prevents number of reflection use cases, in particular support for gRPC service reflection grpc/grpc-swift#768 (which is a valuable tool for interrogating gRPC services without prior schema knowledge).
Support for descriptor would require extra binary space in order to keep metadata required to build such a descriptor. C++ currently handles this by definining two different message types:
Message
vsMessageLite
(https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.message#Message.GetDescriptor, https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.message_lite#MessageLite) whereMessageLite
doesn't have the embedded descriptor. This is controlled at gen time, withoption optimize_for = LITE_RUNTIME;
.The extra binary space issue is already something we have to deal with for JSON/text serialisation. See #18 for discussions of an approach where we could conditionally include the functionality and conformance to a protocol.
The text was updated successfully, but these errors were encountered: