steveloughran commented on code in PR #5315:
URL: https://github.com/apache/hadoop/pull/5315#discussion_r1081327340


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java:
##########
@@ -340,22 +337,14 @@ public void doGet(HttpServletRequest request, 
HttpServletResponse response
         out.println(MARKER
             + "Submitted Class Name: <b>" + logName + "</b><br />");
 
-        Log log = LogFactory.getLog(logName);
+        Logger log = Logger.getLogger(logName);
         out.println(MARKER
             + "Log Class: <b>" + log.getClass().getName() +"</b><br />");
         if (level != null) {
           out.println(MARKER + "Submitted Level: <b>" + level + "</b><br />");
         }
 
-        if (log instanceof Log4JLogger) {
-          process(((Log4JLogger)log).getLogger(), level, out);
-        }
-        else if (log instanceof Jdk14Logger) {
-          process(((Jdk14Logger)log).getLogger(), level, out);
-        }
-        else {
-          out.println("Sorry, " + log.getClass() + " not supported.<br />");
-        }
+        process(log, level, out);

Review Comment:
   still need to handle situations (logback..) where the logger is 
unknown/unsupported, right?



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LogAdapter.java:
##########
@@ -17,61 +17,40 @@
  */
 package org.apache.hadoop.util;
 
-import org.apache.commons.logging.Log;
 import org.slf4j.Logger;
 
 class LogAdapter {

Review Comment:
   this class is obsolete. tag as deprecated and see if all uses can be removed.



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LogAdapter.java:
##########
@@ -17,61 +17,40 @@
  */
 package org.apache.hadoop.util;
 
-import org.apache.commons.logging.Log;
 import org.slf4j.Logger;
 
 class LogAdapter {
-  private Log LOG;
-  private Logger LOGGER;
 
-  private LogAdapter(Log LOG) {
-    this.LOG = LOG;
-  }
+  private final Logger LOGGER;

Review Comment:
   given its not final, case is wrong. 
   
   also, it's never going to be null, so the checks can be cut down below



##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java:
##########
@@ -318,9 +316,9 @@ public class DataNode extends ReconfigurableBase
         ", srvID: %s" +  // DatanodeRegistration
         ", blockid: %s" + // block id
         ", duration(ns): %s";  // duration time
-        
-  static final Log ClientTraceLog =
-    LogFactory.getLog(DataNode.class.getName() + ".clienttrace");
+
+  static final Logger ClientTraceLog =

Review Comment:
   this sould really be CLIENT_TRACE_LOG, shouldn't it?



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestJarFinder.java:
##########
@@ -39,14 +38,6 @@
 
 public class TestJarFinder {
 
-  @Test

Review Comment:
   test should be replaced with anothe class we know is there, maybe one of the 
junit ones



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/IOUtils.java:
##########
@@ -256,7 +255,7 @@ public static void skipFully(InputStream in, long len) 
throws IOException {
    * instead
    */
   @Deprecated
-  public static void cleanup(Log log, java.io.Closeable... closeables) {
+  public static void cleanup(Logger log, java.io.Closeable... closeables) {

Review Comment:
   i'd prefer the method to be completely cut and mvoed to cleanupWithLogger; 
they are now the same methods so you can just invoke that from this and the use 
the IDE to remove the method



-- 
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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to