search

iansmith / parigot

Develop as a monolith, deploy as microservices.
https://parigot.info
BSD 3-Clause "New" or "Revised" License
12 stars 1 forks source link

.proto changes #21

Closed iansmith closed 1 year ago

iansmith commented 1 year ago

@cindyuc This is a large PR but it is mostly fixing the existing code to work correctly with the changes.

This PR allows three new things in .proto files.

  1. It allows .proto files to have both services and messages in the same file, avoiding the need for the "BLAHmsg" for every "BLAH". This vastly simplifies imports; all the services, generated code, and messages are in the same directory g/blah/v1 and will be imported as blah.

  2. It allows the use of protobuf Enum for the error types. This greatly simplifies the use of errors in the entire system. You guilted me the other day about how wasteful my 128bit errors were when they only used about 24 bits... 😀 So now the error type is simply the one generated by protobuf, which is an int32.

This error isn't all unicorns and ice cream because the rules that protobuf applies makes it impossible to put two or more Enum definitions in the same file. If you try it you will see that it produces an error similar to "protobuf using C++ scoping rules..." which means that you cannot have two enums that both have the NoError as the 0 value, for example. Since we generate code that depends on errors being semantically "no error" when the value is 0, we can't have two enums with the name NoError and keep the convention of having this same name and value for every error type.

This is partially mitigated by the fact that I added an option called error_id_name that lets you choose what error type is connected to a given service. This lets you share a single error type between several services.

service Bar {
  option (protosupport.v1.error_id_name) = "MethodCallSuiteErr";

  // this calls the Foo service to test intra-service calls
  rpc Accumulate(AccumulateRequest) returns (AccumulateResponse);
}

Another side effect of this change in the way errors work is that functions that are returning values to something that might be in another language use int32 as the return type, not id.IdRaw. id.IdRaw has been removed. You can see this in queuehost.go for example.

3) Finally, I removed the restriction that service names must end in "Service". This is actually something that was enforced by the buf linter (buf lint in the Makefile) and is supposed to be a best practice when creating .proto files. The guys that built buf generally are very good with their choices, but this one led to the silly names for our generated code, like FooServiceClient and FooServiceServer when really FooServer and FooClient are much clearer. I changed the existing code in the repo to use this new, simpler naming.

netlify[bot] commented 1 year ago

Deploy Preview for parigot canceled.

Name Link
Latest commit 68e64ddea515ac577b18c376d56c494615cec1ae
Latest deploy log https://app.netlify.com/sites/parigot/deploys/6485d8e30e782f0008b0a859