hapylestat commented on a change in pull request #3198: URL: https://github.com/apache/ambari/pull/3198#discussion_r437296355
########## File path: ambari-server/src/main/java/org/apache/ambari/server/utils/ScheduledExecutorCompletionService.java ########## @@ -35,6 +35,7 @@ super(task, null); this.task = task; } + @Override protected void done() { queue.add(task); } Review comment: can we also reformat the line to have proper formatting? (not one-liner) thanks ########## File path: ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkConnection.java ########## @@ -41,6 +41,7 @@ public static ZooKeeper open(String serverAddress, int sessionTimeoutMillis, int { final CountDownLatch connSignal = new CountDownLatch(1); ZooKeeper zooKeeper = new ZooKeeper(serverAddress, sessionTimeoutMillis, new Watcher() { Review comment: can we use here lambda instead without creating anonymous class? ########## File path: ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java ########## @@ -1463,6 +1465,7 @@ public void addLocalAuthentication(UserEntity userEntity, String password, boole UserAuthenticationType.LOCAL, encodedPassword, new Validator() { Review comment: let's do it using lambda approach ########## File path: ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java ########## @@ -1504,6 +1507,7 @@ public void addPamAuthentication(UserEntity userEntity, String userName, boolean UserAuthenticationType.PAM, userName, new Validator() { Review comment: let's do it using lambda approach ########## File path: ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java ########## @@ -1373,6 +1373,7 @@ public void addJWTAuthentication(UserEntity userEntity, String key, boolean pers UserAuthenticationType.JWT, key, new Validator() { Review comment: let's do it using lambda approach ########## File path: ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java ########## @@ -1545,6 +1549,7 @@ public void addLdapAuthentication(UserEntity userEntity, String dn, boolean pers UserAuthenticationType.LDAP, StringUtils.lowerCase(dn), // DNs are case-insensitive and are stored internally as the bytes of lowercase characters new Validator() { Review comment: let's do it using lambda approach ########## File path: ambari-server/src/main/java/org/apache/ambari/server/state/fsm/event/Event.java ########## @@ -26,5 +26,6 @@ TYPE getType(); long getTimestamp(); + @Override Review comment: why would we override method declared in the interface? Well, i have no clue if this method declaration makes any sense at all - coz it is present in the extended interface ########## File path: ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapBindAuthenticator.java ########## @@ -290,6 +290,7 @@ private DirContextOperations setAmbariAdminAttr(DirContextOperations user, LdapS LOG.debug("LDAP login - set admin attr filter: {}", setAmbariAdminAttrFilter); AttributesMapper attributesMapper = new AttributesMapper() { Review comment: let's do it using lambda approach ########## File path: ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java ########## @@ -1415,6 +1416,7 @@ public void addKerberosAuthentication(UserEntity userEntity, String principalNam UserAuthenticationType.KERBEROS, principalName, new Validator() { + @Override Review comment: let's do it using lambda approach ########## File path: ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkConnection.java ########## @@ -41,6 +41,7 @@ public static ZooKeeper open(String serverAddress, int sessionTimeoutMillis, int { final CountDownLatch connSignal = new CountDownLatch(1); ZooKeeper zooKeeper = new ZooKeeper(serverAddress, sessionTimeoutMillis, new Watcher() { Review comment: yep, as starting from Ambari-2.7 and higher, Ambari not supporting Java 7. ########## File path: ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkConnection.java ########## @@ -41,6 +41,7 @@ public static ZooKeeper open(String serverAddress, int sessionTimeoutMillis, int { final CountDownLatch connSignal = new CountDownLatch(1); ZooKeeper zooKeeper = new ZooKeeper(serverAddress, sessionTimeoutMillis, new Watcher() { Review comment: yep, as starting from Ambari-2.7 and higher, Ambari not supporting Java 7. So we can update language level without problem ---------------------------------------------------------------- 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: us...@infra.apache.org