Reamer commented on code in PR #4793:
URL: https://github.com/apache/zeppelin/pull/4793#discussion_r1730858430
##########
python/src/test/java/org/apache/zeppelin/python/PythonInterpreterPandasSqlTest.java:
##########
@@ -58,7 +58,7 @@
abstract class PythonInterpreterPandasSqlTest {
private static final Logger LOGGER =
- LoggerFactory.getLogger(PythonInterpreterPandasSqlTest.class);
+ LoggerFactory.getLogger(PythonInterpreterPandasSqlTest.class);
Review Comment:
Unnecessary change.
##########
file/src/test/java/org/apache/zeppelin/file/HDFSFileInterpreterTest.java:
##########
@@ -34,6 +34,7 @@
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.junit.jupiter.api.Test;
+import org.slf4j.LoggerFactory;
Review Comment:
Please sort the imports here. I would delete `import org.slf4j.logger;` in
line `26` and add it here.
##########
zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java:
##########
@@ -1206,20 +1206,20 @@ private void patchParagraph(NotebookSocket conn,
if (!zConf.isZeppelinNotebookCollaborativeModeEnable()) {
return;
}
- String paragraphId = fromMessage.getType("id", LOG);
+ String paragraphId = fromMessage.getType("id", LOGGER);
Review Comment:
These lines really made me wonder. Passing the logger into a function so
that another class can log via it...
However, your change at this point is correct.
##########
file/src/main/java/org/apache/zeppelin/file/HDFSFileInterpreter.java:
##########
@@ -33,11 +33,14 @@
import org.apache.zeppelin.interpreter.InterpreterContext;
import org.apache.zeppelin.interpreter.InterpreterException;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* HDFS implementation of File interpreter for Zeppelin.
*/
public class HDFSFileInterpreter extends FileInterpreter {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(HDFSFileInterpreter.class);
Review Comment:
:+1:
It is good that these changes are included with this unification.
##########
zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java:
##########
@@ -1605,7 +1605,7 @@ private void runAllParagraphs(NotebookSocket conn,
});
}
} catch (Throwable t) {
- NotebookServer.LOG.error("Error in running all paragraphs", t);
+ NotebookServer.LOGGER.error("Error in running all paragraphs", t);
Review Comment:
```suggestion
LOGGER.error("Error in running all paragraphs", t);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
