chore: expose request func signatures #3728
Conversation
Assign request-func-signatures to unexported fields to allow overriding of the logic.
WalkthroughThe changes primarily involve a shift in the structure of the codebase from using exported functions to variable assignments with anonymous functions. This affects the function signatures, return values, and parameter types. The modifications also impact the templates used for code generation and the handling of client and server streaming scenarios. Changes
Poem
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (17)
- examples/internal/helloworld/helloworld.pb.gw.go
- examples/internal/proto/examplepb/a_bit_of_everything.pb.gw.go
- examples/internal/proto/examplepb/echo_service.pb.gw.go
- examples/internal/proto/examplepb/flow_combination.pb.gw.go
- examples/internal/proto/examplepb/generate_unbound_methods.pb.gw.go
- examples/internal/proto/examplepb/ignore_comment.pb.gw.go
- examples/internal/proto/examplepb/non_standard_names.pb.gw.go
- examples/internal/proto/examplepb/openapi_merge_a.pb.gw.go
- examples/internal/proto/examplepb/openapi_merge_b.pb.gw.go
- examples/internal/proto/examplepb/remove_internal_comment.pb.gw.go
- examples/internal/proto/examplepb/response_body_service.pb.gw.go
- examples/internal/proto/examplepb/stream.pb.gw.go
- examples/internal/proto/examplepb/unannotated_echo_service.pb.gw.go
- examples/internal/proto/examplepb/use_go_template.pb.gw.go
- examples/internal/proto/examplepb/visibility_rule_echo_service.pb.gw.go
- examples/internal/proto/examplepb/wrappers.pb.gw.go
- examples/internal/proto/standalone/unannotated_echo_service.pb.gw.go
Files selected for processing (2)
- protoc-gen-grpc-gateway/internal/gengateway/template.go (2 hunks)
- protoc-gen-grpc-gateway/internal/gengateway/template_test.go (3 hunks)
Additional comments: 5
protoc-gen-grpc-gateway/internal/gengateway/template_test.go (3)
144-148: The function signature has been changed to a variable assignment containing an anonymous function. Ensure that all references to this function have been updated accordingly.
309-313: Similar to the previous comment, the function signature has been changed to a variable assignment containing an anonymous function. Ensure that all references to this function have been updated accordingly.
464-464: The function signature has been changed to a variable assignment containing an anonymous function. Ensure that all references to this function have been updated accordingly.
protoc-gen-grpc-gateway/internal/gengateway/template.go (2)
268-270: The function signature has been changed to a variable assignment containing an anonymous function. Ensure that all references to this function have been updated accordingly. Also, verify that the new function signature is compatible with all its usages.
496-496: Similar to the previous comment, the function signature has been changed to a variable assignment containing an anonymous function. Ensure that all references to this function have been updated accordingly. Also, verify that the new function signature is compatible with all its usages.
|
Hi, thanks for the PR. I think I'd like to understand the use case better before approving this. Are you saying that you'd like to add some non-string metadata into the context in the gRPC gateway request path? Is this similar to the request in #3700? Can you do what you want on the |
|
@johanbrandhorst apologies for not creating an issue. I patched this for my own purposes via bazel and wanted to port over the patch to see if it is a reasonable change. The concrete use case is more convoluted than what the PR says and I think for most use cases, someone can inject things into the request ctx via a handler wrapper. We're trying to bypass the default decoder logic and manually unmarshal in our service RPC. We're not using the proto request message and instead directly unmarshaling into our internal datastructure to avoid duplicative memory allocations. #3700 isn't quite the same because it sounds like they want to extract the HTTP pattern outside of the generated handler. Coincidentally we use |
|
If you already have a way to patch this such that we don't need to make any changes to the gateway, I'd prefer not to merge this as is. It seems a very niche use case to me, and in general I'd recommend avoiding making any changes to the contents of generated packages as it may lead to breakages down the road. We certainly don't consider changing unexported variables in generated packages a breaking change. Please close this PR if you're happy to keep it as a private patch. |
|
Sure, we'll keep it as a private patch then. |
Have you read the Contributing Guidelines?
yes
Brief description of what is fixed or changed
Assign request-func-signatures to unexported fields to allow overriding of the logic.
Other comments
I have a use case where I want to push additional info into the ctx prior to calling my RPC method. At the moment I can only set grpc Metadata which only allows
stringtypes.The request function is already a separate func in the generated code and I'd like to override it to inject additional info into my ctx prior to calling the predefined request-func.
Summary by CodeRabbit