encoding/thrift: Add mechanisms to perform outbound requests via thriftrw's "nowire" implementation

[witriew]

thriftrw's 'nowire' implementation (known as 'streaming' in the thriftrw
context) is an alternative wire-compatible implementation of the Thrift
protocol that bypasses the intermediate 'thriftrw.Wire' representation and
'streams' the protocol directly to the related objects.

This provides a no-wire compatible Client that can perform requests using
thriftrw's 'nowire' implementation. Much of this code is similar to the
existing Clients that use the 'wire' implementation of thriftrw, except that the
new no-wire Client provides an additional method Enabled() that returns
whether or not this client should be used, if set as an incoming option.
Additionally, the response's body is passed in to properly perform the
'Decode'ing of the protocol into the struct itself.

[CLAassistant]

All committers have signed the CLA.

[witriew]

An alternative to creating a new Client interface and corresponding struct for implementing the "no-wire" representation would be to have the existing thriftClient implement the new NoWireClient, but this would create a bit of stuttering and, I feel, poor form in the names of the methods in the NoWireClient interface.

Since Go doesn't allow for multiple names of the same method defined in a struct, the names of the methods would likely have a separate verb. While generally this is probably fine, having the same struct implement two different 'verbages' to satisfy wire and no-wire ThriftRW compatibility feels really awkward. My preference here was to use the same verbage and create a completely independent client.

[codecov]

Codecov Report

Merging #2088 (2e703cb) into nowiredev (6828a09) will increase coverage by 0.05%.
The diff coverage is 91.36%.

@@              Coverage Diff              @@
##           nowiredev    #2088      +/-   ##
=============================================
+ Coverage      87.56%   87.62%   +0.05%     
=============================================
  Files            248      249       +1     
  Lines          13794    13933     +139     
=============================================
+ Hits           12079    12209     +130     
- Misses          1331     1335       +4     
- Partials         384      389       +5     

Impacted Files	Coverage Δ
encoding/thrift/outbound.go	84.61% <ø> (ø)
encoding/thrift/thriftrw-plugin-yarpc/client.go	100.00% <ø> (ø)
encoding/thrift/outbound_nowire.go	88.78% <88.78%> (ø)
encoding/thrift/envelope.go	100.00% <100.00%> (ø)
encoding/thrift/inject.go	100.00% <100.00%> (ø)
encoding/thrift/multiplex.go	100.00% <100.00%> (ø)
transport/tchannel/peer.go	94.73% <0.00%> (+1.31%)	⬆️
encoding/thrift/options.go	92.30% <0.00%> (+7.69%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6828a09...2e703cb. Read the comment docs.

[abhinav]

I don't love this if-else but I don't see any other straightforward way to make this an opt-in flag so I can live with this.

[Dogild]

This is fine I think. Only other way would be to duplicate the last return and have something like this:

	var result test.TestService_Call_Result
 	args := test.TestService_Call_Helper.Args(_Key)

 	if c.nwc != nil && c.nwc.Enabled() {
 		if err = c.nwc.Call(ctx, args, &result, opts...); err != nil {
 			return
 		}
 		
 		success, err = test.TestService_Call_Helper.UnwrapResponse(&result)
	        return
 	} 
 	
        var body wire.Value
 	body, err = c.c.Call(ctx, args, opts...)
 	if err != nil {
 		return
 	}

 	if err = result.FromWire(body); err != nil {
 		return
 	}

        success, err = test.TestService_Call_Helper.UnwrapResponse(&result)
        return

This improves the readability IMO but this is fine to have the else - up to you @witriew
nit: err can be inline

[Dogild]

This is fine I think. Only other way would be to duplicate the last return and have something like this:

	var result test.TestService_Call_Result
 	args := test.TestService_Call_Helper.Args(_Key)

 	if c.nwc != nil && c.nwc.Enabled() {
 		if err = c.nwc.Call(ctx, args, &result, opts...); err != nil {
 			return
 		}
 		
 		success, err = test.TestService_Call_Helper.UnwrapResponse(&result)
	        return
 	} 
 	
        var body wire.Value
 	body, err = c.c.Call(ctx, args, opts...)
 	if err != nil {
 		return
 	}

 	if err = result.FromWire(body); err != nil {
 		return
 	}

        success, err = test.TestService_Call_Helper.UnwrapResponse(&result)
        return

This improves the readability IMO but this is fine to have the else - up to you @witriew
nit: err can be inline