You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2018/08/01 06:17:01 UTC

zeppelin git commit: ZEPPELIN-3666. Use zeppelin.interpreter.default to replace zeppelin.interpreter.group.order

Repository: zeppelin
Updated Branches:
  refs/heads/master 590068f54 -> 0951f2171


ZEPPELIN-3666. Use zeppelin.interpreter.default to replace zeppelin.interpreter.group.order

### What is this PR for?

Actually user only care about the default interpreter rather than the whole interpreter order, so that we can use zeppelin.interpreter.default to replace zeppelin.interpreter.group.order, otherwise we may miss to update zeppelin.interpreter.group.order when adding new interpreter

### What type of PR is it?
[Refactoring]

### Todos
* [ ] - Task

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

### How should this be tested?
* CI pass

### 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 #3103 from zjffdu/ZEPPELIN-3666 and squashes the following commits:

741f51654 [Jeff Zhang] ZEPPELIN-3666. Use zeppelin.interpreter.default to replace zeppelin.interpreter.group.order


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

Branch: refs/heads/master
Commit: 0951f2171ddfeda1e1206066ef0a3521015206e3
Parents: 590068f
Author: Jeff Zhang <zj...@apache.org>
Authored: Fri Jul 27 13:46:55 2018 +0800
Committer: Jeff Zhang <zj...@apache.org>
Committed: Wed Aug 1 14:16:55 2018 +0800

----------------------------------------------------------------------
 .../zeppelin/conf/ZeppelinConfiguration.java    |  4 +---
 .../zeppelin/rest/AbstractTestRestApi.java      |  8 ++++---
 .../interpreter/InterpreterSettingManager.java  | 23 ++++----------------
 .../helium/HeliumApplicationFactoryTest.java    |  1 -
 .../interpreter/AbstractInterpreterTest.java    |  2 +-
 .../interpreter/InterpreterFactoryTest.java     |  4 ++--
 .../apache/zeppelin/notebook/NotebookTest.java  |  1 -
 7 files changed, 13 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0951f217/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
index d63eb01..c6252c0 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
@@ -701,9 +701,7 @@ public class ZeppelinConfiguration extends XMLConfiguration {
         "http://repo1.maven.org/maven2/"),
     ZEPPELIN_INTERPRETER_CONNECT_TIMEOUT("zeppelin.interpreter.connect.timeout", 60000),
     ZEPPELIN_INTERPRETER_MAX_POOL_SIZE("zeppelin.interpreter.max.poolsize", 10),
-    ZEPPELIN_INTERPRETER_GROUP_ORDER("zeppelin.interpreter.group.order", "spark,md,angular,sh,"
-        + "livy,alluxio,file,psql,flink,python,ignite,lens,cassandra,geode,kylin,elasticsearch,"
-        + "scalding,jdbc,hbase,bigquery,beam,pig,scio,groovy,neo4j"),
+    ZEPPELIN_INTERPRETER_DEFAULT("zeppelin.interpreter.default", "spark"),
     ZEPPELIN_INTERPRETER_OUTPUT_LIMIT("zeppelin.interpreter.output.limit", 1024 * 100),
     ZEPPELIN_ENCODING("zeppelin.encoding", "UTF-8"),
     ZEPPELIN_NOTEBOOK_DIR("zeppelin.notebook.dir", "notebook"),

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0951f217/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
index 4dfb46f..6beb179 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
@@ -179,11 +179,13 @@ public abstract class AbstractTestRestApi {
       confDir.mkdirs();
 
       System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_HOME.getVarName(),
-              zeppelinHome.getAbsolutePath());
+          zeppelinHome.getAbsolutePath());
       System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_WAR.getVarName(),
-              new File("../zeppelin-web/dist").getAbsolutePath());
+          new File("../zeppelin-web/dist").getAbsolutePath());
       System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_CONF_DIR.getVarName(),
-              confDir.getAbsolutePath());
+          confDir.getAbsolutePath());
+      System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_DEFAULT.getVarName(),
+          "spark");
 
       // some test profile does not build zeppelin-web.
       // to prevent zeppelin starting up fail, create zeppelin-web/dist directory

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0951f217/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
index ff65c3a..c36a3b1 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
@@ -113,7 +113,7 @@ public class InterpreterSettingManager implements InterpreterSettingManagerMBean
 
   private final List<RemoteRepository> interpreterRepositories;
   private InterpreterOption defaultOption;
-  private List<String> interpreterGroupOrderList;
+  private String defaultInterpreterGroup;
   private final Gson gson;
 
   private AngularObjectRegistryListener angularObjectRegistryListener;
@@ -152,8 +152,7 @@ public class InterpreterSettingManager implements InterpreterSettingManagerMBean
     this.dependencyResolver =
         new DependencyResolver(conf.getString(ConfVars.ZEPPELIN_INTERPRETER_LOCALREPO));
     this.interpreterRepositories = dependencyResolver.getRepos();
