You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2016/10/23 02:27:40 UTC
zeppelin git commit: [Zeppelin-1555] Eliminate prefix in
PythonInterpreter exception
Repository: zeppelin
Updated Branches:
refs/heads/master 5cc7cc56c -> 1b2635cfe
[Zeppelin-1555] Eliminate prefix in PythonInterpreter exception
### What is this PR for?
Solve bug metioned [here](https://github.com/apache/zeppelin/blob/3dec4d7006b8a57136f34ae330ba937d8990f2d2/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java#L139)
Since we launch python interpreter as a process and redirect stdin and stdout, only exception occurred (like syntax error or indentation error, etc) could give string like `...`. Thus, we don't need to determine whether syntax error happened in [`PythonProcess.sendAndGetResult`](https://github.com/apache/zeppelin/blob/3dec4d7006b8a57136f34ae330ba937d8990f2d2/python/src/main/java/org/apache/zeppelin/python/PythonProcess.java#L86) because we have detected error in [`PythonInterpreter.pythonErrorIn`](https://github.com/apache/zeppelin/blob/3dec4d7006b8a57136f34ae330ba937d8990f2d2/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java#L152)
### What type of PR is it?
Bug Fix
### What is the Jira issue?
Jira: https://issues.apache.org/jira/browse/ZEPPELIN-1555
### How should this be tested?
Test locally.
### Screenshots
<img width="1175" alt="screen shot 2016-10-16 at 18 05 00" src="https://cloud.githubusercontent.com/assets/3419881/19422552/192a8b3a-93cb-11e6-89e8-63f2652a7f85.png">
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Author: Kai Jiang <ji...@gmail.com>
Closes #1530 from vectorijk/zeppelin-1555 and squashes the following commits:
8ffc360 [Kai Jiang] add unit test
d7a2ef4 [Kai Jiang] [zeppelin-1555] Eliminate prefix in PythonInterpreter exception
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/1b2635cf
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/1b2635cf
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/1b2635cf
Branch: refs/heads/master
Commit: 1b2635cfe679f43144f12fe79b348ab921ac70b8
Parents: 5cc7cc5
Author: Kai Jiang <ji...@gmail.com>
Authored: Mon Oct 17 12:52:55 2016 -0700
Committer: Lee moon soo <mo...@apache.org>
Committed: Sun Oct 23 11:27:32 2016 +0900
----------------------------------------------------------------------
.../zeppelin/python/PythonInterpreter.java | 10 +++---
.../apache/zeppelin/python/PythonProcess.java | 5 ---
...ythonInterpreterWithPythonInstalledTest.java | 32 ++++++++++++++++++++
3 files changed, 36 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b2635cf/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
----------------------------------------------------------------------
diff --git a/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java b/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
index 0561d86..568b75a 100644
--- a/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
+++ b/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
@@ -68,7 +68,7 @@ public class PythonInterpreter extends Interpreter {
@Override
public void open() {
- LOG.info("Starting Python interpreter .....");
+ LOG.info("Starting Python interpreter ---->");
LOG.info("Python path is set to:" + property.getProperty(ZEPPELIN_PYTHON));
maxResult = Integer.valueOf(getProperty(MAX_RESULT));
@@ -111,7 +111,7 @@ public class PythonInterpreter extends Interpreter {
@Override
public void close() {
- LOG.info("closing Python interpreter .....");
+ LOG.info("closing Python interpreter <----");
try {
if (process != null) {
process.close();
@@ -134,11 +134,9 @@ public class PythonInterpreter extends Interpreter {
InterpreterResult result;
if (pythonErrorIn(output)) {
- result = new InterpreterResult(Code.ERROR, output);
+ result = new InterpreterResult(Code.ERROR, output.replaceAll("\\.\\.\\.", ""));
} else {
- // TODO(zjffdu), we should not do string replacement operation in the result, as it is
- // possible that the output contains the kind of pattern itself, e.g. print("...")
- result = new InterpreterResult(Code.SUCCESS, output.replaceAll("\\.\\.\\.", ""));
+ result = new InterpreterResult(Code.SUCCESS, output);
}
return result;
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b2635cf/python/src/main/java/org/apache/zeppelin/python/PythonProcess.java
----------------------------------------------------------------------
diff --git a/python/src/main/java/org/apache/zeppelin/python/PythonProcess.java b/python/src/main/java/org/apache/zeppelin/python/PythonProcess.java
index 0ab1461..190b3da 100644
--- a/python/src/main/java/org/apache/zeppelin/python/PythonProcess.java
+++ b/python/src/main/java/org/apache/zeppelin/python/PythonProcess.java
@@ -91,11 +91,6 @@ public class PythonProcess {
String line = null;
while (!(line = reader.readLine()).contains(STATEMENT_END)) {
logger.debug("Read line from python shell : " + line);
- if (line.equals("...")) {
- logger.warn("Syntax error ! ");
- output.append("Syntax error ! ");
- break;
- }
output.append(line + "\n");
}
return output.toString();
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b2635cf/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java
----------------------------------------------------------------------
diff --git a/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java
index 38b46e7..383533b 100644
--- a/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java
+++ b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java
@@ -56,6 +56,8 @@ public class PythonInterpreterWithPythonInstalledTest {
//System.out.println("\nInterpreter response: \n" + ret.message());
assertEquals(InterpreterResult.Code.ERROR, ret.code());
assertTrue(ret.message().length() > 0);
+
+ realPython.close();
}
@Test
@@ -73,6 +75,36 @@ public class PythonInterpreterWithPythonInstalledTest {
//System.out.println("\nInterpreter response: \n" + ret.message());
assertEquals(InterpreterResult.Code.SUCCESS, ret.code());
assertTrue(ret.message().length() > 0);
+
+ realPython.close();
+ }
+
+ @Test
+ public void testZeppelin1555() {
+ //given
+ PythonInterpreter realPython = new PythonInterpreter(
+ PythonInterpreterTest.getPythonTestProperties());
+ realPython.open();
+
+ //when
+ InterpreterResult ret1 = realPython.interpret("print \"...\"", null);
+
+ //then
+ //System.out.println("\nInterpreter response: \n" + ret.message());
+ assertEquals(InterpreterResult.Code.SUCCESS, ret1.code());
+ assertEquals("...\n", ret1.message());
+
+
+ InterpreterResult ret2 = realPython.interpret("for i in range(5):", null);
+ //then
+ //System.out.println("\nInterpreterResultterpreter response: \n" + ret2.message());
+ assertEquals(InterpreterResult.Code.ERROR, ret2.code());
+ assertEquals(" File \"<stdin>\", line 2\n" +
+ " \n" +
+ " ^\n" +
+ "IndentationError: expected an indented block\n", ret2.message());
+
+ realPython.close();
}
}