-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Configurable query parameters parser #1138
Conversation
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.
This is very nearly there, but I think we have to make one major change; instead of changing the generated code (which will inevitably break our users), lets change the PopulateQueryParameters
function to call runtime.customQueryParameterParser
if it is non-nil, and have a new runtime.WithQueryParameterParser
set this unexported field. It would mean the parser can only be set globally, not per-servemux. Is that still okay for you?
Global parser is fine I think, if we don't want to change the generated code there is not much else we can do here - I'll try to make changes some time this week, thanks for quick feedback. |
Makes names more consistent and improves docs Co-Authored-By: Johan Brandhorst <johan.brandhorst@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #1138 +/- ##
=========================================
Coverage ? 53.79%
=========================================
Files ? 42
Lines ? 4207
Branches ? 0
=========================================
Hits ? 2263
Misses ? 1696
Partials ? 248
Continue to review full report at Codecov.
|
Changed to use global parser. Added interface for parser implementations. |
Co-Authored-By: Johan Brandhorst <johan.brandhorst@gmail.com>
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, thanks for the contribution!
Pleasure! |
* grpc-ecosystem#1128 make it possible to configure query param parser implementation * Apply suggestions from code review Makes names more consistent and improves docs Co-Authored-By: Johan Brandhorst <johan.brandhorst@gmail.com> * grpc-ecosystem#1128 make configured parser a global parser * update docs for configuring query parser Co-Authored-By: Johan Brandhorst <johan.brandhorst@gmail.com> * grpc-ecosystem#1128 unexport default query parameter parser and rename mux option Co-authored-by: Johan Brandhorst <johan.brandhorst@gmail.com> Fixes grpc-ecosystem#1128
* grpc-ecosystem#1128 make it possible to configure query param parser implementation * Apply suggestions from code review Makes names more consistent and improves docs Co-Authored-By: Johan Brandhorst <johan.brandhorst@gmail.com> * grpc-ecosystem#1128 make configured parser a global parser * update docs for configuring query parser Co-Authored-By: Johan Brandhorst <johan.brandhorst@gmail.com> * grpc-ecosystem#1128 unexport default query parameter parser and rename mux option Co-authored-by: Johan Brandhorst <johan.brandhorst@gmail.com> Fixes grpc-ecosystem#1128
Is there any example implementation for this? I was looking at the docs, didn't find any info |
The default implementation in both master and v2 implements the interface. You'll just have to design your own with the same signature. See https://github.com/grpc-ecosystem/grpc-gateway/blob/master/runtime/query.go#L37 |
Looks like there's some issue with
Any known workarounds for this? |
No, you can't use it with Gogo protobuf. |
Oh okay, thank you for the info @johanbrandhorst . We are trying to |
Any parameters not captured in the body or the path automatically become query parameters, are you sure you need to do anything? |
No issues in reading query parameters. But I need to typecast the query parameters before forwarding them to grpc server. We have some prefix encoded strings for clients as that gives better UX. our gRPC server uses bytes (proto definitions are bytes). We want to support strings from client and typecast them to bytes in grpc gateway middleware |
@anilcse you should be able to write anything you want in some kind of Go middleware. grpc-gateway doesn't have any provisions for doing this directly so you will have to write something custom up. |
Implementing changes for #1128
Added new mux Option to configure query parameter parser to use with instance of mux.