Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8270 )

Change subject: IMPALA-5053: [SECURITY] Make KRPC work with Kerberos
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8270/4/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/8270/4/be/src/rpc/authentication.cc@593
PS4, Line 593: !FLAGS_principal.empty()
> What do you think about wrapping this inside an inline function ?
I wrapped it in a function, but I can't keep it inline, because inline 
functions need to be defined in the .h file (unless there's some other way to 
do it) and we can't access gflags members in the .h file.


http://gerrit.cloudera.org:8080/#/c/8270/3/be/src/util/auth-util.cc
File be/src/util/auth-util.cc:

http://gerrit.cloudera.org:8080/#/c/8270/3/be/src/util/auth-util.cc@90
PS3, Line 90: }
> It seems better to at least make sure / appears before @.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cec5cca5fdb4b1d46bab19e86cb1a8a3ad718fd
Gerrit-Change-Number: 8270
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Comment-Date: Sun, 19 Nov 2017 04:14:14 +0000
Gerrit-HasComments: Yes

Reply via email to