Set RPC-Thrift-Envelope HTTP header for Thrift requests #205
Conversation
See yarpc/yarpc-go/issues/741 for more details. This is part 1 in making yab work with YARPC regardless of envelope options.
| envelope := r.Header.Get(transport.HTTPThriftEnvelopeheader) | ||
| assert.Equal(t, tt.wantEnvelopeHeader, envelope) | ||
| })) | ||
| defer server.Close() |
There was a problem hiding this comment.
nit: Because this isn't a function, all the servers will keep running until the test exits. I would push this into an anonymous function.
There was a problem hiding this comment.
May as well add a t.Run wrapper in the loop.
| @@ -786,11 +787,17 @@ func TestWithTransportSerializer(t *testing.T) { | |||
| Type: wire.Call, | |||
| Value: wire.NewValueStruct(wire.Struct{}), | |||
| }), | |||
| wantTransportHeaders: map[string]string{ | |||
| "RPC-Thrift-Envelope": "true", | |||
There was a problem hiding this comment.
nit: pretty sure that the canonical form of the header key (as defined by net/http) is Rpc-Thrift-Envelope.
You could also just use https://golang.org/pkg/net/http/#CanonicalHeaderKey
| envelope := r.Header.Get(transport.HTTPThriftEnvelopeheader) | ||
| assert.Equal(t, tt.wantEnvelopeHeader, envelope) | ||
| })) | ||
| defer server.Close() |
There was a problem hiding this comment.
May as well add a t.Run wrapper in the loop.
|
@prashantv what is the status on this? |
|
I didn't want to merge this without making sure we were doing the same thing in YARPC. I had a WIP PR in YARPC: The YARPC request object does not have the flexibility to implement this, from the above PR: I'm not planning to work on this in YARPC till the above issue is resolved. |
See yarpc/yarpc-go/issues/741 for more details. This is the first step in making
yab work with YARPC for HTTP+Thrift transparently regardless of whether the request is enveloped or not.