Repository: incubator-zeppelin
Updated Branches:
  refs/heads/master cbed7c453 -> 77dd2f35a


[ZEPPELIN-95] Make environment variables and jvm properties customizable per 
Interpreter

### What is this PR for?
This PR make environment variable and jvm properties customizable per 
interpreter settings.
by export property value as environment variable when property key is matches 
pattern [A-Z_0-9] otherwise set property value as JVM property.

### What type of PR is it?
Feature

### Todos
* [x] - export property as environment variable when property name matches 
pattern [A-Z_0-9]
* [x] - set property as JVM property when property name does not match pattern 
[A-Z_0-9]
* [x] - unit test
* [x] - doc

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-95

### How should this be tested?
set some property in interpreter setting page, such as

| name | value|
| ------------- | ------------- |
| MY_ENV_VAR  |  some value |
| my.jvm.property | some value |

And read value in interpreter. (for example spark interpreter let user access 
environment variable and jvm property)

### Screenshots (if appropriate)
N/A

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? yes

Author: Lee moon soo <[email protected]>

Closes #789 from Leemoonsoo/ZEPPELIN-95 and squashes the following commits:

1a97b12 [Lee moon soo] Clear property when property value is empty
37ffda1 [Lee moon soo] Add docs
f5a130a [Lee moon soo] export interpreter property as environment variable or 
jvm property


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

Branch: refs/heads/master
Commit: 77dd2f35a9f82c02aeff09ab664287d8a93a2995
Parents: cbed7c4
Author: Lee moon soo <[email protected]>
Authored: Wed Mar 23 08:41:32 2016 -0700
Committer: Lee moon soo <[email protected]>
Committed: Sat Apr 16 06:47:40 2016 +0100

----------------------------------------------------------------------
 docs/manual/interpreters.md                     |  2 +
 .../interpreter/remote/RemoteInterpreter.java   | 21 ++++-
 .../remote/RemoteInterpreterServer.java         | 14 ++++
 .../remote/RemoteInterpreterTest.java           | 56 +++++++++++++
 .../remote/mock/MockInterpreterEnv.java         | 87 ++++++++++++++++++++
 5 files changed, 179 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/77dd2f35/docs/manual/interpreters.md
----------------------------------------------------------------------
diff --git a/docs/manual/interpreters.md b/docs/manual/interpreters.md
index f23600f..43af964 100644
--- a/docs/manual/interpreters.md
+++ b/docs/manual/interpreters.md
@@ -36,6 +36,8 @@ Zeppelin interpreter setting is the configuration of a given 
interpreter on Zepp
 
 <img src="/assets/themes/zeppelin/img/screenshots/interpreter_setting.png">
 
+Properties are exported as environment variable when property name is 
consisted of upper characters, numbers and underscore ([A-Z_0-9]). Otherwise 
set properties as JVM property.
+
 Each notebook can be binded to multiple Interpreter Settings using setting 
icon on upper right corner of the notebook.
 
 <img src="/assets/themes/zeppelin/img/screenshots/interpreter_binding.png" 
width="800px">

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/77dd2f35/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreter.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreter.java
 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreter.java
index 137b605..e4273c4 100644
--- 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreter.java
+++ 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreter.java
@@ -71,7 +71,7 @@ public class RemoteInterpreter extends Interpreter {
     this.interpreterRunner = interpreterRunner;
     this.interpreterPath = interpreterPath;
     this.localRepoPath = localRepoPath;
-    env = new HashMap<String, String>();
+    env = getEnvFromInterpreterProperty(property);
     this.connectTimeout = connectTimeout;
     this.maxPoolSize = maxPoolSize;
     this.remoteInterpreterProcessListener = remoteInterpreterProcessListener;
@@ -92,12 +92,31 @@ public class RemoteInterpreter extends Interpreter {
     this.interpreterRunner = interpreterRunner;
     this.interpreterPath = interpreterPath;
     this.localRepoPath = localRepoPath;
+    env.putAll(getEnvFromInterpreterProperty(property));
     this.env = env;
     this.connectTimeout = connectTimeout;
     this.maxPoolSize = 10;
     this.remoteInterpreterProcessListener = remoteInterpreterProcessListener;
   }
 
+  private Map<String, String> getEnvFromInterpreterProperty(Properties 
property) {
+    Map<String, String> env = new HashMap<String, String>();
+    for (Object key : property.keySet()) {
+      if (isEnvString((String) key)) {
+        env.put((String) key, property.getProperty((String) key));
+      }
+    }
+    return env;
+  }
+
+  static boolean isEnvString(String key) {
+    if (key == null || key.length() == 0) {
+      return false;
+    }
+
+    return key.matches("^[A-Z_0-9]*");
+  }
+
   @Override
   public String getClassName() {
     return className;

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/77dd2f35/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
index 6e369c0..b585e31 100644
--- 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
+++ 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
@@ -151,6 +151,7 @@ public class RemoteInterpreterServer
       Class<Interpreter> replClass = (Class<Interpreter>) 
Object.class.forName(className);
       Properties p = new Properties();
       p.putAll(properties);
+      setSystemProperty(p);
 
       Constructor<Interpreter> constructor =
           replClass.getConstructor(new Class[] {Properties.class});
@@ -179,6 +180,19 @@ public class RemoteInterpreterServer
     }
   }
 
+  private void setSystemProperty(Properties properties) {
+    for (Object key : properties.keySet()) {
+      if (!RemoteInterpreter.isEnvString((String) key)) {
+        String value = properties.getProperty((String) key);
+        if (value == null || value.isEmpty()) {
+          System.clearProperty((String) key);
+        } else {
+          System.setProperty((String) key, properties.getProperty((String) 
key));
+        }
+      }
+    }
+  }
+
   private Interpreter getInterpreter(String noteId, String className) throws 
