You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Yuxuan Wang (Jira)" <ji...@apache.org> on 2019/10/19 16:17:00 UTC

[jira] [Comment Edited] (THRIFT-4914) Go: Link between THeader and context object

    [ https://issues.apache.org/jira/browse/THRIFT-4914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16955204#comment-16955204 ] 

Yuxuan Wang edited comment on THRIFT-4914 at 10/19/19 4:16 PM:
---------------------------------------------------------------

>I'm strongly against exposing the transport in handler functions. After all, you should be able to use the same handler for different transports.

Agreed.

What I was thinking is something like this:
{code:go}
type TResponseHelper interface {
    SetHeader(key, value string)
    ClearHeaders()
    // Other non-theader related stuff in the future
}

func GetResponseHelper(ctx context.Context) TResponseHelper
{code}
And we inject an implementation of TResponseHelper (basically a wrapped TTransport with very restrictive access, for non THeader transports the header related functions will just be a noop) into the context object passed into the handlers.

The advantage of this approach, comparing to the response struct, is:
 # This is non-breaking change
 # For people who don't care about setting headers in the responses, they don't need to make any change in the handler code


was (Author: fishywang):
What I was thinking is something like this:

{code:go}
type TResponseHelper interface {
    SetHeader(key, value string)
    ClearHeaders()
    // Other non-theader related stuff in the future
}

func GetResponseHelper(ctx context.Context) TResponseHelper
{code}

And we inject an implementation of TResponseHelper (basically a wrapped TTransport with very restrictive access, for non THeader transports the header related functions will just be a noop) into the context object passed into the handlers.

The advantage of this approach, comparing to the response struct, is:

# This is non-breaking change
# For people who don't care about setting headers in the responses, they don't need to make any change in the handler code

> 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
>          Time Spent: 0.5h
>  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)