Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Upgrade thrift to 0.9.3-p3
......................................................................


Patch Set 1:

> (5 comments)
 >
 > I'm not really qualified to review this change, but what I read
 > looked good to me. Sailesh, please look at the SSL stuff in
 > particular?
 >
 > Do we worry that somebody will accidentally use <<ostream stuff on
 > a Thrift object? I saw you converted a bunch of uses here. What
 > would happen if someone did LOG(INFO) << thriftObject.foo()?

I think accidentally calling << on a thrift object in logging is fine. The 
output is readable, and you will be able to notice it when you are adding 
logging code. The downside of actually using it is that thrift may change it 
from version to version.


--
To view, visit http://gerrit.cloudera.org:8080/9168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Tianyi Wang <[email protected]>
Gerrit-Comment-Date: Thu, 01 Feb 2018 02:39:59 +0000
Gerrit-HasComments: No

Reply via email to