steveloughran commented on a change in pull request #2178:
URL: https://github.com/apache/hadoop/pull/2178#discussion_r463232170



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
##########
@@ -528,6 +528,10 @@ private HadoopLoginContext getLogin() {
   private void setLogin(LoginContext login) {
     user.setLogin(login);
   }
+  
+  private void setLastLogin(long now) {

Review comment:
       add javadoc, note unit of time. Milliseconds, right?

##########
File path: 
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
##########
@@ -100,13 +101,34 @@ public void stopMiniKdc() {
       executor.shutdownNow();
     }
   }
+  
+  /**
+   * Login from keytab using the MiniKDC
+   */
+  @Test
+  public void testUGILoginFromKeytab() throws Exception {
+    long beforeLogin = Time.now();
+    String principal = "foo";
+    File keytab = new File(workDir, "foo.keytab");
+    kdc.createPrincipal(keytab, principal);
+
+    UserGroupInformation.loginUserFromKeytab(principal, keytab.getPath());
+    UserGroupInformation ugi = UserGroupInformation.getLoginUser();
+    Assert.assertTrue("UGI should be configured to login from keytab",
+        ugi.isFromKeytab());
+
+    User user = getUser(ugi.getSubject());
+    Assert.assertNotNull(user.getLogin());
+    Assert.assertTrue("User last login time is not set correctly",

Review comment:
       error text should include the values at fault, so we can debug better 
from nothing but a junit report

##########
File path: 
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
##########
@@ -121,6 +143,9 @@ public void testUGILoginFromKeytab() throws Exception {
     final long firstLogin = user.getLastLogin();
     final LoginContext login1 = user.getLogin();
     Assert.assertNotNull(login1);
+    
+    // Sleep for 1 sec to have a difference between first and second login
+    Thread.sleep(1000);

Review comment:
       if you really want >1s of interval, make it a 2s, delay. There's too 
much risk of clock/sleep granularity issues




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to