TException {
     if (interpreterGroup == null) {
       throw new TException(

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/77dd2f35/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
 
b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
index b500906..ea0bbf4 100644
--- 
a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
+++ 
b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
@@ -30,6 +30,7 @@ import java.util.Properties;
 import org.apache.thrift.transport.TTransportException;
 import org.apache.zeppelin.display.AngularObject;
 import org.apache.zeppelin.display.AngularObjectRegistry;
+import org.apache.zeppelin.interpreter.remote.mock.MockInterpreterEnv;
 import org.apache.zeppelin.interpreter.thrift.RemoteInterpreterService;
 import org.apache.zeppelin.interpreter.thrift.RemoteInterpreterService.Client;
 import org.apache.zeppelin.user.AuthenticationInfo;
@@ -701,4 +702,59 @@ public class RemoteInterpreterTest {
     Mockito.verify(client).angularRegistryPush(expected);
   }
 
+  @Test
+  public void testEnvStringPattern() {
+    assertFalse(RemoteInterpreter.isEnvString(null));
+    assertFalse(RemoteInterpreter.isEnvString(""));
+    assertFalse(RemoteInterpreter.isEnvString("abcDEF"));
+    assertFalse(RemoteInterpreter.isEnvString("ABC-DEF"));
+    assertTrue(RemoteInterpreter.isEnvString("ABCDEF"));
+    assertTrue(RemoteInterpreter.isEnvString("ABC_DEF"));
+    assertTrue(RemoteInterpreter.isEnvString("ABC_DEF123"));
+  }
+
+  @Test
+  public void testEnvronmentAndPropertySet() {
+    Properties p = new Properties();
+    p.setProperty("MY_ENV1", "env value 1");
+    p.setProperty("my.property.1", "property value 1");
+
+    RemoteInterpreter intp = new RemoteInterpreter(
+        p,
+        "note",
+        MockInterpreterEnv.class.getName(),
+        new File("../bin/interpreter.sh").getAbsolutePath(),
+        "fake",
+        "fakeRepo",
+        env,
+        10 * 1000,
+        null);
+
+    intpGroup.put("note", new LinkedList<Interpreter>());
+    intpGroup.get("note").add(intp);
+    intp.setInterpreterGroup(intpGroup);
+
+    intp.open();
+
+    InterpreterContext context = new InterpreterContext(
+        "noteId",
+        "id",
+        "title",
+        "text",
+        new AuthenticationInfo(),
+        new HashMap<String, Object>(),
+        new GUI(),
+        new AngularObjectRegistry(intpGroup.getId(), null),
+        new LocalResourcePool("pool1"),
+        new LinkedList<InterpreterContextRunner>(), null);
+
+
+    assertEquals("env value 1", intp.interpret("getEnv MY_ENV1", 
context).message());
+    assertEquals("", intp.interpret("getProperty MY_ENV1", context).message());
+    assertEquals("", intp.interpret("getEnv my.property.1", 
context).message());
+    assertEquals("property value 1", intp.interpret("getProperty 
my.property.1", context).message());
+
+    intp.close();
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/77dd2f35/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/mock/MockInterpreterEnv.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/mock/MockInterpreterEnv.java
 
b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/mock/MockInterpreterEnv.java
new file mode 100644
index 0000000..bc71acc
--- /dev/null
+++ 
b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/mock/MockInterpreterEnv.java
@@ -0,0 +1,87 @@
+/*
+ * 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.interpreter.remote.mock;
+
+import org.apache.zeppelin.interpreter.*;
+import org.apache.zeppelin.scheduler.Scheduler;
+import org.apache.zeppelin.scheduler.SchedulerFactory;
+
+import java.util.List;
+import java.util.Properties;
+
+
+public class MockInterpreterEnv extends Interpreter {
+  static {
+    Interpreter.register(
+        "interpreterA",
+        "group1",
+        MockInterpreterA.class.getName(),
+        new InterpreterPropertyBuilder().build());
+
+  }
+
+
+  public MockInterpreterEnv(Properties property) {
+    super(property);
+  }
+
+  @Override
+  public void open() {
+  }
+
+  @Override
+  public void close() {
+  }
+
+  @Override
+  public InterpreterResult interpret(String st, InterpreterContext context) {
+    String[] cmd = st.split(" ");
+    if (cmd[0].equals("getEnv")) {
+      return new InterpreterResult(InterpreterResult.Code.SUCCESS, 
System.getenv(cmd[1]));
+    } else if (cmd[0].equals("getProperty")){
+      return new InterpreterResult(InterpreterResult.Code.SUCCESS, 
System.getProperty(cmd[1]));
+    } else {
+      return new InterpreterResult(InterpreterResult.Code.ERROR, cmd[0]);
+    }
+  }
+
+  @Override
+  public void cancel(InterpreterContext context) {
+
+  }
+
+  @Override
+  public FormType getFormType() {
+    return FormType.NATIVE;
+  }
+
+  @Override
+  public int getProgress(InterpreterContext context) {
+    return 0;
+  }
+
+  @Override
+  public List<String> completion(String buf, int cursor) {
+    return null;
+  }
+
+  @Override
+  public Scheduler getScheduler() {
+    return 
SchedulerFactory.singleton().createOrGetFIFOScheduler("interpreter_" + 
this.hashCode());
+  }
+}
+

Reply via email to