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

[WIP | Spec] Protobuf File #67

Open
refs opened this issue May 6, 2020 · 3 comments
Open

[WIP | Spec] Protobuf File #67

refs opened this issue May 6, 2020 · 3 comments
Labels
documentation Improvements or additions to documentation

Comments

@refs
Copy link
Member

refs commented May 6, 2020

While designing a Protobuf API taking into consideration Javascript consumers we need to mind that the generated js client will make use of define endpoints on the protobuf file. As a way of easing development and not having the overhead of writing new Encoder / Decoders and leveraging the use of reflection, requests that make use of the JS generated client MUST be defined as POST.

This is due to RPC and REST are different API design, and as we're designing for RPC and not Restful endpoints, our generated swagger client needs to be aware of this design choice and not get into the nitty-gritty of understanding REST routes, but only Marshal / Unmarshal JSON on the request body.

This design choice was made taking into consideration the HTTP Spec, which recommends GET requests not to contain a body and Swagger enforces this:

GET, DELETE and HEAD are no longer allowed to have request body because it does not have defined semantics as per RFC 7231.

This causes our generation to panic on GET requests.

Root causes: JS clients does not currently support gRPC (at the moment of this writing).

@refs refs added the documentation Improvements or additions to documentation label May 6, 2020
@kulmann
Copy link
Contributor

kulmann commented May 6, 2020

The paths for the generated http post endpoints have the primary requirement to be unique, secondary requirement to be developer friendly. They do not have the requirement to adhere RESTful design.

We decided to go with paths like shown in the following example. The generated http endpoints will never be visible for developers, as they are internally used in the generated JavaScript client. For debugging purposes it might still be useful to have human readable paths.

service ValueService {
  rpc SaveSettingsValue(SaveSettingsValueRequest) returns (SaveSettingsValueResponse) {
    option (google.api.http) = {
      post: "/api/v0/settings/value-save",
      body: "*"
    };
  };
  rpc GetSettingsValue(GetSettingsValueRequest) returns (GetSettingsValueResponse) {
    option (google.api.http) = {
      post: "/api/v0/settings/value-get",
      body: "*"
    };
  };
  rpc ListSettingsValues(ListSettingsValuesRequest) returns (ListSettingsValuesResponse) {
    option (google.api.http) = {
      post: "/api/v0/settings/values-list",
      body: "*"
    };
  };
}

@kulmann
Copy link
Contributor

kulmann commented Jun 4, 2020

One drawback of this is, that the returned status codes have to be handled and will probably not be what a developer would expect. E.g. a successful get request in the above style would not return http 200, but http 201.

@pascalwengerter
Copy link
Contributor

@refs what's the state of things here currently? The repo has experienced quite some changes I assume

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

No branches or pull requests

3 participants