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