Reamer commented on a change in pull request #3925:
URL: https://github.com/apache/zeppelin/pull/3925#discussion_r516099691
##########
File path:
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java
##########
@@ -171,7 +171,7 @@ public SparkInterpreterLauncher(ZeppelinConfiguration
zConf, RecoveryStorage rec
sparkConfBuilder.append(" --proxy-user " + context.getUserName());
}
- env.put("ZEPPELIN_SPARK_CONF",
escapeSpecialCharacter(sparkConfBuilder.toString()));
+ env.put("ZEPPELIN_SPARK_CONF", sparkConfBuilder.toString());
Review comment:
Escaping 'ZEPPELIN_SPARK_CONF' is no longer necessary, because all
values are passed to 'exec' as string arguments. The bash escaped string values
automatically.
Because of `exec` the bash process is replaced by the jvm process, so it is
not possible to insert further commands after `exec`.
What do you think?
##########
File path: rlang/src/test/java/org/apache/zeppelin/r/ShinyInterpreterTest.java
##########
@@ -221,7 +221,7 @@ public void testInvalidShinyApp()
resultMessages = context2.out.toInterpreterResultMessage();
assertTrue(resultMessages.get(1).getData(),
- resultMessages.get(1).getData().contains("object 'Invalid_code'
not found"));
+ resultMessages.get(1).getData().contains("'Invalid_code'"));
Review comment:
My system language is German. When I run tests locally, some tests fail
because the JVM translates the error message into German. With commit
https://github.com/apache/zeppelin/pull/3925/commits/ea099abef02a0b971c863c1bb1b70aa149bb74fc
I have removed the part that is translated by the JVM.
##########
File path:
python/src/test/java/org/apache/zeppelin/python/IPythonInterpreterTest.java
##########
@@ -475,7 +475,7 @@ public void testIPythonFailToLaunch() throws
InterpreterException {
fail("Should not be able to start IPythonInterpreter");
} catch (InterpreterException e) {
String exceptionMsg = ExceptionUtils.getStackTrace(e);
- assertTrue(exceptionMsg, exceptionMsg.contains("No such file or
directory"));
+ assertTrue(exceptionMsg, exceptionMsg.contains("java.io.IOException"));
Review comment:
My system language is German. When I run tests locally, some tests fail
because the JVM translates the error message into German. With commit
https://github.com/apache/zeppelin/pull/3925/commits/ea099abef02a0b971c863c1bb1b70aa149bb74fc
I have removed the part that is translated by the JVM.
##########
File path:
zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
##########
@@ -1232,7 +1234,7 @@ public void testPerSessionInterpreterCloseOnNoteRemoval()
throws IOException, In
setting.getOption().setPerNote(setting.getOption().SCOPED);
notebook.getInterpreterSettingManager().restart(setting.getId());
}
-
+ Thread.sleep(2000);
Review comment:
I add this lines during debugging because of failed tests. But this was
before adding this [magic
line](https://github.com/apache/zeppelin/pull/3925/files#diff-87d8954289365d4fc9ab53cd9b2257ac28cbdef284ec83565e4d5bf9bafc6eedR248),
which removes the shutdownhook thread. I'll try to revert the sleep statements.
I added these lines during debugging because of failed tests. But that was
before adding this [magic
line](https://github.com/apache/zeppelin/pull/3925/files#diff-87d8954289365d4fc9ab53cd9b2257ac28cbdef284ec83565e4d5bf9bafc6eedR248)
that removes the shutdownhook thread in case of a regular shutdown. I will try
to remove the sleep statements.
##########
File path:
zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
##########
@@ -1232,7 +1234,7 @@ public void testPerSessionInterpreterCloseOnNoteRemoval()
throws IOException, In
setting.getOption().setPerNote(setting.getOption().SCOPED);
notebook.getInterpreterSettingManager().restart(setting.getId());
}
-
+ Thread.sleep(2000);
Review comment:
I added these lines during debugging because of failed tests. But that
was before adding this [magic
line](https://github.com/apache/zeppelin/pull/3925/files#diff-87d8954289365d4fc9ab53cd9b2257ac28cbdef284ec83565e4d5bf9bafc6eedR248)
that removes the shutdownhook thread in case of a regular shutdown. I will try
to remove the sleep statements.
##########
File path:
zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
##########
@@ -1232,7 +1234,7 @@ public void testPerSessionInterpreterCloseOnNoteRemoval()
throws IOException, In
setting.getOption().setPerNote(setting.getOption().SCOPED);
notebook.getInterpreterSettingManager().restart(setting.getId());
}
-
+ Thread.sleep(2000);
Review comment:
I have removed the Sleep Statements.
----------------------------------------------------------------
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:
[email protected]
