From e8ba58a94743b743f2a05cef8b9755d5bf79510c Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Mon, 24 Jul 2017 16:59:29 +0530 Subject: [PATCH 01/18] ZEPPELIN-2807: Passing Z variables to SQL Interpreter (One part of ZEPPELIN-1967) --- .../zeppelin/spark/SparkSqlInterpreter.java | 38 ++++++++++++++++++- .../spark/SparkSqlInterpreterTest.java | 37 ++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java index 134a65f39c7..a528683b86c 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java @@ -22,6 +22,8 @@ import java.util.List; import java.util.Properties; import java.util.concurrent.atomic.AtomicInteger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.spark.SparkContext; import org.apache.spark.sql.SQLContext; @@ -33,6 +35,8 @@ import org.apache.zeppelin.interpreter.LazyOpenInterpreter; import org.apache.zeppelin.interpreter.WrappedInterpreter; import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; +import org.apache.zeppelin.resource.Resource; +import org.apache.zeppelin.resource.ResourcePool; import org.apache.zeppelin.scheduler.Scheduler; import org.apache.zeppelin.scheduler.SchedulerFactory; import org.slf4j.Logger; @@ -85,6 +89,37 @@ public boolean concurrentSQL() { @Override public void close() {} + String interpolateVariable(String originalCommand) { + + Pattern variablePattern = Pattern.compile("([^{]*)([{][^}]+[}]).*"); + Matcher variableMatcher; + + StringBuilder newCommandBuffer = new StringBuilder(); + + SparkZeppelinContext sparkZeppelinContext = getSparkInterpreter().getZeppelinContext(); + + String residualCommand = originalCommand; + // iterate while there is any pattern left in the residual-command text + while ((variableMatcher = variablePattern.matcher(residualCommand)).matches()) { + // get any characters until the next pattern, add to new-command buffer + newCommandBuffer.append(variableMatcher.group(1)); + // get the variable pattern + String variableSpec = variableMatcher.group(2); + // extract variable-name from pattern + String variableName = variableSpec.substring(1, variableSpec.length() - 1); + // get variable-value from interpreter's context + Object variableValue = sparkZeppelinContext.get(variableName); + // add substitution-text into new-command buffer + newCommandBuffer.append(variableValue == null ? variableSpec : variableValue.toString()); + // update residual-command text + residualCommand = residualCommand.substring(variableMatcher.end(2)); + } + // add any remaining text without patterns to the new-command buffer + newCommandBuffer.append(residualCommand); + + return newCommandBuffer.toString(); + } + @Override public InterpreterResult interpret(String st, InterpreterContext context) { SQLContext sqlc = null; @@ -114,7 +149,8 @@ public InterpreterResult interpret(String st, InterpreterContext context) { // to def sql(sqlText: String): DataFrame (1.3 and later). // Therefore need to use reflection to keep binary compatibility for all spark versions. Method sqlMethod = sqlc.getClass().getMethod("sql", String.class); - rdd = sqlMethod.invoke(sqlc, st); + String interpolatedSt = interpolateVariable(st); + rdd = sqlMethod.invoke(sqlc, interpolatedSt); } catch (InvocationTargetException ite) { if (Boolean.parseBoolean(getProperty("zeppelin.spark.sql.stacktrace"))) { throw new InterpreterException(ite); diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java index ebb5e9a9117..66f8071befe 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java @@ -23,6 +23,7 @@ import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.resource.LocalResourcePool; +import org.apache.zeppelin.resource.ResourcePool; import org.apache.zeppelin.user.AuthenticationInfo; import org.apache.zeppelin.display.GUI; import org.apache.zeppelin.interpreter.*; @@ -31,6 +32,7 @@ import org.junit.rules.TemporaryFolder; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; public class SparkSqlInterpreterTest { @@ -161,6 +163,41 @@ public void test_null_value_in_row() { assertEquals("name\tage\ngates\tnull\n", ret.message().get(0).getData()); } + private SparkInterpreter getSparkInterpreter() { + LazyOpenInterpreter lazy = null; + SparkInterpreter spark = null; + Interpreter p = sql.getInterpreterInTheSameSessionByClassName(SparkInterpreter.class.getName()); + + while (p instanceof WrappedInterpreter) { + if (p instanceof LazyOpenInterpreter) { + lazy = (LazyOpenInterpreter) p; + } + p = ((WrappedInterpreter) p).getInnerInterpreter(); + } + spark = (SparkInterpreter) p; + + if (lazy != null) { + lazy.open(); + } + return spark; + } + + @Test + public void testVariableInterpolation() { + context = new InterpreterContext("NoteId", "ParaId", null, + "testVariableInterpolation", "", + null, null, null, null, + new LocalResourcePool("testVariableInterpolation"), + null, null); + SparkZeppelinContext zc = getSparkInterpreter().getZeppelinContext(); + zc.setInterpreterContext(context); + zc.put("n", "100"); + zc.put("table", "name"); + String commandWithVariables = "select * from {table} where count = {n}"; + String resultString = sql.interpolateVariable(commandWithVariables); + assertEquals("select * from name where count = 100", resultString); + } + @Test public void testMaxResults() { repl.interpret("case class P(age:Int)", context); From 565475d051db7bc30504233a0ca9293c413ca852 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Mon, 24 Jul 2017 20:36:07 +0530 Subject: [PATCH 02/18] Changed dist to precise in .travis.yml --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a883de10c0a..6ecd722c088 100644 --- a/.travis.yml +++ b/.travis.yml @@ -50,7 +50,7 @@ matrix: # also, can't use JDK 7 in trusty: https://github.com/travis-ci/travis-ci/issues/7884 - os: linux sudo: false - dist: trusty + dist: precise jdk: "oraclejdk8" env: WEB_E2E="true" SCALA_VER="2.11" SPARK_VER="2.1.0" HADOOP_VER="2.6" PROFILE="-Pweb-ci -Pscala-2.11" BUILD_FLAG="package -DskipTests -DskipRat" TEST_FLAG="verify -DskipRat" MODULES="-pl ${INTERPRETERS}" TEST_MODULES="-pl zeppelin-web" TEST_PROJECTS="-Pweb-e2e" addons: From 2b3292a2ed03a50b1ca72400c3c1db277b52c7bf Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Mon, 24 Jul 2017 20:45:36 +0530 Subject: [PATCH 03/18] Changed dist to precise in .travis.yml - 2nd attempt --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 6ecd722c088..324517247d1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,6 +17,8 @@ language: java sudo: false +dist: precise + cache: apt: true directories: @@ -50,7 +52,7 @@ matrix: # also, can't use JDK 7 in trusty: https://github.com/travis-ci/travis-ci/issues/7884 - os: linux sudo: false - dist: precise + dist: trusty jdk: "oraclejdk8" env: WEB_E2E="true" SCALA_VER="2.11" SPARK_VER="2.1.0" HADOOP_VER="2.6" PROFILE="-Pweb-ci -Pscala-2.11" BUILD_FLAG="package -DskipTests -DskipRat" TEST_FLAG="verify -DskipRat" MODULES="-pl ${INTERPRETERS}" TEST_MODULES="-pl zeppelin-web" TEST_PROJECTS="-Pweb-e2e" addons: From 149eafa14f2831bfc656b126cb4ea55fbbf55cce Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Tue, 25 Jul 2017 09:38:58 +0530 Subject: [PATCH 04/18] Corrected regression caused by newly added unit test --- .../org/apache/zeppelin/spark/SparkSqlInterpreterTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java index 66f8071befe..966700331da 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java @@ -184,13 +184,13 @@ private SparkInterpreter getSparkInterpreter() { @Test public void testVariableInterpolation() { - context = new InterpreterContext("NoteId", "ParaId", null, + InterpreterContext newContext = new InterpreterContext("NoteId", "ParaId", null, "testVariableInterpolation", "", null, null, null, null, new LocalResourcePool("testVariableInterpolation"), null, null); SparkZeppelinContext zc = getSparkInterpreter().getZeppelinContext(); - zc.setInterpreterContext(context); + zc.setInterpreterContext(newContext); zc.put("n", "100"); zc.put("table", "name"); String commandWithVariables = "select * from {table} where count = {n}"; From 70fbbc1b5e07ac623dfa1ba873074082d6670562 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Sat, 29 Jul 2017 09:53:56 +0530 Subject: [PATCH 05/18] Added {{ for escaping single {, and more unit tests for all cases --- .../zeppelin/spark/SparkSqlInterpreter.java | 62 +++++++++++++------ .../spark/SparkSqlInterpreterTest.java | 33 ++++++++++ 2 files changed, 76 insertions(+), 19 deletions(-) diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java index a528683b86c..0763a32e6bd 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java @@ -91,30 +91,51 @@ public void close() {} String interpolateVariable(String originalCommand) { - Pattern variablePattern = Pattern.compile("([^{]*)([{][^}]+[}]).*"); - Matcher variableMatcher; + // Pattern allows errors in number of '{' and '}' characters + Pattern pattern = Pattern.compile("([^{]*)([{]+[^}]+[}]*).*"); + Matcher matcher; StringBuilder newCommandBuffer = new StringBuilder(); SparkZeppelinContext sparkZeppelinContext = getSparkInterpreter().getZeppelinContext(); String residualCommand = originalCommand; - // iterate while there is any pattern left in the residual-command text - while ((variableMatcher = variablePattern.matcher(residualCommand)).matches()) { - // get any characters until the next pattern, add to new-command buffer - newCommandBuffer.append(variableMatcher.group(1)); - // get the variable pattern - String variableSpec = variableMatcher.group(2); - // extract variable-name from pattern - String variableName = variableSpec.substring(1, variableSpec.length() - 1); - // get variable-value from interpreter's context - Object variableValue = sparkZeppelinContext.get(variableName); - // add substitution-text into new-command buffer - newCommandBuffer.append(variableValue == null ? variableSpec : variableValue.toString()); - // update residual-command text - residualCommand = residualCommand.substring(variableMatcher.end(2)); + // Iterate while there is any pattern left in the residual-command text + while ((matcher = pattern.matcher(residualCommand)).matches()) { + // Get any characters until the next pattern, add to new-command buffer + newCommandBuffer.append(matcher.group(1)); + // Get the variable pattern + String template = matcher.group(2); + if (template.matches("[{][^{}][^}]*[}]")) { + // Handle variable substitution pattern ... + // Extract variable-name from template + String variableName = template.substring(1, template.length() - 1); + // Get variable-value from interpreter's context + Object variableValue = sparkZeppelinContext.get(variableName); + if (variableValue == null) { + // No such variable - Error! + return String.format("ERROR:unknown variable '%s'", variableName); + } else { + // Append value to new-command buffer + newCommandBuffer.append(variableValue.toString()); + } + } else if (template.matches("[{]{2}[^{}][^}]*[}]{2}")) { + // Handle '{' escaping pattern ... + // Extract escaped sub-string from template + String escapedSubString = template.substring(1, template.length() - 1); + // Append escaped sub-string to new-command buffer + newCommandBuffer.append(escapedSubString); + } else { + // Unknown pattern - Error! + if (template.endsWith("}")) + return String.format("ERROR:bad pattern '%s'", template); + else + return String.format("ERROR:unpaired '{' in '%s'", template); + } + // Update residual-command text + residualCommand = residualCommand.substring(matcher.end(2)); } - // add any remaining text without patterns to the new-command buffer + // Add any remaining text without patterns to the new-command buffer newCommandBuffer.append(residualCommand); return newCommandBuffer.toString(); @@ -149,8 +170,11 @@ public InterpreterResult interpret(String st, InterpreterContext context) { // to def sql(sqlText: String): DataFrame (1.3 and later). // Therefore need to use reflection to keep binary compatibility for all spark versions. Method sqlMethod = sqlc.getClass().getMethod("sql", String.class); - String interpolatedSt = interpolateVariable(st); - rdd = sqlMethod.invoke(sqlc, interpolatedSt); + String interpolateResult = interpolateVariable(st); + if (interpolateResult.startsWith("ERROR:")) + return new InterpreterResult(Code.ERROR, interpolateResult.substring("ERROR:".length())); + else + rdd = sqlMethod.invoke(sqlc, interpolateResult); } catch (InvocationTargetException ite) { if (Boolean.parseBoolean(getProperty("zeppelin.spark.sql.stacktrace"))) { throw new InterpreterException(ite); diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java index 966700331da..1bb673c960e 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java @@ -191,11 +191,44 @@ public void testVariableInterpolation() { null, null); SparkZeppelinContext zc = getSparkInterpreter().getZeppelinContext(); zc.setInterpreterContext(newContext); + zc.put("n", "100"); zc.put("table", "name"); + + // Correct variable substitutions ... String commandWithVariables = "select * from {table} where count = {n}"; String resultString = sql.interpolateVariable(commandWithVariables); assertEquals("select * from name where count = 100", resultString); + + // Correct escaping of { and } characters ... + commandWithVariables = "select * from names where name rlike 'a{{2}}'"; + resultString = sql.interpolateVariable(commandWithVariables); + assertEquals("select * from names where name rlike 'a{2}'", resultString); + + // Error due to unknown variable ... + commandWithVariables = "select * from names where count = {n2}"; + resultString = sql.interpolateVariable(commandWithVariables); + assertEquals("ERROR:unknown variable 'n2'", resultString); + + // Error due to bad pattern (1) ... + commandWithVariables = "select * from names where name rlike 'a{2}}'"; + resultString = sql.interpolateVariable(commandWithVariables); + assertEquals("ERROR:bad pattern '{2}}'", resultString); + + // Error due to bad pattern (2) ... + commandWithVariables = "select * from names where name rlike 'a{{2}'"; + resultString = sql.interpolateVariable(commandWithVariables); + assertEquals("ERROR:bad pattern '{{2}'", resultString); + + // Error due to bad pattern (3) ... + commandWithVariables = "select * from names where name rlike 'a{{{2}}}'"; + resultString = sql.interpolateVariable(commandWithVariables); + assertEquals("ERROR:bad pattern '{{{2}}}'", resultString); + + // Error due to unpaired { ... + commandWithVariables = "select * from names where name = {alpha"; + resultString = sql.interpolateVariable(commandWithVariables); + assertEquals("ERROR:unpaired '{' in '{alpha'", resultString); } @Test From 901098a72560676888537398eb6d4412ca0b6e1f Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Sat, 29 Jul 2017 10:41:53 +0530 Subject: [PATCH 06/18] Aligned travis.yml with main apache/zeppelin repo --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0973ea54bd3..099fb385d60 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,8 +17,6 @@ language: java sudo: false -dist: precise - cache: apt: true directories: From 3ca4ba1c2c16cebd884c2bc656932058b585886b Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Fri, 11 Aug 2017 13:04:03 +0530 Subject: [PATCH 07/18] Aligned travis.yml with main apache/zeppelin repo --- docs/interpreter/spark.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/interpreter/spark.md b/docs/interpreter/spark.md index 122c8db3b84..422cc6cafb2 100644 --- a/docs/interpreter/spark.md +++ b/docs/interpreter/spark.md @@ -343,6 +343,38 @@ myScalaDataFrame = DataFrame(z.get("myScalaDataFrame"), sqlContext) +In addition to the programmatic access using `z.get()` shown above, +some interpreters also support inline interpolation of such objects +into command lines by using substitution patterns of the form `{object-key}`. +The following example shows how the value of an object `put` from Scala is included within a SQL command. + +
+
+ +{% highlight scala %} +// Put object from scala +%spark +val tableName = ... +z.put("tableName", tableName) + +
+
+ +{% highlight sql %} +# Inline interpolate into SQL command +%spark.sql +select * from {tableName} + +
+
+ +The escaping pattern `{{anything}}` may be used in situations where `{` and `}` are needed +in the command line. The pattern `{{anything}}` is translated into `{anything}` and spliced +into the containing command. + +The use of the `{object-key}` format for object interpolation is currently only available +in the SQL interpreter, but will be provided in other interpreters progressively. + ### Form Creation `ZeppelinContext` provides functions for creating forms. From 861360343e8435b3cd9c3e7ec10a0ac382706f55 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Fri, 11 Aug 2017 15:01:28 +0530 Subject: [PATCH 08/18] Fixing errors in code-panel display --- docs/interpreter/spark.md | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/docs/interpreter/spark.md b/docs/interpreter/spark.md index 422cc6cafb2..a43f2301191 100644 --- a/docs/interpreter/spark.md +++ b/docs/interpreter/spark.md @@ -348,25 +348,6 @@ some interpreters also support inline interpolation of such objects into command lines by using substitution patterns of the form `{object-key}`. The following example shows how the value of an object `put` from Scala is included within a SQL command. -
-
- -{% highlight scala %} -// Put object from scala -%spark -val tableName = ... -z.put("tableName", tableName) - -
-
- -{% highlight sql %} -# Inline interpolate into SQL command -%spark.sql -select * from {tableName} - -
-
The escaping pattern `{{anything}}` may be used in situations where `{` and `}` are needed in the command line. The pattern `{{anything}}` is translated into `{anything}` and spliced From 9b7673b7048992cbd07ccaad300abd16b4021521 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Fri, 11 Aug 2017 19:21:12 +0530 Subject: [PATCH 09/18] Retrying with restored content --- docs/interpreter/spark.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/interpreter/spark.md b/docs/interpreter/spark.md index a43f2301191..422cc6cafb2 100644 --- a/docs/interpreter/spark.md +++ b/docs/interpreter/spark.md @@ -348,6 +348,25 @@ some interpreters also support inline interpolation of such objects into command lines by using substitution patterns of the form `{object-key}`. The following example shows how the value of an object `put` from Scala is included within a SQL command. +
+
+ +{% highlight scala %} +// Put object from scala +%spark +val tableName = ... +z.put("tableName", tableName) + +
+
+ +{% highlight sql %} +# Inline interpolate into SQL command +%spark.sql +select * from {tableName} + +
+
The escaping pattern `{{anything}}` may be used in situations where `{` and `}` are needed in the command line. The pattern `{{anything}}` is translated into `{anything}` and spliced From d6cbd7205d6721ccff142b82d2e07626c90f8b94 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Sat, 12 Aug 2017 19:53:18 +0530 Subject: [PATCH 10/18] Changes to comply with review comments (https://github.com/apache/zeppelin/pull/2502/files/9b7673b7048992cbd07ccaad300abd16b4021521) --- docs/interpreter/spark.md | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/docs/interpreter/spark.md b/docs/interpreter/spark.md index 422cc6cafb2..c4f3d6ff7fe 100644 --- a/docs/interpreter/spark.md +++ b/docs/interpreter/spark.md @@ -343,31 +343,22 @@ myScalaDataFrame = DataFrame(z.get("myScalaDataFrame"), sqlContext) +#### Alternative Object Retrieval Mechanism In addition to the programmatic access using `z.get()` shown above, some interpreters also support inline interpolation of such objects into command lines by using substitution patterns of the form `{object-key}`. -The following example shows how the value of an object `put` from Scala is included within a SQL command. +The following example shows how the value of an object `put` into `ZeppelinContext` +by the Scala interpreter is spliced into a command of the SQL interpreter. -
-
- -{% highlight scala %} -// Put object from scala -%spark +__In Scala interpreter:__ +``` val tableName = ... z.put("tableName", tableName) - -
-
- -{% highlight sql %} -# Inline interpolate into SQL command -%spark.sql +``` +__In SQL interpreter:__ +``` select * from {tableName} - -
-
- +``` The escaping pattern `{{anything}}` may be used in situations where `{` and `}` are needed in the command line. The pattern `{{anything}}` is translated into `{anything}` and spliced into the containing command. From 04acdd854436d5c4a505330b73e444c34e96ab9c Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Sat, 12 Aug 2017 20:38:53 +0530 Subject: [PATCH 11/18] Switched E2E tests to precise from trusty --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 099fb385d60..96bf3039680 100644 --- a/.travis.yml +++ b/.travis.yml @@ -51,13 +51,13 @@ matrix: # also, can't use JDK 7 in trusty: https://github.com/travis-ci/travis-ci/issues/7884 - os: linux sudo: false - dist: trusty + dist: precise jdk: "oraclejdk8" env: WEB_E2E="true" SCALA_VER="2.11" SPARK_VER="2.1.0" HADOOP_VER="2.6" PROFILE="-Pweb-ci -Pscala-2.11" BUILD_FLAG="package -DskipTests -DskipRat" TEST_FLAG="verify -DskipRat" MODULES="-pl ${INTERPRETERS}" TEST_MODULES="-pl zeppelin-web" TEST_PROJECTS="-Pweb-e2e" addons: apt: sources: - - r-packages-trusty + - r-packages-precise packages: - google-chrome-stable - r-base-dev From 906b698187bf5d4f358cb1263bdb15bfb0b286f4 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Sat, 12 Aug 2017 20:44:35 +0530 Subject: [PATCH 12/18] Revert "Switched E2E tests to precise from trusty" (commit 04acdd8) --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 96bf3039680..099fb385d60 100644 --- a/.travis.yml +++ b/.travis.yml @@ -51,13 +51,13 @@ matrix: # also, can't use JDK 7 in trusty: https://github.com/travis-ci/travis-ci/issues/7884 - os: linux sudo: false - dist: precise + dist: trusty jdk: "oraclejdk8" env: WEB_E2E="true" SCALA_VER="2.11" SPARK_VER="2.1.0" HADOOP_VER="2.6" PROFILE="-Pweb-ci -Pscala-2.11" BUILD_FLAG="package -DskipTests -DskipRat" TEST_FLAG="verify -DskipRat" MODULES="-pl ${INTERPRETERS}" TEST_MODULES="-pl zeppelin-web" TEST_PROJECTS="-Pweb-e2e" addons: apt: sources: - - r-packages-precise + - r-packages-trusty packages: - google-chrome-stable - r-base-dev From d70bf3eddae3fdd52bcdf888cdb3442b462e40ad Mon Sep 17 00:00:00 2001 From: Shu Jiaming Date: Wed, 9 Aug 2017 15:47:36 +0800 Subject: [PATCH 13/18] [ZEPPELIN-2841] fix a problem in shell interpreter . Working directory '.' can not be found in docker environment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What is this PR for? shell interpreter complained that working directory '.' can not be found in docker environment. I add a line of code to set current working directory to USER`s home, and it works. ### What type of PR is it? Bug Fix ### Todos * tests ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-2841 ### How should this be tested? run shell interpreter`s test units ### 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: Shu Jiaming Author: 束佳明 Closes #2521 from vistep/master and squashes the following commits: 34a0049 [Shu Jiaming] ZEPPELIN-2841 fix a bug where shell interpreter complained that working directory '.' can not be found while zeppelin was running in docker enviroment. d02104a [束佳明] Merge pull request #1 from apache/master --- .../main/java/org/apache/zeppelin/shell/ShellInterpreter.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java b/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java index 07eed5f9ef1..daad0b32886 100644 --- a/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java +++ b/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java @@ -20,6 +20,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.File; import java.util.List; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; @@ -98,6 +99,7 @@ public InterpreterResult interpret(String cmd, InterpreterContext contextInterpr contextInterpreter.out, contextInterpreter.out)); executor.setWatchdog(new ExecuteWatchdog(Long.valueOf(getProperty(TIMEOUT_PROPERTY)))); executors.put(contextInterpreter.getParagraphId(), executor); + executor.setWorkingDirectory(new File(System.getProperty("user.home"))); int exitVal = executor.execute(cmdLine); LOGGER.info("Paragraph " + contextInterpreter.getParagraphId() + " return with exit value: " + exitVal); From 0a76163306fc51024821e6b82b2d6020c86b07f1 Mon Sep 17 00:00:00 2001 From: Prabhjyot Singh Date: Thu, 10 Aug 2017 15:24:17 -0700 Subject: [PATCH 14/18] [ZEPPELIN-2823] Notebook saved status is wrong if there was a network disconnect or a flaky network. ### What is this PR for? Notebook content doesn't get saved if there is a flaky network, and at times user's paragraph content also gets lost in this process. ### What type of PR is it? [Bug Fix] ### What is the Jira issue? * [ZEPPELIN-2823](https://issues.apache.org/jira/browse/ZEPPELIN-2823) ### How should this be tested? Steps to re-produce: - create a new notebook - in the first paragraph enter text, say "version1" - now disconnect the network (say by removing LAN cable) - update this paragraph again with text "version2" - reconnect network - now observe the on the WebSocket reconnect, the content of this paragraph will go back to "version1" ### Screenshots (if appropriate) Before ![before](https://user-images.githubusercontent.com/674497/28852738-5772029e-76e0-11e7-82ed-8c2a25d3ab47.gif) After ![after](https://user-images.githubusercontent.com/674497/28852739-5774efcc-76e0-11e7-9e48-4bda935c4686.gif) ### Questions: * Does the licenses files need an update? N/A * Is there breaking changes for older versions? N/A * Does this needs documentation? N/A Author: Prabhjyot Singh Closes #2512 from prabhjyotsingh/ZEPPELIN-2823 and squashes the following commits: 5f693ab93 [Prabhjyot Singh] - replace _.forEach with .map - extract BootstrapDialog.show outside of the for loop db30f479b [Prabhjyot Singh] alter text to `Changes that you have made will not be saved` 947be70b4 [Prabhjyot Singh] check if noteId exists in session or take it from fromMessage 8b8c2f974 [Prabhjyot Singh] check for empty originalText d2a835f77 [Prabhjyot Singh] wait for server confirmation before updating stats of notebook --- .../zeppelin/socket/NotebookServer.java | 3 + .../src/app/notebook/notebook.controller.js | 60 +++++++++++++++++-- .../paragraph/paragraph.controller.js | 36 +++++++++-- .../paragraph/result/result.controller.js | 2 +- .../websocket/websocket-event.factory.js | 2 +- .../websocket/websocket-message.service.js | 5 +- 6 files changed, 95 insertions(+), 13 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java index 61bc536c8c4..3ddeec034e2 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java @@ -1186,6 +1186,9 @@ private void updateParagraph(NotebookSocket conn, HashSet userAndRoles, Map params = (Map) fromMessage.get("params"); Map config = (Map) fromMessage.get("config"); String noteId = getOpenNoteId(conn); + if (noteId == null) { + noteId = (String) fromMessage.get("noteId"); + } if (!hasParagraphWriterPermission(conn, notebook, noteId, userAndRoles, fromMessage.principal, "write")) { diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js index a51ad4fff82..4b8b23fe024 100644 --- a/zeppelin-web/src/app/notebook/notebook.controller.js +++ b/zeppelin-web/src/app/notebook/notebook.controller.js @@ -51,6 +51,7 @@ function NotebookCtrl ($scope, $route, $routeParams, $location, $rootScope, $scope.interpreterBindings = [] $scope.isNoteDirty = null $scope.saveTimer = null + $scope.paragraphWarningDialog = {} let connectedOnce = false let isRevisionPath = function (path) { @@ -396,11 +397,6 @@ function NotebookCtrl ($scope, $route, $routeParams, $location, $rootScope, }, 10000) } - angular.element(window).on('beforeunload', function (e) { - $scope.killSaveTimer() - $scope.saveNote() - }) - $scope.setLookAndFeel = function (looknfeel) { $scope.note.config.looknfeel = looknfeel if ($scope.revisionView === true) { @@ -1277,6 +1273,60 @@ function NotebookCtrl ($scope, $route, $routeParams, $location, $rootScope, $scope.note.config.personalizedMode = isPersonalized }) + $scope.$on('$routeChangeStart', function (event, next, current) { + if (!$scope.note || !$scope.note.paragraphs) { + return + } + if ($scope.note && $scope.note.paragraphs) { + $scope.note.paragraphs.map(par => { + if ($scope.allowLeave === true) { + return + } + let thisScope = angular.element( + '#' + par.id + '_paragraphColumn_main').scope() + + if (thisScope.dirtyText === undefined || + thisScope.originalText === undefined || + thisScope.dirtyText === thisScope.originalText) { + return true + } else { + event.preventDefault() + $scope.showParagraphWarning(next) + } + }) + } + }) + + $scope.showParagraphWarning = function (next) { + if ($scope.paragraphWarningDialog.opened !== true) { + $scope.paragraphWarningDialog = BootstrapDialog.show({ + closable: false, + closeByBackdrop: false, + closeByKeyboard: false, + title: 'Do you want to leave this site?', + message: 'Changes that you have made will not be saved.', + buttons: [{ + label: 'Stay', + action: function (dialog) { + dialog.close() + } + }, { + label: 'Leave', + action: function (dialog) { + dialog.close() + let locationToRedirect = next['$$route']['originalPath'] + Object.keys(next.pathParams).map(key => { + locationToRedirect = locationToRedirect.replace(':' + key, + next.pathParams[key]) + }) + $scope.allowLeave = true + $location.path(locationToRedirect) + } + }] + }) + } + } + $scope.$on('$destroy', function () { angular.element(window).off('beforeunload') $scope.killSaveTimer() diff --git a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js index 141f7b3997e..b4c79dd6dc6 100644 --- a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js +++ b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js @@ -382,14 +382,41 @@ function ParagraphCtrl ($scope, $rootScope, $route, $window, $routeParams, $loca paragraphText, $scope.paragraph.config, $scope.paragraph.settings.params) } + $scope.bindBeforeUnload = function () { + angular.element(window).off('beforeunload') + + let confirmOnPageExit = function (e) { + // If we haven't been passed the event get the window.event + e = e || window.event + let message = 'Do you want to reload this site?' + + // For IE6-8 and Firefox prior to version 4 + if (e) { + e.returnValue = message + } + // For Chrome, Safari, IE8+ and Opera 12+ + return message + } + angular.element(window).on('beforeunload', confirmOnPageExit) + } + + $scope.unBindBeforeUnload = function () { + angular.element(window).off('beforeunload') + } + $scope.saveParagraph = function (paragraph) { const dirtyText = paragraph.text if (dirtyText === undefined || dirtyText === $scope.originalText) { return } - commitParagraph(paragraph) - $scope.originalText = dirtyText - $scope.dirtyText = undefined + + $scope.bindBeforeUnload() + + commitParagraph(paragraph).then(function () { + $scope.originalText = dirtyText + $scope.dirtyText = undefined + $scope.unBindBeforeUnload() + }) } $scope.toggleEnableDisable = function (paragraph) { @@ -1092,7 +1119,8 @@ function ParagraphCtrl ($scope, $rootScope, $route, $window, $routeParams, $loca settings: {params}, } = paragraph - websocketMsgSrv.commitParagraph(id, title, text, config, params) + return websocketMsgSrv.commitParagraph(id, title, text, config, params, + $route.current.pathParams.noteId) } /** Utility function */ diff --git a/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js b/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js index df9ebe96316..be71d9c8492 100644 --- a/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js +++ b/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js @@ -645,7 +645,7 @@ function ResultCtrl ($scope, $rootScope, $route, $window, $routeParams, $locatio }, newParagraphConfig.results[resultIndex], paragraph, resultIndex) renderResult($scope.type, true) } else { - websocketMsgSrv.commitParagraph(paragraph.id, title, text, newParagraphConfig, params) + return websocketMsgSrv.commitParagraph(paragraph.id, title, text, newParagraphConfig, params) } } diff --git a/zeppelin-web/src/components/websocket/websocket-event.factory.js b/zeppelin-web/src/components/websocket/websocket-event.factory.js index db058bbc68b..10cfd9c2190 100644 --- a/zeppelin-web/src/components/websocket/websocket-event.factory.js +++ b/zeppelin-web/src/components/websocket/websocket-event.factory.js @@ -42,7 +42,7 @@ function WebsocketEventFactory ($rootScope, $websocket, $location, baseUrlSrv) { data.roles = '' } console.log('Send >> %o, %o, %o, %o, %o', data.op, data.principal, data.ticket, data.roles, data) - websocketCalls.ws.send(JSON.stringify(data)) + return websocketCalls.ws.send(JSON.stringify(data)) } websocketCalls.isConnected = function () { diff --git a/zeppelin-web/src/components/websocket/websocket-message.service.js b/zeppelin-web/src/components/websocket/websocket-message.service.js index 0dc02c3bfdc..cafc61b1f92 100644 --- a/zeppelin-web/src/components/websocket/websocket-message.service.js +++ b/zeppelin-web/src/components/websocket/websocket-message.service.js @@ -233,11 +233,12 @@ function WebsocketMessageService ($rootScope, websocketEvents) { }) }, - commitParagraph: function (paragraphId, paragraphTitle, paragraphData, paragraphConfig, paragraphParams) { - websocketEvents.sendNewEvent({ + commitParagraph: function (paragraphId, paragraphTitle, paragraphData, paragraphConfig, paragraphParams, noteId) { + return websocketEvents.sendNewEvent({ op: 'COMMIT_PARAGRAPH', data: { id: paragraphId, + noteId: noteId, title: paragraphTitle, paragraph: paragraphData, config: paragraphConfig, From ce7de3dda66bcae7be17c57b26171a7041f49269 Mon Sep 17 00:00:00 2001 From: Prabhjyot Singh Date: Thu, 10 Aug 2017 16:27:28 -0700 Subject: [PATCH 15/18] [ZEPPELIN-2846] Add selenium test case for AnyOfRolesAuthorizationFilter ### What is this PR for? This is to test the new feature that was brought in with ZEPPELIN-2825 (org.apache.zeppelin.utils.AnyOfRolesAuthorizationFilter) ### What type of PR is it? [Improvement] ### What is the Jira issue? * [ZEPPELIN-2846](https://issues.apache.org/jira/browse/ZEPPELIN-2846) ### How should this be tested? CI should be green ### Questions: * Does the licenses files need update? N/A * Is there breaking changes for older versions? N/A * Does this needs documentation? N/A Author: Prabhjyot Singh Closes #2524 from prabhjyotsingh/ZEPPELIN-2846 and squashes the following commits: e2a7ad548 [Prabhjyot Singh] add selenium test case for AnyOfRolesAuthorizationFilter --- .../apache/zeppelin/AbstractZeppelinIT.java | 1 + .../integration/AuthenticationIT.java | 62 ++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/AbstractZeppelinIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/AbstractZeppelinIT.java index 475be50270c..6f537fd80c8 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/AbstractZeppelinIT.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/AbstractZeppelinIT.java @@ -40,6 +40,7 @@ abstract public class AbstractZeppelinIT { protected static WebDriver driver; protected final static Logger LOG = LoggerFactory.getLogger(AbstractZeppelinIT.class); + protected static final long MIN_IMPLICIT_WAIT = 5; protected static final long MAX_IMPLICIT_WAIT = 30; protected static final long MAX_BROWSER_TIMEOUT_SEC = 30; protected static final long MAX_PARAGRAPH_TIMEOUT_SEC = 120; diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java index f87bff2ce5a..38fe5744d9d 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java @@ -23,7 +23,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.List; - import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.AbstractZeppelinIT; @@ -38,6 +37,7 @@ import org.junit.rules.ErrorCollector; import org.openqa.selenium.By; import org.openqa.selenium.Keys; +import org.openqa.selenium.TimeoutException; import org.openqa.selenium.WebElement; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,12 +63,14 @@ public class AuthenticationIT extends AbstractZeppelinIT { "securityManager.sessionManager = $sessionManager\n" + "securityManager.sessionManager.globalSessionTimeout = 86400000\n" + "shiro.loginUrl = /api/login\n" + + "anyofroles = org.apache.zeppelin.utils.AnyOfRolesAuthorizationFilter\n" + "[roles]\n" + "admin = *\n" + "hr = *\n" + "finance = *\n" + "[urls]\n" + "/api/version = anon\n" + + "/api/interpreter/** = authc, anyofroles[admin, finance]\n" + "/** = authc"; static String originalShiro = ""; @@ -182,6 +184,62 @@ public void testSimpleAuthentication() throws Exception { } } + @Test + public void testAnyOfRoles() throws Exception { + if (!endToEndTestEnabled()) { + return; + } + try { + AuthenticationIT authenticationIT = new AuthenticationIT(); + authenticationIT.authenticationUser("admin", "password1"); + + pollingWait(By.xpath("//div/button[contains(@class, 'nav-btn dropdown-toggle ng-scope')]"), + MAX_BROWSER_TIMEOUT_SEC).click(); + clickAndWait(By.xpath("//li/a[contains(@href, '#/interpreter')]")); + + collector.checkThat("Check is user has permission to view this page", true, + CoreMatchers.equalTo(pollingWait(By.xpath( + "//div[@id='main']/div/div[2]"), + MIN_IMPLICIT_WAIT).isDisplayed()) + ); + + authenticationIT.logoutUser("admin"); + + authenticationIT.authenticationUser("finance1", "finance1"); + + pollingWait(By.xpath("//div/button[contains(@class, 'nav-btn dropdown-toggle ng-scope')]"), + MAX_BROWSER_TIMEOUT_SEC).click(); + clickAndWait(By.xpath("//li/a[contains(@href, '#/interpreter')]")); + + collector.checkThat("Check is user has permission to view this page", true, + CoreMatchers.equalTo(pollingWait(By.xpath( + "//div[@id='main']/div/div[2]"), + MIN_IMPLICIT_WAIT).isDisplayed()) + ); + + authenticationIT.logoutUser("finance1"); + + authenticationIT.authenticationUser("hr1", "hr1"); + + pollingWait(By.xpath("//div/button[contains(@class, 'nav-btn dropdown-toggle ng-scope')]"), + MAX_BROWSER_TIMEOUT_SEC).click(); + clickAndWait(By.xpath("//li/a[contains(@href, '#/interpreter')]")); + + try { + collector.checkThat("Check is user has permission to view this page", + true, CoreMatchers.equalTo( + pollingWait(By.xpath("//li[contains(@class, 'ng-toast__message')]//span/span"), + MIN_IMPLICIT_WAIT).isDisplayed())); + } catch (TimeoutException e) { + throw new Exception("Expected ngToast not found", e); + } + authenticationIT.logoutUser("hr1"); + + } catch (Exception e) { + handleException("Exception in AuthenticationIT while testAnyOfRoles ", e); + } + } + @Test public void testGroupPermission() throws Exception { if (!endToEndTestEnabled()) { @@ -254,7 +312,7 @@ public void testGroupPermission() throws Exception { } catch (Exception e) { - handleException("Exception in ParagraphActionsIT while testGroupPermission ", e); + handleException("Exception in AuthenticationIT while testGroupPermission ", e); } } From 2edc4df7befc3b988fba33d25fc3cee982753e09 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Thu, 17 Aug 2017 14:05:08 +0530 Subject: [PATCH 16/18] Embedded newline issue found by Leemoonsoo (https://github.com/apache/zeppelin/pull/2502#issuecomment-322979150) --- .../java/org/apache/zeppelin/spark/SparkSqlInterpreter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java index 0763a32e6bd..20a04578c5f 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java @@ -92,7 +92,7 @@ public void close() {} String interpolateVariable(String originalCommand) { // Pattern allows errors in number of '{' and '}' characters - Pattern pattern = Pattern.compile("([^{]*)([{]+[^}]+[}]*).*"); + Pattern pattern = Pattern.compile("([^{]*)([{]+[^}]+[}]*).*", Pattern.DOTALL); Matcher matcher; StringBuilder newCommandBuffer = new StringBuilder(); From 9e75333ff7ef9c27678382c598b524eafc6191f7 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Sun, 20 Aug 2017 18:10:33 +0530 Subject: [PATCH 17/18] Changes to comply with review https://github.com/apache/zeppelin/pull/2502#pullrequestreview-57362016 by felixcheung --- docs/interpreter/spark.md | 16 ++++- .../zeppelin/spark/SparkSqlInterpreter.java | 63 ++----------------- .../spark/SparkSqlInterpreterTest.java | 57 +++++++++-------- .../zeppelin/interpreter/Interpreter.java | 62 ++++++++++++++++++ 4 files changed, 111 insertions(+), 87 deletions(-) diff --git a/docs/interpreter/spark.md b/docs/interpreter/spark.md index c4f3d6ff7fe..eba9d563757 100644 --- a/docs/interpreter/spark.md +++ b/docs/interpreter/spark.md @@ -343,7 +343,7 @@ myScalaDataFrame = DataFrame(z.get("myScalaDataFrame"), sqlContext) -#### Alternative Object Retrieval Mechanism +### Object Interpolation into Command In addition to the programmatic access using `z.get()` shown above, some interpreters also support inline interpolation of such objects into command lines by using substitution patterns of the form `{object-key}`. @@ -361,7 +361,19 @@ select * from {tableName} ``` The escaping pattern `{{anything}}` may be used in situations where `{` and `}` are needed in the command line. The pattern `{{anything}}` is translated into `{anything}` and spliced -into the containing command. +into the containing command. If the key `anything` is not defined, the entire +pattern (from first '{' to last '}') is passed unchanged. + +This feature is inspired by a similar facility in Jupyter (when spawning a command using the +bang `!` operator). As with Jupyter the interpolation described above is applied only when the command line +contains some well-formed patterns consisting of a prefix containing 1 or more '{' characters, +some plain text (without { and } characters), and a suffix containing 1 or more '}' characters. +The number of '{' characters must match the number of '}' characters for the patterns to be +considered well-formed. + +Each pair of '{{' ... '}}' is translated in to a '{' ... '}'. If the command contains '{' or '}' +characters used in any other way, no interpolation is performed on the command line, even if +the command contains other well-formed patterns. The use of the `{object-key}` format for object interpolation is currently only available in the SQL interpreter, but will be provided in other interpreters progressively. diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java index 20a04578c5f..c20add64ea5 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkSqlInterpreter.java @@ -35,8 +35,6 @@ import org.apache.zeppelin.interpreter.LazyOpenInterpreter; import org.apache.zeppelin.interpreter.WrappedInterpreter; import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; -import org.apache.zeppelin.resource.Resource; -import org.apache.zeppelin.resource.ResourcePool; import org.apache.zeppelin.scheduler.Scheduler; import org.apache.zeppelin.scheduler.SchedulerFactory; import org.slf4j.Logger; @@ -89,60 +87,11 @@ public boolean concurrentSQL() { @Override public void close() {} - String interpolateVariable(String originalCommand) { - - // Pattern allows errors in number of '{' and '}' characters - Pattern pattern = Pattern.compile("([^{]*)([{]+[^}]+[}]*).*", Pattern.DOTALL); - Matcher matcher; - - StringBuilder newCommandBuffer = new StringBuilder(); - - SparkZeppelinContext sparkZeppelinContext = getSparkInterpreter().getZeppelinContext(); - - String residualCommand = originalCommand; - // Iterate while there is any pattern left in the residual-command text - while ((matcher = pattern.matcher(residualCommand)).matches()) { - // Get any characters until the next pattern, add to new-command buffer - newCommandBuffer.append(matcher.group(1)); - // Get the variable pattern - String template = matcher.group(2); - if (template.matches("[{][^{}][^}]*[}]")) { - // Handle variable substitution pattern ... - // Extract variable-name from template - String variableName = template.substring(1, template.length() - 1); - // Get variable-value from interpreter's context - Object variableValue = sparkZeppelinContext.get(variableName); - if (variableValue == null) { - // No such variable - Error! - return String.format("ERROR:unknown variable '%s'", variableName); - } else { - // Append value to new-command buffer - newCommandBuffer.append(variableValue.toString()); - } - } else if (template.matches("[{]{2}[^{}][^}]*[}]{2}")) { - // Handle '{' escaping pattern ... - // Extract escaped sub-string from template - String escapedSubString = template.substring(1, template.length() - 1); - // Append escaped sub-string to new-command buffer - newCommandBuffer.append(escapedSubString); - } else { - // Unknown pattern - Error! - if (template.endsWith("}")) - return String.format("ERROR:bad pattern '%s'", template); - else - return String.format("ERROR:unpaired '{' in '%s'", template); - } - // Update residual-command text - residualCommand = residualCommand.substring(matcher.end(2)); - } - // Add any remaining text without patterns to the new-command buffer - newCommandBuffer.append(residualCommand); - - return newCommandBuffer.toString(); - } - @Override public InterpreterResult interpret(String st, InterpreterContext context) { + SparkZeppelinContext sparkZeppelinContext = getSparkInterpreter().getZeppelinContext(); + + String interpolatedCommand = interpolateZeppelinContextObjects(st, sparkZeppelinContext); SQLContext sqlc = null; SparkInterpreter sparkInterpreter = getSparkInterpreter(); @@ -170,11 +119,7 @@ public InterpreterResult interpret(String st, InterpreterContext context) { // to def sql(sqlText: String): DataFrame (1.3 and later). // Therefore need to use reflection to keep binary compatibility for all spark versions. Method sqlMethod = sqlc.getClass().getMethod("sql", String.class); - String interpolateResult = interpolateVariable(st); - if (interpolateResult.startsWith("ERROR:")) - return new InterpreterResult(Code.ERROR, interpolateResult.substring("ERROR:".length())); - else - rdd = sqlMethod.invoke(sqlc, interpolateResult); + rdd = sqlMethod.invoke(sqlc, interpolatedCommand); } catch (InvocationTargetException ite) { if (Boolean.parseBoolean(getProperty("zeppelin.spark.sql.stacktrace"))) { throw new InterpreterException(ite); diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java index 1bb673c960e..b7b1b9745f9 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkSqlInterpreterTest.java @@ -23,7 +23,6 @@ import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.resource.LocalResourcePool; -import org.apache.zeppelin.resource.ResourcePool; import org.apache.zeppelin.user.AuthenticationInfo; import org.apache.zeppelin.display.GUI; import org.apache.zeppelin.interpreter.*; @@ -197,38 +196,44 @@ public void testVariableInterpolation() { // Correct variable substitutions ... String commandWithVariables = "select * from {table} where count = {n}"; - String resultString = sql.interpolateVariable(commandWithVariables); + String resultString = sql.interpolateZeppelinContextObjects(commandWithVariables, zc); assertEquals("select * from name where count = 100", resultString); - // Correct escaping of { and } characters ... + // Correct escaping of { and } characters (1) ... commandWithVariables = "select * from names where name rlike 'a{{2}}'"; - resultString = sql.interpolateVariable(commandWithVariables); + resultString = sql.interpolateZeppelinContextObjects(commandWithVariables, zc); assertEquals("select * from names where name rlike 'a{2}'", resultString); + // Correct escaping of { and } characters (2) ... + commandWithVariables = "this: {table}, {{{{{{any}}}}}} is not SQL"; + resultString = sql.interpolateZeppelinContextObjects(commandWithVariables, zc); + assertEquals("this: name, {{{any}}} is not SQL", resultString); + // Error due to unknown variable ... commandWithVariables = "select * from names where count = {n2}"; - resultString = sql.interpolateVariable(commandWithVariables); - assertEquals("ERROR:unknown variable 'n2'", resultString); - - // Error due to bad pattern (1) ... - commandWithVariables = "select * from names where name rlike 'a{2}}'"; - resultString = sql.interpolateVariable(commandWithVariables); - assertEquals("ERROR:bad pattern '{2}}'", resultString); - - // Error due to bad pattern (2) ... - commandWithVariables = "select * from names where name rlike 'a{{2}'"; - resultString = sql.interpolateVariable(commandWithVariables); - assertEquals("ERROR:bad pattern '{{2}'", resultString); - - // Error due to bad pattern (3) ... - commandWithVariables = "select * from names where name rlike 'a{{{2}}}'"; - resultString = sql.interpolateVariable(commandWithVariables); - assertEquals("ERROR:bad pattern '{{{2}}}'", resultString); - - // Error due to unpaired { ... - commandWithVariables = "select * from names where name = {alpha"; - resultString = sql.interpolateVariable(commandWithVariables); - assertEquals("ERROR:unpaired '{' in '{alpha'", resultString); + resultString = sql.interpolateZeppelinContextObjects(commandWithVariables, zc); + assertEquals("select * from names where count = {n2}", resultString); + + // Incorrect pairing of { and } characters (1) ... + commandWithVariables = "these: {{2}}, {{{{{{any}}}}} are not SQL"; + resultString = sql.interpolateZeppelinContextObjects(commandWithVariables, zc); + assertEquals("these: {{2}}, {{{{{{any}}}}} are not SQL", resultString); + + // Incorrect pairing of { and } characters (2) ... + commandWithVariables = "select * from {table} where count = {n}}"; + resultString = sql.interpolateZeppelinContextObjects(commandWithVariables, zc); + assertEquals("select * from {table} where count = {n}}", resultString); + + // Incorrect pairing of { and } characters (3) ... + commandWithVariables = "select * from {table} where count = {n}}"; + resultString = sql.interpolateZeppelinContextObjects(commandWithVariables, zc); + assertEquals("select * from {table} where count = {n}}", resultString); + + // Combined substitution and escaping ... + commandWithVariables = "select * from {{{table}}} where count = {n}"; + resultString = sql.interpolateZeppelinContextObjects(commandWithVariables, zc); + assertEquals("select * from {name} where count = 100", resultString); + } @Test diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java index 74506dd402d..f71aa3139be 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java @@ -26,6 +26,8 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.reflect.FieldUtils; @@ -487,4 +489,64 @@ public static RegisteredInterpreter findRegisteredInterpreterByClassName(String } return null; } + + public String interpolateZeppelinContextObjects(String originalCommand, BaseZeppelinContext sparkZeppelinContext) { + + // Pattern allows mismatch in number of '{' and '}' characters + Pattern pattern = Pattern.compile("([^}{]*)(([{]+)([^}]*)([}]+)).*", Pattern.DOTALL); + Matcher matcher; + + StringBuilder newCommandBuffer = new StringBuilder(); + + String residualCommand = originalCommand; + Boolean allBracePairsBalanced = true; + // Iterate while the residual text matches the pattern + while (!residualCommand.isEmpty() && (matcher = pattern.matcher(residualCommand)).matches()) { + // Get characters prefixing the next pattern, add to new-command buffer + newCommandBuffer.append(matcher.group(1)); + // Handle variable substitution pattern ... + // Get parts of the variable pattern + int openingBraceCount = matcher.group(3).length(); + int closingBraceCount = matcher.group(5).length(); + if (openingBraceCount != closingBraceCount) { + allBracePairsBalanced = false; + // No need to perform any substitutions if any braces are mismatched + break; + } + + String substitutedText = null; + String objectName = matcher.group(4); + // Substitution performed only for odd number of braces + if (openingBraceCount % 2 == 1) { + Object variableValue = sparkZeppelinContext.get(objectName); + if (variableValue == null) + // No corresponding value, use pattern itself ... + substitutedText = String.format("{%s}", objectName); + else + // Substitute corresponding value if found ... + substitutedText = variableValue.toString(); + } else + // No substitution is to be performed ... + substitutedText = objectName; + + // Escape each '{{' ... '}}' pair into '{' ... '}' + while (openingBraceCount >= 2) { + substitutedText = String.format("{%s}", substitutedText); + openingBraceCount -= 2; + } + + newCommandBuffer.append(substitutedText); + + // Update residual-command text + residualCommand = residualCommand.substring(matcher.end(2)); + } + // Add any remaining non-matching characters + newCommandBuffer.append(residualCommand); + + if (allBracePairsBalanced && residualCommand.matches("[^}{]*")) + return newCommandBuffer.toString(); + else + return originalCommand; + } + } From f1bed2aecb18c0a29a95c88543dbcae9a41ddd79 Mon Sep 17 00:00:00 2001 From: Sanjay Dasgupta Date: Sun, 20 Aug 2017 18:20:28 +0530 Subject: [PATCH 18/18] Reduced line length to allow travis-ci tests to pass --- .../java/org/apache/zeppelin/interpreter/Interpreter.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java index f71aa3139be..dac18f8751c 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java @@ -490,7 +490,8 @@ public static RegisteredInterpreter findRegisteredInterpreterByClassName(String return null; } - public String interpolateZeppelinContextObjects(String originalCommand, BaseZeppelinContext sparkZeppelinContext) { + public String interpolateZeppelinContextObjects(String originalCommand, + BaseZeppelinContext sparkZeppelinContext) { // Pattern allows mismatch in number of '{' and '}' characters Pattern pattern = Pattern.compile("([^}{]*)(([{]+)([^}]*)([}]+)).*", Pattern.DOTALL); @@ -501,7 +502,8 @@ public String interpolateZeppelinContextObjects(String originalCommand, BaseZepp String residualCommand = originalCommand; Boolean allBracePairsBalanced = true; // Iterate while the residual text matches the pattern - while (!residualCommand.isEmpty() && (matcher = pattern.matcher(residualCommand)).matches()) { + while (!residualCommand.isEmpty() && + (matcher = pattern.matcher(residualCommand)).matches()) { // Get characters prefixing the next pattern, add to new-command buffer newCommandBuffer.append(matcher.group(1)); // Handle variable substitution pattern ...