works with latest grpc - Githubissues

jhump commented 3 years ago

Here's a summary of the changes

Use new grpc.ServiceRegistrar; deprecate grpchan.ServiceRegistry
Re-generate code, support new ServiceDesc var naming
Document that the protoc plugin is not needed at all since new grpc-plugin generates code using ServiceRegistrar (yay!)
Add GetServiceInfo method to HandlerMap, so it is closer to the interface of *grpc.Server
Most of the code motion: the newer grpc module pulls in a newer proto module, which deprecates lots of packages from the v1 runtime. The staticcheck step of running make ci complains about using deprecated stuff. So this migrates to using the non-deprecated stuff in the v2 runtime, with the exception of two places that are needed so that this module still interoperates with older generated code (e.g. messages generated using older version of protoc-gen-go).

jhump commented 3 years ago

is this something you can actually promote to master without breaking things? Is it really still a v1? Do we need to run another branch for a while?

I don't see any problem with merging to master. It's definitely not a new major version since none of the API has changed in backwards compatible ways. The biggest change is the protobuf runtime dependency moving from v1.3 to v1.4 (also just a minor version change). And that by itself breaks nothing: the new protobuf runtime is backwards-compatible with all older generated code (our internal repo is proof of that, where we intentionally run two different versions between runtime and code generator).

I didn't even manually upgrade the proto runtime: it came along with depending on newer version of gRPC. The gRPC runtime has required it since October, as of v1.33 (also just a minor version change). And anyone that wants to use the newer gRPC code generator (also released October 2020) will need these changes.

dragonsinth commented 3 years ago

Gotcha. Could you push an internal branch that uses this, so we can verify that these changes will work generally?

dragonsinth commented 3 years ago

Merged via fast forward

jhump commented 3 years ago

@dragonsinth, thanks for the merge assist.

fullstorydev / grpchan

works with latest grpc #56