You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2016/09/10 17:44:15 UTC
zeppelin git commit: [ZEPPELIN-1334] Environment variable defined in
interpreter setting doesn't take effect
Repository: zeppelin
Updated Branches:
refs/heads/master 2a7fb95c2 -> db99ccb70
[ZEPPELIN-1334] Environment variable defined in interpreter setting doesn't take effect
### What is this PR for?
I define SPAKR_HOME in interpreter setting, but it doesn't take effect. This PR is for bring back the environment variable defined in interpreter setting. The root cause is that we reset the env after creating RemoteInterpreter.
```
new RemoteInterpreter(property, noteId, className, conf.getInterpreterRemoteRunnerPath(),
interpreterPath, localRepoPath, connectTimeout, maxPoolSize,
remoteInterpreterProcessListener, appEventListener);
remoteInterpreter.setEnv(env);
```
### What type of PR is it?
[Bug Fix]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-1334
### How should this be tested?
Tested manually. Create 2 spark interpreter setting, one for spark1 another is for spark2. And define SPARK_HOME for each interpreter. Then I can run both spark1 and spark2 in one zeppelin instance.
### 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 #1333 from zjffdu/ZEPPELIN-1334 and squashes the following commits:
febbf3f [Jeff Zhang] fix unit test
dd59b35 [Jeff Zhang] fix code style
39c9140 [Jeff Zhang] add test case
32ae1a2 [Jeff Zhang] [ZEPPELIN-1334] Environment variable defined in interpreter setting doesn't take effect
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/db99ccb7
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/db99ccb7
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/db99ccb7
Branch: refs/heads/master
Commit: db99ccb7056282033978244fe1ffb3f204c0aedd
Parents: 2a7fb95
Author: Jeff Zhang <zj...@apache.org>
Authored: Thu Sep 8 13:40:06 2016 +0800
Committer: Lee moon soo <mo...@apache.org>
Committed: Sat Sep 10 10:44:11 2016 -0700
----------------------------------------------------------------------
.../interpreter/remote/RemoteInterpreter.java | 7 +++++
zeppelin-zengine/pom.xml | 9 ++++++
.../interpreter/InterpreterFactory.java | 2 +-
.../interpreter/InterpreterFactoryTest.java | 31 +++++++++++++++++++-
4 files changed, 47 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/db99ccb7/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 3e6fa39..073b84b 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
@@ -527,4 +527,11 @@ public class RemoteInterpreter extends Interpreter {
public void setEnv(Map<String, String> env) {
this.env = env;
}
+
+ public void addEnv(Map<String, String> env) {
+ if (this.env == null) {
+ this.env = new HashMap<>();
+ }
+ this.env.putAll(env);
+ }
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/db99ccb7/zeppelin-zengine/pom.xml
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/pom.xml b/zeppelin-zengine/pom.xml
index 2c56b4d..84fb624 100644
--- a/zeppelin-zengine/pom.xml
+++ b/zeppelin-zengine/pom.xml
@@ -252,5 +252,14 @@
<filtering>true</filtering>
</resource>
</resources>
+ <plugins>
+ <plugin>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>2.17</version>
+ <configuration combine.children="append">
+ <forkMode>always</forkMode>
+ </configuration>
+ </plugin>
+ </plugins>
</build>
</project>
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/db99ccb7/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
index 30b0153..ab67b5b 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
@@ -994,7 +994,7 @@ public class InterpreterFactory implements InterpreterGroupFactory {
new RemoteInterpreter(property, noteId, className, conf.getInterpreterRemoteRunnerPath(),
interpreterPath, localRepoPath, connectTimeout, maxPoolSize,
remoteInterpreterProcessListener, appEventListener);
- remoteInterpreter.setEnv(env);
+ remoteInterpreter.addEnv(env);
return new LazyOpenInterpreter(remoteInterpreter);
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/db99ccb7/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
index 5e44d62..5cdda05 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
@@ -21,6 +21,8 @@ import java.io.*;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
import java.util.Properties;
import org.apache.commons.io.FileUtils;
@@ -31,6 +33,7 @@ import org.apache.zeppelin.dep.Dependency;
import org.apache.zeppelin.dep.DependencyResolver;
import org.apache.zeppelin.interpreter.mock.MockInterpreter1;
import org.apache.zeppelin.interpreter.mock.MockInterpreter2;
+import org.apache.zeppelin.interpreter.remote.RemoteInterpreter;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -52,7 +55,10 @@ public class InterpreterFactoryTest {
tmpDir.mkdirs();
new File(tmpDir, "conf").mkdirs();
- MockInterpreter1.register("mock1", "org.apache.zeppelin.interpreter.mock.MockInterpreter1");
+ Map<String, InterpreterProperty> propertiesMockInterpreter1 = new HashMap<String, InterpreterProperty>();
+ propertiesMockInterpreter1.put("PROPERTY_1", new InterpreterProperty("PROPERTY_1", "", "VALUE_1", "desc"));
+ propertiesMockInterpreter1.put("property_2", new InterpreterProperty("", "property_2", "value_2", "desc"));
+ MockInterpreter1.register("mock1", "mock1", "org.apache.zeppelin.interpreter.mock.MockInterpreter1", propertiesMockInterpreter1);
MockInterpreter2.register("mock2", "org.apache.zeppelin.interpreter.mock.MockInterpreter2");
System.setProperty(ConfVars.ZEPPELIN_HOME.getVarName(), tmpDir.getAbsolutePath());
@@ -96,6 +102,29 @@ public class InterpreterFactoryTest {
}
@Test
+ public void testRemoteRepl() throws Exception {
+ factory = new InterpreterFactory(conf, new InterpreterOption(true), null, null, null, depResolver);
+ List<InterpreterSetting> all = factory.get();
+ InterpreterSetting mock1Setting = null;
+ for (InterpreterSetting setting : all) {
+ if (setting.getName().equals("mock1")) {
+ mock1Setting = setting;
+ break;
+ }
+ }
+ InterpreterGroup interpreterGroup = mock1Setting.getInterpreterGroup("sharedProcess");
+ factory.createInterpretersForNote(mock1Setting, "sharedProcess", "session");
+ // get interpreter
+ assertNotNull("get Interpreter", interpreterGroup.get("session").get(0));
+ assertTrue(interpreterGroup.get("session").get(0) instanceof LazyOpenInterpreter);
+ LazyOpenInterpreter lazyInterpreter = (LazyOpenInterpreter)(interpreterGroup.get("session").get(0));
+ assertTrue(lazyInterpreter.getInnerInterpreter() instanceof RemoteInterpreter);
+ RemoteInterpreter remoteInterpreter = (RemoteInterpreter) lazyInterpreter.getInnerInterpreter();
+ assertEquals("VALUE_1", remoteInterpreter.getEnv().get("PROPERTY_1"));
+ assertEquals("value_2", remoteInterpreter.getProperty("property_2"));
+ }
+
+ @Test
public void testFactoryDefaultList() throws IOException, RepositoryException {
// get default settings
List<String> all = factory.getDefaultInterpreterSettingList();