impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4129: Use KRPC's Kinit code to avoid expensive fork
Date Fri, 01 Sep 2017 21:57:43 GMT
Michael Ho has posted comments on this change.

Change subject: IMPALA-4129: Use KRPC's Kinit code to avoid expensive fork
......................................................................


Patch Set 2:

(8 comments)

Some quick comments.

http://gerrit.cloudera.org:8080/#/c/7938/2//COMMIT_MSG
Commit Message:

PS2, Line 9: subprocess
child process


PS2, Line 23:  util
utility


PS2, Line 33: Verified with thrift-server-test and also manually on a
            : live kerberized cluster.
Did you verify that our current Jenkins AMI will be able to find the necessary util to run
the tests ?


http://gerrit.cloudera.org:8080/#/c/7938/2/CMakeLists.txt
File CMakeLists.txt:

PS2, Line 305: find_package(KerberosPrograms REQUIRED)
Wouldn't build fail if it's not found ? Did you try packaging build ?


http://gerrit.cloudera.org:8080/#/c/7938/2/be/src/kudu/security/CMakeLists.txt
File be/src/kudu/security/CMakeLists.txt:

PS2, Line 89: set(SECURITY_TEST_SRCS_FOR_IMPALA
            :   test/mini_kdc.cc)
nit: this can be one line


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

PS2, Line 74: Remove this flag in a compatibility-breaking release
Is there a JIRA for that ?


Line 843:       KUDU_RETURN_IF_ERROR(kudu::security::InitKerberosForServer(), "Could not init
kerberos");
long line.


http://gerrit.cloudera.org:8080/#/c/7938/2/be/src/rpc/thrift-server-test.cc
File be/src/rpc/thrift-server-test.cc:

PS2, Line 231: ASSERT_FALSE(non_ssl_client.Open().ok());
Is it possible to match against the expected error status instead ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9cea56cc6e7412d87f4c2e92399a2f91ea6af6c7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message