You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by jo...@apache.org on 2016/08/31 08:08:39 UTC

zeppelin git commit: [ZEPPELIN-1383][ Interpreters][r-interpreter] remove SparkInterpreter.getSystemDefault and update relative code

Repository: zeppelin
Updated Branches:
  refs/heads/master d11221fb8 -> 223d225e0


[ZEPPELIN-1383][ Interpreters][r-interpreter] remove SparkInterpreter.getSystemDefault and update relative code

### What is this PR for?
clean some redundant code:
remove `SparkInterpreter.getSystemDefault` methods,
and replace it with `InterpreterProperty.getValue`

### What type of PR is it?
Improvement

### Todos
N/A

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1383
remove SparkInterpreter.getSystemDefault and update relative code

### How should this be tested?
Existing tests.

### 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: WeichenXu <We...@outlook.com>

Closes #1372 from WeichenXu123/remove_spark_interpreter_getSystemDefault and squashes the following commits:

204a34c [WeichenXu] improve code stype.
841b757 [WeichenXu] update.


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/223d225e
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/223d225e
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/223d225e

Branch: refs/heads/master
Commit: 223d225e0119d9e3013206a211df6b70a71b6a66
Parents: d11221f
Author: WeichenXu <We...@outlook.com>
Authored: Sun Aug 28 00:44:26 2016 -0700
Committer: Jongyoul Lee <jo...@apache.org>
Committed: Wed Aug 31 17:07:57 2016 +0900

----------------------------------------------------------------------
 .../zeppelin/rinterpreter/RInterpreter.scala    |  8 +++---
 .../apache/zeppelin/spark/SparkInterpreter.java | 27 +++-----------------
 .../interpreter/InterpreterProperty.java        |  1 -
 .../interpreter/InterpreterPropertyBuilder.java |  7 +++++
 4 files changed, 15 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/223d225e/r/src/main/scala/org/apache/zeppelin/rinterpreter/RInterpreter.scala
----------------------------------------------------------------------
diff --git a/r/src/main/scala/org/apache/zeppelin/rinterpreter/RInterpreter.scala b/r/src/main/scala/org/apache/zeppelin/rinterpreter/RInterpreter.scala
index f0558a9..9504573 100644
--- a/r/src/main/scala/org/apache/zeppelin/rinterpreter/RInterpreter.scala
+++ b/r/src/main/scala/org/apache/zeppelin/rinterpreter/RInterpreter.scala
@@ -111,10 +111,10 @@ object RInterpreter {
 
   // These are the additional properties we need on top of the ones provided by the spark interpreters
   lazy val props: Map[String, InterpreterProperty] = new InterpreterPropertyBuilder()
-    .add("rhadoop.cmd",           SparkInterpreter.getSystemDefault("HADOOP_CMD", "rhadoop.cmd", ""), "Usually /usr/bin/hadoop")
-    .add("rhadooop.streamingjar", SparkInterpreter.getSystemDefault("HADOOP_STREAMING", "rhadooop.streamingjar", ""), "Usually /usr/lib/hadoop/contrib/streaming/hadoop-streaming-<version>.jar")
-    .add("rscala.debug",          SparkInterpreter.getSystemDefault("RSCALA_DEBUG", "rscala.debug","false"), "Whether to turn on rScala debugging") // TEST:  Implemented but not tested
-    .add("rscala.timeout",        SparkInterpreter.getSystemDefault("RSCALA_TIMEOUT", "rscala.timeout","60"), "Timeout for rScala") // TEST:  Implemented but not tested
+    .add("rhadoop.cmd",          "HADOOP_CMD", "rhadoop.cmd", "", "Usually /usr/bin/hadoop")
+    .add("rhadooop.streamingjar", "HADOOP_STREAMING", "rhadooop.streamingjar", "", "Usually /usr/lib/hadoop/contrib/streaming/hadoop-streaming-<version>.jar")
+    .add("rscala.debug",          "RSCALA_DEBUG", "rscala.debug","false", "Whether to turn on rScala debugging") // TEST:  Implemented but not tested
+    .add("rscala.timeout",        "RSCALA_TIMEOUT", "rscala.timeout","60", "Timeout for rScala") // TEST:  Implemented but not tested
     .build
 
   def getProps() = {

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/223d225e/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java
----------------------------------------------------------------------
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 e6ce17f..9a54912 100644
--- a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java
+++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java
@@ -49,6 +49,7 @@ import org.apache.spark.ui.jobs.JobProgressListener;
 import org.apache.zeppelin.interpreter.Interpreter;
 import org.apache.zeppelin.interpreter.InterpreterContext;
 import org.apache.zeppelin.interpreter.InterpreterException;
+import org.apache.zeppelin.interpreter.InterpreterProperty;
 import org.apache.zeppelin.interpreter.InterpreterResult;
 import org.apache.zeppelin.interpreter.InterpreterResult.Code;
 import org.apache.zeppelin.interpreter.InterpreterUtils;
@@ -447,10 +448,11 @@ public class SparkInterpreter extends Interpreter {
   }
 
   private void setupConfForPySpark(SparkConf conf) {
-    String pysparkBasePath = getSystemDefault("SPARK_HOME", null, null);
+    String pysparkBasePath = new InterpreterProperty("SPARK_HOME", null, null, null).getValue();
     File pysparkPath;
     if (null == pysparkBasePath) {
-      pysparkBasePath = getSystemDefault("ZEPPELIN_HOME", "zeppelin.home", "../");
+      pysparkBasePath =
+              new InterpreterProperty("ZEPPELIN_HOME", "zeppelin.home", "../", null).getValue();
       pysparkPath = new File(pysparkBasePath,
           "interpreter" + File.separator + "spark" + File.separator + "pyspark");
     } else {
@@ -500,27 +502,6 @@ public class SparkInterpreter extends Interpreter {
     return null != System.getenv("SPARK_SUBMIT");
   }
 
-  public static String getSystemDefault(
-      String envName,
-      String propertyName,
-      String defaultValue) {
-
-    if (envName != null && !envName.isEmpty()) {
-      String envValue = System.getenv().get(envName);
-      if (envValue != null) {
-        return envValue;
-      }
-    }
-
-    if (propertyName != null && !propertyName.isEmpty()) {
-      String propValue = System.getProperty(propertyName);
-      if (propValue != null) {
-        return propValue;
-      }
-    }
-    return defaultValue;
-  }
-
   public boolean printREPLOutput() {
     return java.lang.Boolean.parseBoolean(getProperty("zeppelin.spark.printREPLOutput"));
   }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/223d225e/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterProperty.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterProperty.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterProperty.java
index 488f2a1..5067586 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterProperty.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterProperty.java
@@ -71,7 +71,6 @@ public class InterpreterProperty {
   }
 
   public String getValue() {
-    //TODO(jongyoul): Remove SparkInterpreter's getSystemDefault method
     if (envName != null && !envName.isEmpty()) {
       String envValue = System.getenv().get(envName);
       if (envValue != null) {

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/223d225e/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterPropertyBuilder.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterPropertyBuilder.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterPropertyBuilder.java
index f077b4e..f33dc7c 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterPropertyBuilder.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterPropertyBuilder.java
@@ -31,6 +31,13 @@ public class InterpreterPropertyBuilder {
     return this;
   }
 
+  public InterpreterPropertyBuilder add(String name, String envName, String propertyName,
+        String defaultValue, String description){
+    properties.put(name,
+            new InterpreterProperty(envName, propertyName, defaultValue, description));
+    return this;
+  }
+
   public Map<String, InterpreterProperty> build(){
     return properties;
   }