Repository: zeppelin Updated Branches: refs/heads/master 867facb6e -> 2ee7f48cf
ZEPPELIN-1105: Python - add paragraph ERROR status ### What is this PR for? Implement paragraph ERROR status for Python interpreter in case of Error or Exception in the output. ### What type of PR is it? Improvement ### What is the Jira issue? [ZEPPELIN-1105](https://issues.apache.org/jira/browse/ZEPPELIN-1105) ### How should this be tested? CI should pass, or ``` mvn "-Dtest=org.apache.zeppelin.python.PythonInterpreterWithPythonInstalledTest" test -pl python ``` should pass, or paragraph status should be ERROR for something like ``` import some-thing ``` ### Screenshots (if appropriate)  ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Alexander Bezzubov <[email protected]> Closes #1124 from bzz/ZEPPELIN-1105/python/add-paragraph-error-statu and squashes the following commits: a7bf8f3 [Alexander Bezzubov] Python: add missing license header b585982 [Alexander Bezzubov] Python: include Python-dependant tests to 1 CI profile e7d5371 [Alexander Bezzubov] Python: add ERROR paragraph status on Error and Exception in output 4c1107b [Alexander Bezzubov] Refactoring: rename and extract var assignment Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/2ee7f48c Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/2ee7f48c Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/2ee7f48c Branch: refs/heads/master Commit: 2ee7f48cff6ded6be6a8bb905b89e48336923d0e Parents: 867facb Author: Alexander Bezzubov <[email protected]> Authored: Mon Jul 4 22:35:46 2016 +0900 Committer: Alexander Bezzubov <[email protected]> Committed: Tue Jul 5 17:57:22 2016 +0900 ---------------------------------------------------------------------- .travis.yml | 2 +- python/pom.xml | 26 ++++++--- .../zeppelin/python/PythonInterpreter.java | 37 +++++++++--- .../zeppelin/python/PythonInterpreterTest.java | 14 ++++- ...ythonInterpreterWithPythonInstalledTest.java | 61 ++++++++++++++++++++ 5 files changed, 121 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2ee7f48c/.travis.yml ---------------------------------------------------------------------- diff --git a/.travis.yml b/.travis.yml index 8ea1a40..eec3b7c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,7 +35,7 @@ matrix: include: # Test all modules - jdk: "oraclejdk7" - env: SPARK_VER="1.6.1" HADOOP_VER="2.3" PROFILE="-Pspark-1.6 -Pr -Phadoop-2.3 -Ppyspark -Psparkr -Pscalding -Pexamples" BUILD_FLAG="package -Pbuild-distr" TEST_FLAG="verify -Pusing-packaged-distr" TEST_PROJECTS="" + env: SPARK_VER="1.6.1" HADOOP_VER="2.3" PROFILE="-Pspark-1.6 -Pr -Phadoop-2.3 -Ppyspark -Psparkr -Pscalding -Pexamples" BUILD_FLAG="package -Pbuild-distr" TEST_FLAG="verify -Pusing-packaged-distr" TEST_PROJECTS="-Dpython.test.exclude=''" # Test spark module for 1.5.2 - jdk: "oraclejdk7" http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2ee7f48c/python/pom.xml ---------------------------------------------------------------------- diff --git a/python/pom.xml b/python/pom.xml index b91142e..c98e35f 100644 --- a/python/pom.xml +++ b/python/pom.xml @@ -35,6 +35,7 @@ <properties> <py4j.version>0.9.2</py4j.version> + <python.test.exclude>**/PythonInterpreterWithPythonInstalledTest.java</python.test.exclude> </properties> <dependencies> @@ -67,7 +68,7 @@ <groupId>org.slf4j</groupId> <artifactId>slf4j-log4j12</artifactId> </dependency> - + <dependency> <groupId>junit</groupId> <artifactId>junit</artifactId> @@ -89,16 +90,26 @@ <plugins> <plugin> <artifactId>maven-enforcer-plugin</artifactId> - <version>1.3.1</version> - <executions> - <execution> - <id>enforce</id> - <phase>none</phase> + <version>1.3.1</version> + <executions> + <execution> + <id>enforce</id> + <phase>none</phase> </execution> </executions> </plugin> <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-surefire-plugin</artifactId> + <configuration> + <excludes> + <exclude>${python.test.exclude}</exclude> + </excludes> + </configuration> + </plugin> + + <plugin> <artifactId>maven-dependency-plugin</artifactId> <version>2.8</version> <executions> @@ -135,11 +146,12 @@ <version>${project.version}</version> <type>${project.packaging}</type> </artifactItem> - </artifactItems> + </artifactItems> </configuration> </execution> </executions> </plugin> + </plugins> </build> http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2ee7f48c/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 06e0c99..c985a54 100644 --- a/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java +++ b/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java @@ -24,6 +24,8 @@ import java.net.ServerSocket; import java.util.Collection; import java.util.List; import java.util.Properties; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.zeppelin.display.GUI; import org.apache.zeppelin.interpreter.Interpreter; @@ -34,7 +36,6 @@ import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; import org.apache.zeppelin.scheduler.Job; import org.apache.zeppelin.scheduler.Scheduler; import org.apache.zeppelin.scheduler.SchedulerFactory; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,8 +55,9 @@ public class PythonInterpreter extends Interpreter { private Integer port; private GatewayServer gatewayServer; - private Boolean py4J = false; + private Boolean py4JisInstalled = false; private InterpreterContext context; + private Pattern errorInLastLine = Pattern.compile(".*(Error|Exception): .*$"); private int maxResult; PythonProcess process = null; @@ -91,7 +93,8 @@ public class PythonInterpreter extends Interpreter { LOG.error("Can't execute " + BOOTSTRAP_PY + " to initiate python process", e); } - if (py4J = isPy4jInstalled()) { + py4JisInstalled = isPy4jInstalled(); + if (py4JisInstalled) { port = findRandomOpenPortOnAllLocalInterfaces(); LOG.info("Py4j gateway port : " + port); try { @@ -123,13 +126,31 @@ public class PythonInterpreter extends Interpreter { @Override public InterpreterResult interpret(String cmd, InterpreterContext contextInterpreter) { - this.context = contextInterpreter; if (cmd == null || cmd.isEmpty()) { return new InterpreterResult(Code.SUCCESS, ""); } + this.context = contextInterpreter; String output = sendCommandToPython(cmd); - return new InterpreterResult(Code.SUCCESS, output.replaceAll(">>>", "") - .replaceAll("\\.\\.\\.", "").trim()); + + InterpreterResult result; + if (pythonErrorIn(output)) { + result = new InterpreterResult(Code.ERROR, output.replaceAll(">>>", "").trim()); + } else { + result = new InterpreterResult(Code.SUCCESS, output.replaceAll(">>>", "") + .replaceAll("\\.\\.\\.", "").trim()); + } + return result; + } + + /** + * Checks if there is a syntax error or an exception + * + * @param output Python interpreter output + * @return true if syntax error or exception has happened + */ + private boolean pythonErrorIn(String output) { + Matcher errorMatcher = errorInLastLine.matcher(output); + return errorMatcher.find(); } @Override @@ -195,7 +216,7 @@ public class PythonInterpreter extends Interpreter { return output; } - private void bootStrapInterpreter(String file) throws IOException { + void bootStrapInterpreter(String file) throws IOException { BufferedReader bootstrapReader = new BufferedReader( new InputStreamReader( PythonInterpreter.class.getResourceAsStream(file))); @@ -205,7 +226,7 @@ public class PythonInterpreter extends Interpreter { while ((line = bootstrapReader.readLine()) != null) { bootstrapCode += line + "\n"; } - if (py4J && port != null && port != -1) { + if (py4JisInstalled && port != null && port != -1) { bootstrapCode = bootstrapCode.replaceAll("\\%PORT\\%", port.toString()); } LOG.info("Bootstrap python interpreter with code from \n " + file); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2ee7f48c/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java ---------------------------------------------------------------------- diff --git a/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java index 92fde2d..c9fee13 100644 --- a/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java +++ b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java @@ -17,7 +17,9 @@ package org.apache.zeppelin.python; -import static org.apache.zeppelin.python.PythonInterpreter.*; +import static org.apache.zeppelin.python.PythonInterpreter.DEFAULT_ZEPPELIN_PYTHON; +import static org.apache.zeppelin.python.PythonInterpreter.MAX_RESULT; +import static org.apache.zeppelin.python.PythonInterpreter.ZEPPELIN_PYTHON; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -45,8 +47,12 @@ import org.mockito.stubbing.Answer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + /** * Python interpreter unit test + * + * Important: ALL tests here DO NOT REQUIRE Python to be installed + * If Python dependency is required, please look at PythonInterpreterWithPythonInstalledTest */ public class PythonInterpreterTest { private static final Logger LOG = LoggerFactory.getLogger(PythonProcess.class); @@ -91,7 +97,8 @@ public class PythonInterpreterTest { * If Py4J is not installed, bootstrap_input.py * is not sent to Python process and py4j JavaGateway is not running */ - @Test public void testPy4jIsNotInstalled() { + @Test + public void testPy4jIsNotInstalled() { pythonInterpreter.open(); assertNull(pythonInterpreter.getPy4jPort()); assertTrue(cmdHistory.contains("def help()")); @@ -108,7 +115,8 @@ public class PythonInterpreterTest { * * @throws IOException */ - @Test public void testPy4jInstalled() throws IOException { + @Test + public void testPy4jInstalled() throws IOException { when(mockPythonProcess.sendAndGetResult(eq("\n\nimport py4j\n"))).thenReturn(">>>"); pythonInterpreter.open(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2ee7f48c/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 new file mode 100644 index 0000000..c6b5a51 --- /dev/null +++ b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java @@ -0,0 +1,61 @@ +/* +* Licensed to the Apache Software Foundation (ASF) under one or more +* contributor license agreements. See the NOTICE file distributed with +* this work for additional information regarding copyright ownership. +* The ASF licenses this file to You under the Apache License, Version 2.0 +* (the "License"); you may not use this file except in compliance with +* the License. You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +package org.apache.zeppelin.python; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import org.apache.zeppelin.interpreter.InterpreterResult; +import org.junit.Test; + +/** + * Python interpreter unit test that user real Python + * + * Important: ALL tests here REQUIRE Python to be installed + * They are excluded from default build, to run them manually do: + * + * <code> + * mvn "-Dtest=org.apache.zeppelin.python.PythonInterpreterWithPythonInstalledTest" test -pl python + * </code> + * + * or + * <code> + * mvn -Dpython.test.exclude='' test -pl python + * </code> + */ +public class PythonInterpreterWithPythonInstalledTest { + + @Test + public void badSqlSyntaxFails() { + //given + PythonInterpreter realPython = new PythonInterpreter( + PythonInterpreterTest.getPythonTestProperties()); + realPython.open(); + + //when + InterpreterResult ret = realPython.interpret("select wrong syntax", null); + + //then + assertNotNull("Interpreter returned 'null'", ret); + //System.out.println("\nInterpreter response: \n" + ret.message()); + assertEquals(InterpreterResult.Code.ERROR, ret.code()); + assertTrue(ret.message().length() > 0); + } + +}