-    this.interpreterGroupOrderList =
-        Arrays.asList(conf.getString(ConfVars.ZEPPELIN_INTERPRETER_GROUP_ORDER).split(","));
+    this.defaultInterpreterGroup = conf.getString(ConfVars.ZEPPELIN_INTERPRETER_DEFAULT);
     this.gson = new GsonBuilder().setPrettyPrinting().create();
 
     this.angularObjectRegistryListener = angularObjectRegistryListener;
@@ -899,23 +898,9 @@ public class InterpreterSettingManager implements InterpreterSettingManagerMBean
     Collections.sort(orderedSettings, new Comparator<InterpreterSetting>() {
       @Override
       public int compare(InterpreterSetting o1, InterpreterSetting o2) {
-        int i = interpreterGroupOrderList.indexOf(o1.getGroup());
-        int j = interpreterGroupOrderList.indexOf(o2.getGroup());
-        if (i < 0) {
-          LOGGER.warn("InterpreterGroup " + o1.getGroup()
-              + " is not specified in " + ConfVars.ZEPPELIN_INTERPRETER_GROUP_ORDER.getVarName());
-          // move the unknown interpreter to last
-          i = Integer.MAX_VALUE;
-        }
-        if (j < 0) {
-          LOGGER.warn("InterpreterGroup " + o2.getGroup()
-              + " is not specified in " + ConfVars.ZEPPELIN_INTERPRETER_GROUP_ORDER.getVarName());
-          // move the unknown interpreter to last
-          j = Integer.MAX_VALUE;
-        }
-        if (i < j) {
+        if (o1.getGroup().equals(defaultInterpreterGroup)) {
           return -1;
-        } else if (i > j) {
+        } else if (o2.getGroup().equals(defaultInterpreterGroup)) {
           return 1;
         } else {
           return o1.getName().compareTo(o2.getName());

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0951f217/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java
index e485078..5e4bfe1 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java
@@ -59,7 +59,6 @@ public class HeliumApplicationFactoryTest extends AbstractInterpreterTest implem
 
   @Before
   public void setUp() throws Exception {
-    System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_GROUP_ORDER.getVarName(), "mock1,mock2");
     super.setUp();
 
     this.schedulerFactory = SchedulerFactory.singleton();

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0951f217/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/AbstractInterpreterTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/AbstractInterpreterTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/AbstractInterpreterTest.java
index 1628596..dd24dfc 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/AbstractInterpreterTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/AbstractInterpreterTest.java
@@ -54,7 +54,7 @@ public abstract class AbstractInterpreterTest {
     System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_CONF_DIR.getVarName(), confDir.getAbsolutePath());
     System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_DIR.getVarName(), interpreterDir.getAbsolutePath());
     System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(), notebookDir.getAbsolutePath());
-    System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_GROUP_ORDER.getVarName(), "test,mock1,mock2,mock_resource_pool");
+    System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_DEFAULT.getVarName(), "test");
 
     conf = new ZeppelinConfiguration();
     interpreterSettingManager = new InterpreterSettingManager(conf,

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0951f217/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 2fef05d..8415a57 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
@@ -30,6 +30,7 @@ import static org.junit.Assert.fail;
 
 public class InterpreterFactoryTest extends AbstractInterpreterTest {
 
+
   @Test
   public void testGetFactory() throws IOException, InterpreterException {
     // no default interpreter because there's no interpreter setting binded to this note
@@ -42,8 +43,7 @@ public class InterpreterFactoryTest extends AbstractInterpreterTest {
     interpreterSettingManager.setInterpreterBinding("user1", "note1", interpreterSettingManager.getSettingIds());
     assertTrue(interpreterFactory.getInterpreter("user1", "note1", "") instanceof RemoteInterpreter);
     RemoteInterpreter remoteInterpreter = (RemoteInterpreter) interpreterFactory.getInterpreter("user1", "note1", "");
-    // EchoInterpreter is the default interpreter because mock1 is the default interpreter group
-
+    // EchoInterpreter is the default interpreter because test is the default interpreter group
     assertEquals(EchoInterpreter.class.getName(), remoteInterpreter.getClassName());
 
     assertTrue(interpreterFactory.getInterpreter("user1", "note1", "test") instanceof RemoteInterpreter);

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0951f217/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
index 4158eec..32db88b 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
@@ -88,7 +88,6 @@ public class NotebookTest extends AbstractInterpreterTest implements JobListener
   @Before
   public void setUp() throws Exception {
     System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_PUBLIC.getVarName(), "true");
-    System.setProperty(ConfVars.ZEPPELIN_INTERPRETER_GROUP_ORDER.getVarName(), "mock1,mock2");
     System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_CRON_ENABLE.getVarName(), "true");
     super.setUp();