[ https://issues.apache.org/jira/browse/THRIFT-4914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17269948#comment-17269948 ]
Yuxuan Wang commented on THRIFT-4914: ------------------------------------- [~dcelasun] After wrote https://github.com/apache/thrift/pull/2314, thinking about it more, I'm starting to think that maybe we should make a bigger breaking change to make TClient.Call return both THeaderMap and error. The current 2 step way (first use Call, then use the helper function to get the response header map), makes it impossible for a client pool implementation to abstract the pool part away. Ideally we would want a client pool implementation to just implement the Call function by getting a client from the pool, call its Call, then return the client back to the pool (this is how we currently implement client pool in https://github.com/reddit/baseplate.go/blob/fe86cb54978d69b04f3d7817ef2cbdb2d3901ff3/thriftbp/client_pool.go#L484-L499, but we do not support response headers currently). Making Call return both resolves that problem. Also making change to TClient.Call might not be as big as a breaking change as we thought it would be. For most users they don't really interact with TClient.Call directly. They create a TStandardClient, then feed that into the client from thrift compiled code, and call the service methods instead of using the low-level Call. So with this change, we just make a compiler change, so that the compiled clients will just store the header part, and provide a LastResponseHeaders (name tbd) function to return the stored header. To give a concrete example, say we have a thrift service of: {code} service Service { i32 add(1: i32 a, 2: i32 b) } {code} Then users would just write go code like: {code:go} client := pkg.NewServiceClient(thrift.NewTStandardClient(...)) result, err := client.Add(a, b) headers := client.LastResponseHeaders() {code} What do you think? > Go: Link between THeader and context object > ------------------------------------------- > > Key: THRIFT-4914 > URL: https://issues.apache.org/jira/browse/THRIFT-4914 > Project: Thrift > Issue Type: New Feature > Components: Go - Library > Reporter: Yuxuan Wang > Priority: Major > Fix For: 0.14.0 > > Time Spent: 1h > Remaining Estimate: 0h > > We have "raw" THeader support for Go in THRIFT-4612 now, the next step would > be to make them more easily accessible. > The are 2 directions, 4 parts of this ticket: > * client -> server (requests) > ** Read headers on server > ** Write headers on client > * server -> client (responses) > ** Write headers on server > ** Read headers on client > Take the reading on server as an example. Currently we can read the headers > from either the transport or the protocol, but neither the transport nor the > protocol objects are "easily accessible" when you are writing the business > logic code (writing the request handler in the server code). It would be much > better if we inject the headers into the context object passed into the > request handlers. > We already have code injecting the headers to the context object that lives > outside of the thrift library working. I'll send out a PR in the coming days > to add that to the thrift library, so the performance would be better (we no > longer need to do the extra injecting work), and it would be a lot easier to > use. > We'll think about how to best do the client writing part (probably auto read > the headers from the context object that passed into the client request code, > and write to THeaderProtocol automatically), and send out a PR soon-ish. > The other direction, server -> client, is used much less often with headers, > so we'll probably punt on it for now, and come back revisit them in the > future. -- This message was sent by Atlassian Jira (v8.3.4#803005)