[zeppelin] branch master updated: [ZEPPELIN-4698]. empty env is passed to spark interpreter

[zj...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new b8a9386  [ZEPPELIN-4698]. empty env is passed to spark interpreter
b8a9386 is described below

commit b8a93861bf124877f35d3e9883c8598911e0b1e9
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Sat Mar 28 14:38:19 2020 +0800

    [ZEPPELIN-4698]. empty env is passed to spark interpreter
    
    ### What is this PR for?
    This PR just filter the empty env in SparkInterpreterLauncher, only pass non-empty env to spark interpreter. Otherwise PySparkInterpreter will fail to launch due to invalid `SPARK_HOME`
    
    ### What type of PR is it?
    [Bug Fix ]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-4698
    
    ### How should this be tested?
    * Unit test is added
    
    ### 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: Jeff Zhang <zj...@apache.org>
    
    Closes #3701 from zjffdu/ZEPPELIN-4698 and squashes the following commits:
    
    41aa620b0 [Jeff Zhang] [ZEPPELIN-4698]. empty env is passed to spark interpreter
---
 .../zeppelin/interpreter/launcher/SparkInterpreterLauncher.java       | 4 ++--
 .../zeppelin/interpreter/launcher/SparkInterpreterLauncherTest.java   | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java
index 9255c98..02e8f1b 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java
@@ -63,7 +63,7 @@ public class SparkInterpreterLauncher extends StandardInterpreterLauncher {
     Properties sparkProperties = new Properties();
     String sparkMaster = getSparkMaster(properties);
     for (String key : properties.stringPropertyNames()) {
-      if (RemoteInterpreterUtils.isEnvString(key)) {
+      if (RemoteInterpreterUtils.isEnvString(key) && !StringUtils.isBlank(properties.getProperty(key))) {
         env.put(key, properties.getProperty(key));
       }
       if (isSparkConf(key, properties.getProperty(key))) {
@@ -171,7 +171,7 @@ public class SparkInterpreterLauncher extends StandardInterpreterLauncher {
     // we also fallback to zeppelin-env.sh if it is not specified in interpreter setting.
     for (String envName : new String[]{"SPARK_HOME", "SPARK_CONF_DIR", "HADOOP_CONF_DIR"})  {
       String envValue = getEnv(envName);
-      if (envValue != null) {
+      if (!StringUtils.isBlank(envValue)) {
         env.put(envName, envValue);
       }
     }
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncherTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncherTest.java
index 8560ba1..b4b2889 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncherTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncherTest.java
@@ -36,6 +36,7 @@ import java.nio.file.Paths;
 import java.util.Properties;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 public class SparkInterpreterLauncherTest {
@@ -88,6 +89,7 @@ public class SparkInterpreterLauncherTest {
     SparkInterpreterLauncher launcher = new SparkInterpreterLauncher(zConf, null);
     Properties properties = new Properties();
     properties.setProperty("SPARK_HOME", sparkHome);
+    properties.setProperty("ENV_1", "");
     properties.setProperty("property_1", "value_1");
     properties.setProperty("master", "local[*]");
     properties.setProperty("spark.files", "file_1");
@@ -104,6 +106,7 @@ public class SparkInterpreterLauncherTest {
     assertEquals(zConf.getInterpreterRemoteRunnerPath(), interpreterProcess.getInterpreterRunner());
     assertTrue(interpreterProcess.getEnv().size() >= 2);
     assertEquals(sparkHome, interpreterProcess.getEnv().get("SPARK_HOME"));
+    assertFalse(interpreterProcess.getEnv().containsKey("ENV_1"));
     assertEquals(InterpreterLauncher.escapeSpecialCharacter(" --master local[*] --conf spark.files=file_1 --conf spark.jars=jar_1"),
             interpreterProcess.getEnv().get("ZEPPELIN_SPARK_CONF"));
   }