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 <[email protected]> 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 <[email protected]> Authored: Mon Oct 17 12:52:55 2016 -0700 Committer: Lee moon soo <[email protected]> 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(); } }
![https://cloud.githubusercontent.com/assets/3419881/19422552/192a8b3a-93cb-11e6-89e8-63f2652a7f85.png"](https://cloud.githubusercontent.com/assets/3419881/19422552/192a8b3a-93cb-11e6-89e8-63f2652a7f85.png"){kind=link}