[
https://issues.apache.org/jira/browse/THRIFT-5164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17134473#comment-17134473
]
Duru Can Celasun edited comment on THRIFT-5164 at 6/12/20, 7:35 PM:
--------------------------------------------------------------------
I think both approaches have their merits. [~fishywang]'s minimal approach is
cleaner by being limited to the library, which is why I quite liked it and
accepted the PR.
The [other one|https://github.com/apache/thrift/pull/1992] is much heavier and
requires extensive changes to the compiler. I'm not opposed to it in principle
and I like that it makes it easier to intercept RPC parameters, but it adds a
lot of verbosity and duplicated code to the generator output for something
that's not strictly necessary. As [~fishywang] showed above,
ProcessorMiddleware can be used to the same effect, it just needs a bit of
manual mapping.
There is also the fact that the thrift project is understaffed nowadays and
active contributors who understand the compiler are few, so I'm hesitant to
increase our maintenance burden when the same functionality can be achieved in
user code.
That said, I still haven't made up my mind about it so the PR remains open.
was (Author: calcifer):
I think both approaches have their merits. [~fishywang]'s minimal approach is
cleaner by being limited to the library, which is why I quite liked it and
accepted the PR.
The [other one|https://github.com/apache/thrift/pull/1992] is much heavier and
requires extensive changes to the compiler. I'm not opposed to it in principle,
but it adds a lot of verbosity and duplicated code to the generator output for
something that's not strictly necessary. As [~fishywang] showed above,
ProcessorMiddleware can be used to the same effect, it just needs a bit of
manual mapping.
There is also the fact that the thrift project is understaffed nowadays and
active contributors who understand the compiler are few, so I'm hesitant to
increase our maintenance burden when the same functionality can be achieved in
user code.
That said, I still haven't made up my mind about it so the PR remains open.
> Go middleware support
> ---------------------
>
> Key: THRIFT-5164
> URL: https://issues.apache.org/jira/browse/THRIFT-5164
> Project: Thrift
> Issue Type: Improvement
> Components: Go - Library
> Reporter: Yuxuan Wang
> Assignee: Duru Can Celasun
> Priority: Major
> Labels: Breaking-Change
> Fix For: 0.14.0
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> I saw that this idea was discussed before in THRIFT-4553, but want to reopen
> the discussion to see if there's a change of mind.
> We (Reddit) recently implemented TProcessor level middleware support in our
> Baseplate.go library, and we think that our implementation is generic enough
> that it might make sense to contribute that into the thrift library. The
> related implementation are all in this file:
> https://github.com/reddit/baseplate.go/blob/4225e42dc8dde56b222ac7ea3a4ff63aa726f6a6/thriftbp/middlewares.go
> https://github.com/reddit/baseplate.go/blob/4225e42dc8dde56b222ac7ea3a4ff63aa726f6a6/tracing/middlewares.go#L40-L64
> is an example of how a tracing middleware is implemented.
> If we think that's a good idea, then we can contribute that into thrift repo
> (with renames, of course). One implementation detail I'm not sure is that
> whether we want to expand the TProcessor interface to also include
> ProcessorMap and AddToProcessorMap, or do we want to keep the two iinterfaces
> separate.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
