-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
cmd/protoc-gen-go-grpc: allow hooks to modify client structs and service handlers #5240
Conversation
cmd/protoc-gen-go-grpc/grpc.go
Outdated
@@ -36,9 +36,11 @@ const ( | |||
|
|||
type serviceGenerateHelperInterface interface { | |||
formatFullMethodName(service *protogen.Service, method *protogen.Method) string | |||
generateNewClientConnInterface(g *protogen.GeneratedFile, clientName string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "New"?
And it's generating a client struct, not the ClientConn interface, isn't it? So generateClientStruct
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM; @menghanl any concerns?
cmd/protoc-gen-go-grpc/grpc.go
Outdated
generateNewClientDefinitions(g *protogen.GeneratedFile, service *protogen.Service, clientName string) | ||
generateUnimplementedServerType(gen *protogen.Plugin, file *protogen.File, g *protogen.GeneratedFile, service *protogen.Service) | ||
generateServerFunctions(gen *protogen.Plugin, file *protogen.File, g *protogen.GeneratedFile, service *protogen.Service, serverType string, serviceDescVar string) | ||
generateServerFunctions(gen *protogen.Plugin, file *protogen.File, g *protogen.GeneratedFile, service *protogen.Service, serverType string, serviceDescVar string) []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will the return value be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Removed.
cmd/protoc-gen-go-grpc/grpc.go
Outdated
handlerNames = append(handlerNames, hname) | ||
} | ||
genServiceDesc(file, g, serviceDescVar, serverType, service, handlerNames) | ||
return handlerNames |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compiling error.
Delete this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Removed
We are supporting the service renaming by using a global variables which will cause issues in some scenarios.
This change allows us to modify the ClientConn interface and handlers, which allows us to store the renamed service name by each clientconn. This matches how the internal service renaming is implemented.
RELEASE NOTES: n/a