Allow buf curl to use multiple schemas when resolving elements#2587
Allow buf curl to use multiple schemas when resolving elements#2587
Conversation
|
@stefanvanburen, this is the other |
| func (c combinedResolver) FindFileByPath(s string) (protoreflect.FileDescriptor, error) { | ||
| var lastErr error | ||
| for _, res := range c { | ||
| file, err := res.FindFileByPath(s) |
There was a problem hiding this comment.
- What about if multiple resolvers have the same file? What is the defined behavior? In the storage package as an example, we error, although we might want the behavior to be different here. At the least, we need to be able to explain in our documentation some deterministic, defined behavior ("local schemas are checked first, then reflection, local schemas checked in order of flags" as an example potential behavior)
There was a problem hiding this comment.
The documentation already states that reflection is checked and then the others are checked in the order they appear on the command line: https://github.com/bufbuild/buf/pull/2587/files#diff-05c240e5d4191f595d5b747c112c314cf14ee4d77505cee99aac9500eed39527R257-R260
I guess the language could be tightened up a bit to make it more clear.
What about if multiple resolvers have the same file? What is the defined behavior?
They are treated as separate resolvers and consulted in the order they are specified on the command-line. We definitely do not want any errors here, because these are totally separate modules -- so it's quite realistic that they might have some shared dependencies and thus contain redundant files. So making that an error could make the feature unusable.
This enables the use of multiple
--schemaflags, and it also enables using both--reflectand--schema. By default, if a--schemaflag is present, reflection is not used. But if one also explicitly specifies--reflect, then both will be used.The value is that sometimes an RPC response may include extensions or messages inside
google.protobuf.Anyvalues that are not defined in the same schema that defines the RPC service. For reflection, this is usually less of an issue. But it can still be an issue if the RPC server also does not know about an extension or message inside anAny, which can happen when the data actually originates in some other server before being returned by the RPC handling server.When not using reflection, the value is even greater, since it is quite common for extensions or error details to be defined in separate modules. Sometimes they are defined in dependencies of the RPC schema, but if the RPC schema doesn't directly import the files in question, they may be omitted (since downloading a module only includes the files in dependencies that are necessary to compile).
This addresses an issue raised in Slack:
https://bufbuild.slack.com/archives/CRZ680FUH/p1699560915612389
The user was using the reflection endpoint to download descriptors. However, the reflection endpoint schema does not know about any custom options that the user has defined. So the way for custom options to be "known" and included in the JSON representation of the response is to actually point
buf curlat both schemas: the reflection module (which defines the RPC endpoint) and another (which defines the custom options).So, without this change, we could only specify a single schema:
buf curl --netrc --schema buf.build/bufbuild/reflect \ https://buf.build/buf.reflect.v1beta1.FileDescriptorSetService/GetFileDescriptorSet \ --data @- <<EOM { "module": "buf.build/bufbuild/knit-demo", "symbols":["buf.knit.demo.swapi.relations.v1.FilmResolverService"] } EOMIn the output of the above commands, all of the method options look like so:
{ "name": "GetStarshipFilms", "inputType": ".buf.knit.demo.swapi.relations.v1.GetStarshipRelationsRequest", "outputType": ".buf.knit.demo.swapi.relations.v1.GetFilmsResponse", "options": { "idempotencyLevel": "NO_SIDE_EFFECTS" } },And if we use
-vand also have #2586 applied, we see that it prints the following:So, with this branch, we can actually point buf at two schemas, including the one that defines the relevant custom options:
buf curl --netrc \ --schema buf.build/bufbuild/reflect \ --schema buf.build/bufbuild/knit \ https://buf.build/buf.reflect.v1beta1.FileDescriptorSetService/GetFileDescriptorSet \ --data @- <<EOM { "module": "buf.build/bufbuild/knit-demo", "symbols":["buf.knit.demo.swapi.relations.v1.FilmResolverService"] } EOMAnd now we see the output includes method options that look like so:
{ "name": "GetStarshipFilms", "inputType": ".buf.knit.demo.swapi.relations.v1.GetStarshipRelationsRequest", "outputType": ".buf.knit.demo.swapi.relations.v1.GetFilmsResponse", "options": { "idempotencyLevel": "NO_SIDE_EFFECTS", "[buf.knit.v1alpha1.relation]": { "name": "films" } } },