You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by GitBox <gi...@apache.org> on 2020/06/09 16:31:28 UTC

[GitHub] [ambari] hapylestat commented on a change in pull request #3198: AMBARI-25516. Missing @Override on 91 methods

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:
users@infra.apache.org