Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9050 )
Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials ...................................................................... Patch Set 4: (11 comments) http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@140 PS3, Line 140: * the ticket cache to obtain a new ticket with a later expiration time. So, if Do we need to mention that Kudu will refuse the re-login attempt if the principal switched? http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java: http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@95 PS3, Line 95: static 'static' is redundant for inner enums. http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@107 PS3, Line 107: . Nit: add ')' http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@130 PS3, Line 130: * @param subject JAAS Subject that the client's credentials are stored in Nit: remove the extra @param. http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@171 PS3, Line 171: public void refreshSubject() { Nit: Maybe add a brief comment in front of the function since it is public. http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@191 PS3, Line 191: Extra line. http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java: http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@55 PS3, Line 55: static Redundant. http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@118 PS3, Line 118: miniCluster.killMasters(); I do not quite follow why do we have to kill the masters. Does it mean as long as the client has an existing connection, even though the credentials expired the client is still able to work fine? http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@273 PS3, Line 273: public void testRenewAndReacquireKeberosCredentials() throws Exception { Nit: addd a comment here? http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@355 PS3, Line 355: Closeable c Not used? http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@386 PS3, Line 386: Closeable c Same here. -- To view, visit http://gerrit.cloudera.org:8080/9050 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539 Gerrit-Change-Number: 9050 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Anonymous Coward #380 Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 07 Mar 2018 21:05:04 +0000 Gerrit-HasComments: Yes
