From 4c1107b7611b91b381e7c23e7e1b8a467325bcb5 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Mon, 4 Jul 2016 18:21:43 +0900 Subject: [PATCH 1/4] Refactoring: rename and extract var assignment --- .../java/org/apache/zeppelin/python/PythonInterpreter.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 06e0c99205e..22a327e1ea0 100644 --- a/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java +++ b/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java @@ -54,7 +54,7 @@ public class PythonInterpreter extends Interpreter { private Integer port; private GatewayServer gatewayServer; - private Boolean py4J = false; + private Boolean py4JisInstalled = false; private InterpreterContext context; private int maxResult; @@ -91,7 +91,8 @@ public void open() { 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 { @@ -205,7 +206,7 @@ private void bootStrapInterpreter(String file) throws IOException { 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); From e7d537174816c7fc7cb698d482f394842820446e Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Mon, 4 Jul 2016 21:21:12 +0900 Subject: [PATCH 2/4] Python: add ERROR paragraph status on Error and Exception in output --- python/pom.xml | 26 ++++++++--- .../zeppelin/python/PythonInterpreter.java | 30 ++++++++++--- .../python/PythonInterpreterTest.java | 14 ++++-- ...honInterpreterWithPythonInstalledTest.java | 44 +++++++++++++++++++ 4 files changed, 99 insertions(+), 15 deletions(-) create mode 100644 python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java diff --git a/python/pom.xml b/python/pom.xml index b91142e24e1..c98e35f4740 100644 --- a/python/pom.xml +++ b/python/pom.xml @@ -35,6 +35,7 @@ 0.9.2 + **/PythonInterpreterWithPythonInstalledTest.java @@ -67,7 +68,7 @@ org.slf4j slf4j-log4j12 - + junit junit @@ -89,15 +90,25 @@ maven-enforcer-plugin - 1.3.1 - - - enforce - none + 1.3.1 + + + enforce + none + + org.apache.maven.plugins + maven-surefire-plugin + + + ${python.test.exclude} + + + + maven-dependency-plugin 2.8 @@ -135,11 +146,12 @@ ${project.version} ${project.packaging} - + + 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 22a327e1ea0..c985a545d48 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.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.scheduler.Job; import org.apache.zeppelin.scheduler.Scheduler; import org.apache.zeppelin.scheduler.SchedulerFactory; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -56,6 +57,7 @@ public class PythonInterpreter extends Interpreter { private GatewayServer gatewayServer; private Boolean py4JisInstalled = false; private InterpreterContext context; + private Pattern errorInLastLine = Pattern.compile(".*(Error|Exception): .*$"); private int maxResult; PythonProcess process = null; @@ -124,13 +126,31 @@ public void close() { @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 @@ -196,7 +216,7 @@ private String sendCommandToPython(String cmd) { 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))); 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 92fde2d459c..c9fee13d618 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.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 void testOpenInterpreter() { * 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 void testOpenInterpreter() { * * @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(); 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 00000000000..17665d8be25 --- /dev/null +++ b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java @@ -0,0 +1,44 @@ +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: + * + * + * mvn "-Dtest=org.apache.zeppelin.python.PythonInterpreterWithPythonInstalledTest" test -pl python + * + * + * or + * + * mvn -Dpython.test.exclude='' test -pl python + * + */ +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); + } + +} From b58598200a57c249b68d5872f79ddacc7c50229a Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Mon, 4 Jul 2016 21:16:01 +0900 Subject: [PATCH 3/4] Python: include Python-dependant tests to 1 CI profile --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 19a05cb1d55..26539007f47 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" From a7bf8f3516514c72e37975f4798fa7329750f378 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Mon, 4 Jul 2016 22:35:46 +0900 Subject: [PATCH 4/4] Python: add missing license header --- ...ythonInterpreterWithPythonInstalledTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 17665d8be25..c6b5a51b044 100644 --- a/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java +++ b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterWithPythonInstalledTest.java @@ -1,3 +1,20 @@ +/* +* 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;