Add proper support for 'identity' encoding type #1664
Conversation
|
Just to be sure: did you verify that a Python client can now talk to a Go server with this change? |
|
@tsuna, I did not specifically test this using a Python client, but I added a test case that sets "identity" in the grpc-encoding header and makes sure the server behaves correctly. Would you be able to patch this change into your environment and test it? |
|
I can try it but it's not the first time we run into this sort of interop issue, it would be nice if upstream did better testing of interop between the various implementations of gRPC. |
|
We have interop tests, both in open source and inside Google, and I am following-up to figure out why they didn't catch this issue. |
Documentation/compression.md
Outdated
| [`WithDefaultCallOptions`](https://godoc.org/google.golang.org/grpc#WithDefaultCallOptions) | ||
| `DialOption`. | ||
|
|
||
| Server-side, registered compressors will be used automatically to decode request |
There was a problem hiding this comment.
Also mention what happens if the request compressor is not registered?
There was a problem hiding this comment.
Done, and added the same thing above in the case that UseCompressor specifies an unregistered compressor. (I left out the details about "identity", because I don't think it's worth mentioning, and it's covered in the bottom section.)
call.go
Outdated
| // otherwise set comp if a registered compressor exists for it. | ||
| var comp encoding.Compressor | ||
| var dc Decompressor | ||
| if rc := stream.RecvCompress(); dopts.dc == nil || dopts.dc.Type() != rc { |
There was a problem hiding this comment.
Reorder if and else?
if rc := stream.RecvCompress(); dopts.dc != nil && dopts.dc.Type() == rc {
dc = dopts.dc
} else if rc != "" && rc != encoding.Identity {
comp = encoding.GetCompressor(rc)
}
server.go
Outdated
|
|
||
| // If dc is set and matches the stream's compression, use it. Otherwise, try | ||
| // to find a matching registered compressor for decomp. | ||
| if rc := stream.RecvCompress(); s.opts.dc == nil || s.opts.dc.Type() != rc { |
There was a problem hiding this comment.
reorder if and else?
if rc := stream.RecvCompress(); s.opts.dc != nil && s.opts.dc.Type() == rc {
dc = s.opts.dc
} else if rc != "" && rc != encoding.Identity {
decomp = encoding.GetCompressor(rc)
}
stream.go
Outdated
| @@ -637,6 +666,7 @@ func (ss *serverStream) SendMsg(m interface{}) (err error) { | |||
| } | |||
|
|
|||
| func (ss *serverStream) RecvMsg(m interface{}) (err error) { | |||
|
|
|||
vet.sh
Outdated
| @@ -76,5 +76,5 @@ if [[ "$check_proto" = "true" ]]; then | |||
| fi | |||
|
|
|||
| # TODO(menghanl): fix errors in transport_test. | |||
| staticcheck -ignore google.golang.org/grpc/transport/transport_test.go:SA2002 ./... | |||
| staticcheck -ignore "google.golang.org/grpc/transport/transport_test.go:SA2002 google.golang.org/grpc/benchmark/benchmain/main.go:SA1019 google.golang.org/grpc/stats/stats_test.go:SA1019 google.golang.org/grpc/test/end2end_test.go:SA1019" ./... | |||
There was a problem hiding this comment.
How about this: create a file staticcheck.ignore, and do
staticcheck -ignore "$(cat staticcheck.ignore)" ./...
I also added a document to explain how this is supposed to work, and how it works if you use the legacy compressor/decompressor.
Fixes #1654