From 0ae83f4d87ffd6ad078164a5ae6b5326441572f0 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Thu, 16 Apr 2015 12:36:21 +0900 Subject: [PATCH 1/9] ZEPPELIN-46: set only non-empty properties for spark.* --- .../org/apache/zeppelin/spark/SparkInterpreter.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java index b038dd6dbad..c858691ba16 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java @@ -259,17 +259,20 @@ public SparkContext createSparkContext() { for (Object k : intpProperty.keySet()) { String key = (String) k; - Object value = intpProperty.get(key); - logger.debug(String.format("SparkConf: key = [%s], value = [%s]", key, value)); - conf.set(key, (String) value); + String val = toString(intpProperty.get(key)); + + if (!val.startsWith("spark.") || !val.trim().isEmpty()) { + logger.debug(String.format("SparkConf: key = [%s], value = [%s]", key, val)); + conf.set(key, val); + } } SparkContext sparkContext = new SparkContext(conf); return sparkContext; } - public static boolean isEmptyString(Object val) { - return val instanceof String && ((String) val).trim().isEmpty(); + static final String toString(Object o) { + return (o instanceof String) ? (String) o : ""; } public static String getSystemDefault( From 00e467623ddea702b3d532e019daf460c130a969 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Thu, 16 Apr 2015 14:31:10 +0900 Subject: [PATCH 2/9] ZEPPELIN-46 fixing a typo --- .../main/java/org/apache/zeppelin/spark/SparkInterpreter.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java index c858691ba16..1b41d7c275e 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java @@ -260,8 +260,7 @@ public SparkContext createSparkContext() { for (Object k : intpProperty.keySet()) { String key = (String) k; String val = toString(intpProperty.get(key)); - - if (!val.startsWith("spark.") || !val.trim().isEmpty()) { + if (!key.startsWith("spark.") || !val.trim().isEmpty()) { logger.debug(String.format("SparkConf: key = [%s], value = [%s]", key, val)); conf.set(key, val); } From 3ff8460d7e792ef2783bb2452de1e78b6e6e9000 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Thu, 16 Apr 2015 17:31:11 +0900 Subject: [PATCH 3/9] ZEPPELIN-46 adding tests, by @jongyoul --- .../apache/zeppelin/spark/SparkInterpreterTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java index a5e0fe22760..6034795f0ad 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java @@ -18,6 +18,7 @@ package org.apache.zeppelin.spark; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.io.File; @@ -38,6 +39,8 @@ import org.junit.Test; import org.junit.runners.MethodSorters; +import scala.Tuple2; + @FixMethodOrder(MethodSorters.NAME_ASCENDING) public class SparkInterpreterTest { public static SparkInterpreter repl; @@ -138,4 +141,13 @@ public void testZContextDependencyLoading() { repl.interpret("z.load(\"org.apache.commons:commons-csv:1.1\")", context); assertEquals(InterpreterResult.Code.SUCCESS, repl.interpret("import org.apache.commons.csv.CSVFormat", context).code()); } + + @Test + public void emptyConfigurationVariablesOnlyForNonSparkProperties() { + for (Tuple2 tuple2 : repl.getSparkContext().getConf().getAll()) { + if (tuple2._1().startsWith("spark.")) + assertFalse(String.format("configuration starting from 'spark.' should not be empty. [%s]: [%s]", tuple2._1(), tuple2._2()), tuple2._2().isEmpty()); + } + } + } From 9d94910b058fd2539921d6c1e54ed8d2171ee9f3 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Fri, 17 Apr 2015 10:21:42 +0900 Subject: [PATCH 4/9] ZEPPELIN-46: make tests pass on local Spark --- .../java/org/apache/zeppelin/spark/SparkInterpreterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java index 6034795f0ad..a7fb3e2fb3e 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java @@ -145,7 +145,7 @@ public void testZContextDependencyLoading() { @Test public void emptyConfigurationVariablesOnlyForNonSparkProperties() { for (Tuple2 tuple2 : repl.getSparkContext().getConf().getAll()) { - if (tuple2._1().startsWith("spark.")) + if (tuple2._1().startsWith("spark.") && !tuple2._1().equals("spark.jars")) //is empty in local Spark assertFalse(String.format("configuration starting from 'spark.' should not be empty. [%s]: [%s]", tuple2._1(), tuple2._2()), tuple2._2().isEmpty()); } } From 5e22509f63c967a97f6dafc807caa0e26f7feab1 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Fri, 17 Apr 2015 11:36:51 +0900 Subject: [PATCH 5/9] Resolve conflicts --- .../apache/zeppelin/spark/SparkInterpreter.java | 4 +++- .../zeppelin/spark/SparkInterpreterTest.java | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java index 1b41d7c275e..53edca84108 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java @@ -244,9 +244,11 @@ public SparkContext createSparkContext() { new SparkConf() .setMaster(getProperty("master")) .setAppName(getProperty("spark.app.name")) - .setJars(jars) .set("spark.repl.class.uri", classServerUri); + if(0 < jars.length) + conf.setJars(jars); + if (execUri != null) { conf.set("spark.executor.uri", execUri); } diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java index a7fb3e2fb3e..965211510ab 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java @@ -26,6 +26,7 @@ import java.util.LinkedList; import java.util.Properties; +import org.apache.spark.SparkConf; import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.display.GUI; import org.apache.zeppelin.interpreter.InterpreterContext; @@ -144,10 +145,16 @@ public void testZContextDependencyLoading() { @Test public void emptyConfigurationVariablesOnlyForNonSparkProperties() { - for (Tuple2 tuple2 : repl.getSparkContext().getConf().getAll()) { - if (tuple2._1().startsWith("spark.") && !tuple2._1().equals("spark.jars")) //is empty in local Spark - assertFalse(String.format("configuration starting from 'spark.' should not be empty. [%s]: [%s]", tuple2._1(), tuple2._2()), tuple2._2().isEmpty()); + Properties intpProperty = repl.getProperty(); + SparkConf sparkConf = repl.getSparkContext().getConf(); + for (Object oKey : intpProperty.keySet()) { + String key = (String) oKey; + String value = (String)intpProperty.get(key); + repl.logger.debug(String.format("[%s]: [%s]", key, value)); + if (key.startsWith("spark.") && value.isEmpty()) { + if (value.isEmpty()) + assertTrue(String.format("configuration starting from 'spark.' should not be empty. [%s]", key), !sparkConf.contains(key) || !sparkConf.get(key).isEmpty()); + } } } - } From aec31d3124fcbbeb6f4f2e89da0a1fa2d348775b Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Fri, 17 Apr 2015 10:44:28 +0900 Subject: [PATCH 6/9] [ZEPPELIN-46] Some spark env must have a valid value - Fixed styles - Fixed some test cases --- .../java/org/apache/zeppelin/spark/SparkInterpreter.java | 3 ++- .../java/org/apache/zeppelin/spark/SparkInterpreterTest.java | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java index 53edca84108..dd55fee41a8 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java @@ -246,8 +246,9 @@ public SparkContext createSparkContext() { .setAppName(getProperty("spark.app.name")) .set("spark.repl.class.uri", classServerUri); - if(0 < jars.length) + if(0 < jars.length) { conf.setJars(jars); + } if (execUri != null) { conf.set("spark.executor.uri", execUri); diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java index 965211510ab..2f4e237cc51 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java @@ -152,9 +152,8 @@ public void emptyConfigurationVariablesOnlyForNonSparkProperties() { String value = (String)intpProperty.get(key); repl.logger.debug(String.format("[%s]: [%s]", key, value)); if (key.startsWith("spark.") && value.isEmpty()) { - if (value.isEmpty()) - assertTrue(String.format("configuration starting from 'spark.' should not be empty. [%s]", key), !sparkConf.contains(key) || !sparkConf.get(key).isEmpty()); - } + assertTrue(String.format("configuration starting from 'spark.' should not be empty. [%s]", key), !sparkConf.contains(key) || !sparkConf.get(key).isEmpty()); + } } } } From f259a5a0c05dd6b394591e11af50b13a68700c84 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Fri, 17 Apr 2015 10:45:42 +0900 Subject: [PATCH 7/9] [ZEPPELIN-46] Some spark env must have a valid value - Fixed styles --- .../org/apache/zeppelin/spark/SparkInterpreterTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java index 2f4e237cc51..cee535237ba 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java @@ -149,11 +149,11 @@ public void emptyConfigurationVariablesOnlyForNonSparkProperties() { SparkConf sparkConf = repl.getSparkContext().getConf(); for (Object oKey : intpProperty.keySet()) { String key = (String) oKey; - String value = (String)intpProperty.get(key); + String value = (String) intpProperty.get(key); repl.logger.debug(String.format("[%s]: [%s]", key, value)); if (key.startsWith("spark.") && value.isEmpty()) { - assertTrue(String.format("configuration starting from 'spark.' should not be empty. [%s]", key), !sparkConf.contains(key) || !sparkConf.get(key).isEmpty()); - } + assertTrue(String.format("configuration starting from 'spark.' should not be empty. [%s]", key), !sparkConf.contains(key) || !sparkConf.get(key).isEmpty()); + } } } } From cd0b4d4a8d1a5866596c0f6cf6a4f599c3094f87 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Fri, 17 Apr 2015 11:26:49 +0900 Subject: [PATCH 8/9] [ZEPPELIN-46] Some spark env must have a valid value - Fixed unused imports --- .../java/org/apache/zeppelin/spark/SparkInterpreterTest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java index cee535237ba..87df793bb54 100644 --- a/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java +++ b/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java @@ -18,7 +18,6 @@ package org.apache.zeppelin.spark; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.io.File; @@ -40,8 +39,6 @@ import org.junit.Test; import org.junit.runners.MethodSorters; -import scala.Tuple2; - @FixMethodOrder(MethodSorters.NAME_ASCENDING) public class SparkInterpreterTest { public static SparkInterpreter repl; From 56a5ce1d7b75713d4be3bbed5b4339cbf2af6a68 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Fri, 17 Apr 2015 17:19:03 +0900 Subject: [PATCH 9/9] ZEPPELIN-46: fix style convention --- .../main/java/org/apache/zeppelin/spark/SparkInterpreter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java index dd55fee41a8..c875e8557b8 100644 --- a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java +++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java @@ -246,7 +246,7 @@ public SparkContext createSparkContext() { .setAppName(getProperty("spark.app.name")) .set("spark.repl.class.uri", classServerUri); - if(0 < jars.length) { + if (jars.length > 0) { conf.setJars(jars); }