Support service definitions without any annotations.

[plaflamme]

🚀 Feature

When a service definition does not use any REST / transcoding annotations, it should still be possible to generate a gateway and OpenAPI spec by using the default gRPC path.

As per the gRPC spec, the path of a particular operation is well-defined: /<service name>/<method name>. Moreover, the request and response are serialized in the body and there's no use of any request parameter or header to pass information around. Lastly, the HTTP verb is also well-defined: POST.

Thus, there's a clear default to use when a service has no api annotation. In fact, this is what "Cloud Endpoint" does https://cloud.google.com/endpoints/docs/grpc/transcoding#where_to_configure_transcoding

This would be useful because:

• running the generators on any source would "just work" with no additional effort on the end user;
• provides an alternative, which requires less effort, to the external configuration file when the proto definition is not controlled by the end user;
• allows mapping directly to the underlying (well-defined) gRPC protocol (simplest possible thing that works, no indirection);

This is probably a breaking change since users can no longer "selectively" pick the rpc methods to expose in the gateway. That said, having the default behaviour be exposing all methods instead of none of them seems less surprising. Furthermore, users can have separate service definitions or perhaps additional annotations could be used to "turn off" gateway generation per method.

Could such a feature be considered for v2?

[johanbrandhorst]

Hi Philippe, thanks for raising this issue! I had a read through the http.proto spec to see if there was any recommendation around what should be done for methods that that aren't annotated and I couldn't find anything. You're right that this would be a breaking change, but using the Cloud Endpoints precedent is a good argument to consider this.

My initial reaction to this was that it isn't been requested before and so I don't know how useful this is to users, at the same time I believe users are making use of the feature that allows you to select which endpoints to expose, so with that in mind, if this was something we couldn't still offer to users, I think I'd be inclined to say no.

However, you make a fair argument and I think we could get the best of both worlds with a generator parameter that can be toggled on and off. We could even consider changing the default of such a parameter for v2 if it proved uncontroversial. I do like the idea of being able to just run the generator on an existing or third-party proto file and generating a transcoding server.

So I'm tentatively going to accept this proposal, behind a flag for v1 and v2 for now, and I'd be happy to discuss changing the default behavior in v2 after some testing, and if we are given the time before we tag v2.0.0. This feature would not block a v2 release as we can introduce it in a backwards compatible manner.

What do you need from me to start implementing this?

[plaflamme]

@johanbrandhorst Great! thanks for the quick reply and considering this addition. I'll take a look around the codebase (v1 and v2) and see if any questions come up.

The first thing that comes to mind is naming. Do you have any opinions on how to name this flag/parameter? Given that there's a warn_on_unbound_methods related to this, here are some possible names: generate_all, generate_unbound_methods, allow_unbound_methods.

[johanbrandhorst]

The first thing that comes to mind is naming. Do you have any opinions on how to name this flag/parameter? Given that there's a warn_on_unbound_methods related to this, here are some possible names: generate_all, generate_unbound_methods, allow_unbound_methods.

Great, lets start painting this bike shed before we get to deep into the implementation 😂. I think for v1 we'll want a switch that's off by default, and we probably want the naming to reflect that. I like generate_unbound_methods, I think that's fairly descriptive. Confusingly enough, the existing warn_on_unbound_methods flag is only used when the user is using an external YAML file to generate the gateway, so though the naming is similar, the context is different.

On that note, I wonder if this new feature should affect the behavior of that use case. I expect it probably should, so that's something to keep in mind. If we set grpc_api_configuration and generate_unbound_methods, that probably overrides the need for warn_on_unbound_methods, as we'll simply generate the default path and verb for any unmatched methods.

Does that make sense?

[plaflamme]

@johanbrandhorst I took a quick stab at this for the gateway generator; see #1652

In this version, the interaction between the new option and warn_on_unbound_methods is that the new option will simply override the behaviour of the warning option and simply not print the warning. If both are enabled, a warning is printed on startup telling the user that the warn_on_unbound_methods will have no effect...

WDYT?