voodoo-dn edited a comment on pull request #1992:
URL: https://github.com/apache/thrift/pull/1992#issuecomment-705765002
> In your interceptor implementations both requests and responses are
`interface{}`, e.g. typeless. Why can they not be `thrift.TStruct` instead?
You are totally right, it this TStruct instead of interface{} is very good
solution.
> Middlewares can do everything in the list you mentioned, but without the
requests and responses. And looking at your example code, they are only used in
logging. So it seems to me that they are not something required to function,
but more like nice to have things.
Are you sure? Logging is like "nice to have"? How you will develop
something, where request and responses in binary data and you do not know what
data creates errors in you application? Do you think that is better to add 100
calls to logger for every thrift handler?
> So to summarize my impression is that this is a relatively high price to
pay (with compiler changes and weaker type system) for a relatively low gain
(as I mentioned in [the discussion in the
ticket](https://issues.apache.org/jira/browse/THRIFT-5164?focusedCommentId=17134411&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17134411),
it's doable in middleware as well, just more complex). I'm also doubtful of
the usefulness of blindly logging all requests/responses for every request in
every endpoint.
Weak type system will be fixed if you are agree to merge this MR.
Guys, your project is great, but let's face it:
gRPC is more popular(without great things as Exception, that your project
supports), there is bigger community, it's contains such things as
Interceptor(what is from your vision is useless) from the box. Without new
features which scares you, your project become less popular and will die
without community supporting. I don't injure you, but your relation to new
features reflect attitude to community. I do not say: if you do not merge it,
we will move our projects to gRPC, not, i'm trying to understand why such great
feature as Interceptor, which is done not by you, you didn't spent time on
coding and testing(this Interceptor is using half year in production without
any problems by our company) makes such problems.
> Also I'm curious: how do you implement your `maskArg` function so it works
for all your different request types? (Serhii mentioned that they have 100
endpoints, and I assume they are from the same company as you?)
What is maskArg do: marshal args to json, then we cut from json some
sensitive data and log this json.
Yes, we work with Serhii in one team.
Sorry if I offended you in some way. Have a good time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
