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