Skip to content
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

Clarification on RPC and Request/Response Names #2791

Closed
a-priestley opened this issue Feb 28, 2024 · 2 comments
Closed

Clarification on RPC and Request/Response Names #2791

a-priestley opened this issue Feb 28, 2024 · 2 comments

Comments

@a-priestley
Copy link

I was hoping to clear something up that I could not find clarification on in the Buf documentation.

These rules specify that messages used and returned by RPCs should be named using the name of that RPC, respectively suffixed with "Request" or "Response". Additionally, all requests and responses should be used only once.
I believe that this rule only makes sense when streaming is not taken into consideration.
Here's an example:

message Hello {
  uint32 id = 1;
  string message = 2;
}
message CreateHelloRequest {
  Hello hello = 1;
}
message CreateHelloResponse {
  string response = 1;
}
message CreateHellosResponse {
  repeated string responses = 1;
}
service HelloService {
  rpc CreateHello(CreateHelloRequest) returns (CreateHelloResponse);
  rpc CreateHellos(stream CreateHelloRequest) returns (CreateHellosResponse);
}

In this example, I am defining a unary rpc that can be called to create a single "Hello" object in some storage implementation or another. It uses a single "Hello" message to create the record. Similarly, the rpc "CreateHellos" accepts a stream of Hello messages, which will create many Hello objects over the lifetime of the stream.

The streaming rpc definition in this example breaks the rules since:
a): The request name passed to the rpc does not match the name of the rpc, suffixed with "Request"
b): The request message has already been used by the "CreateHello" unary call defined above it.
To adhere to both rules, I would need a new message containing a single Hello called "CreateHellosRequest":

message CreateHellosRequest {
  Hello hello = 1;
}
service HelloService {
  rpc CreateHello(CreateHelloRequest) returns (CreateHelloResponse);
  rpc CreateHellos(stream CreateHellosRequest) returns (CreateHellosResponse);
}

This name makes little sense to me because I am not creating multiple hellos in a single request - I am creating one at a time over a single rpc. But it also doesn't make sense to use the singular in naming the rpc (collision with the unary rpc notwithstanding), because its job is to create multiple hellos.

Are there currently any guidelines for dealing with this situation?

@bufdev
Copy link
Member

bufdev commented Mar 6, 2024

Your second example of creating a CreateHellosRequest is what you should do. The whole goal of the rule is to not allow any interdependence between RPC request/response types - if you edit one request message, it should only impact the input of one RPC. There's many situations where people find this important for forwards-compatibility - for example, you're adding a field to your RPCs, and are doing it piecemeal, updating (and rolling out) the unary RPC first, and the streaming one later. Or you want to apply different validations via protovalidate to different requests.

There's nothing special about the fact that one RPC is doing streaming, and another isn't - they're both distinct RPCs that just happen to have overlapping behavior as you've defined it. If you really believe you will have no issue with having the same request for these RPCs for the lifetime of your API, then you can ignore the lint rule for this specific RPC set.

@bufdev bufdev closed this as completed Mar 6, 2024
@a-priestley
Copy link
Author

Thanks for the advice!

Let's say, expanding on the second example, I want to define a new request that actually does create multiple Hellos in a single shot:

message CreateHellosRequest {
  repeated Hello hellos = 1;
}
service HelloService {
  rpc CreateHellos(CreateHellosRequest) returns (CreateHellosResponse);
}

It might be a good idea to be more particular in naming with regards to streaming RPCs:

message CreateHellosRequest {
  repeated Hello hellos = 1;
}
message CreateHelloSendStreamRequest {
  Hello hello = 1;
}
service HelloService {
  rpc CreateHellos(CreateHellosRequest) returns (CreateHellosResponse);
  rpc CreateHelloSendStream(stream CreateHelloSendStreamRequest) returns (CreateHelloSendStreamResponse);
}

And the reason for the use of singular in the streaming definitions is that you could possibly want an additional stream for sending multiple hellos in each request:

message CreateHellosSendStreamRequest {
  repeated Hello hello = 1;
}
service HelloService {
  rpc CreateHellosSendStream(stream CreateHellosSendStreamRequest) returns (CreateHellosSendStreamResponse);
}

...Specifying the word "Send" to differentiate between one-directional and potential bidirectional streams as well.

Would you say this is a reasonable approach?

Thanks again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants