Apache9 commented on code in PR #5315:
URL: https://github.com/apache/hadoop/pull/5315#discussion_r1091726604
##########
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:
Is it possible for our users to use a logging framework other than log4j?
##########
hadoop-project/pom.xml:
##########
@@ -1076,6 +1075,9 @@
<artifactId>jetty-servlet-tester</artifactId>
<version>${jetty.version}</version>
</dependency>
+ <!-- Hadoop's own codebase is not supposed to use commons-logging but
some of the
Review Comment:
Do we really need to declare this? If other dependencies depend on it, maven
will pull it in?
##########
hadoop-project/pom.xml:
##########
@@ -1199,6 +1196,12 @@
<artifactId>junit-platform-launcher</artifactId>
<version>${junit.platform.version}</version>
<scope>test</scope>
+ <exclusions>
Review Comment:
Why do we need to exclude transitive dependencies on commons-logging? These
libraries use commons-logging as logging facade, exclude it may cause problems?
##########
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:
Or maybe here we should use slf4j's logger instead of log4j's logger
directly? As the directly replacement for commons-logging is slf4j, which are
both logging facade, log4j is the actual implementation.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LogAdapter.java:
##########
@@ -17,62 +17,34 @@
*/
package org.apache.hadoop.util;
-import org.apache.commons.logging.Log;
import org.slf4j.Logger;
-class LogAdapter {
- private Log LOG;
- private Logger LOGGER;
+@Deprecated
+final class LogAdapter {
Review Comment:
There is no IA annotation for this class and it is package private, I think
here we could just remove it if we can remove all the references?
##########
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:
IA.Public and IS.Evolving means we can remove it when doing a minor release?
SO we can remove it in 3.4.0? For the whole 'removing log4j' work, I do not
think we can include it in 3.3.x...
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ReflectionUtils.java:
##########
@@ -223,35 +222,6 @@ public synchronized static void
printThreadInfo(PrintStream stream,
private static long previousLogTime = 0;
- /**
- * Log the current thread stacks at INFO level.
- * @param log the logger that logs the stack trace
- * @param title a descriptive title for the call stacks
- * @param minInterval the minimum time from the last
- */
- public static void logThreadInfo(Log log,
Review Comment:
Oh, another leaking of commons-logging class in IA.Public class...
--
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
