Repository: incubator-zeppelin
Updated Branches:
  refs/heads/master 7599efb54 -> d4a2ab430


ZEPPELIN-74 Change interpreter selection from %[Name] to %[Group].[Name]

This PR implements https://issues.apache.org/jira/browse/ZEPPELIN-74.

Plus, changing existing interpreter's name

spark.spark -> spark.scala
spark.pyspark -> spark.py
hive.hive -> hive.hql
tajo.tajo -> tajo.tql

All changes are backward compatible except for spark.pyspark -> spark.py.
User need either %spark.py, %py (when Spark is selected first) instead of 
%pyspark.

Author: Lee moon soo <[email protected]>

Closes #96 from Leemoonsoo/ZEPPELIN-74 and squashes the following commits:

dadc0cd [Lee moon soo] Revert pyspark interpreter name from py -> pyspark
9f9a9c3 [Lee moon soo] Fix test
a5496bc [Lee moon soo] tajo.tajo -> tajo.tql
96705ab [Lee moon soo] hive.hive -> hive.hql
0e2cfb3 [Lee moon soo] Rename spark.pyspark -> spark.py, spark.spark -> 
spark.scala
335bc8f [Lee moon soo] ZEPPELIN-74 Change interpreter selection from %[Name] to 
%[Group].[Name]


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

Branch: refs/heads/master
Commit: d4a2ab430de8eb228ee48b2a04932ed75e1f9826
Parents: 7599efb
Author: Lee moon soo <[email protected]>
Authored: Mon Jun 22 13:15:18 2015 -0700
Committer: Lee moon soo <[email protected]>
Committed: Wed Jun 24 13:14:23 2015 -0700

----------------------------------------------------------------------
 .../apache/zeppelin/hive/HiveInterpreter.java   | 19 +++--
 .../apache/zeppelin/spark/SparkInterpreter.java |  2 +-
 .../apache/zeppelin/tajo/TajoInterpreter.java   | 19 +++--
 .../zeppelin/interpreter/Interpreter.java       |  5 +-
 .../notebook/NoteInterpreterLoader.java         | 65 ++++++++++++---
 .../interpreter/InterpreterFactoryTest.java     |  2 +-
 .../interpreter/mock/MockInterpreter11.java     | 74 +++++++++++++++++
 .../notebook/NoteInterpreterLoaderTest.java     | 84 ++++++++++++++++++++
 8 files changed, 242 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d4a2ab43/hive/src/main/java/org/apache/zeppelin/hive/HiveInterpreter.java
----------------------------------------------------------------------
diff --git a/hive/src/main/java/org/apache/zeppelin/hive/HiveInterpreter.java 
b/hive/src/main/java/org/apache/zeppelin/hive/HiveInterpreter.java
index 9d5c9d7..5c3dee3 100644
--- a/hive/src/main/java/org/apache/zeppelin/hive/HiveInterpreter.java
+++ b/hive/src/main/java/org/apache/zeppelin/hive/HiveInterpreter.java
@@ -17,18 +17,25 @@
  */
 package org.apache.zeppelin.hive;
 
-import java.sql.*;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
 import java.util.List;
 import java.util.Properties;
 
-import org.apache.zeppelin.interpreter.*;
 import org.apache.commons.lang.StringUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
+import org.apache.zeppelin.interpreter.Interpreter;
+import org.apache.zeppelin.interpreter.InterpreterContext;
+import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
+import org.apache.zeppelin.interpreter.InterpreterResult;
 import org.apache.zeppelin.interpreter.InterpreterResult.Code;
 import org.apache.zeppelin.scheduler.Scheduler;
 import org.apache.zeppelin.scheduler.SchedulerFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Hive interpreter for Zeppelin.
@@ -43,7 +50,7 @@ public class HiveInterpreter extends Interpreter {
 
   static {
     Interpreter.register(
-      "hive",
+      "hql",
       "hive",
       HiveInterpreter.class.getName(),
       new InterpreterPropertyBuilder()

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d4a2ab43/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 75ab263..ab3609a 100644
--- a/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java
+++ b/spark/src/main/java/org/apache/zeppelin/spark/SparkInterpreter.java
@@ -106,7 +106,7 @@ public class SparkInterpreter extends Interpreter {
                 "The location of the Spark jar file. If you use yarn as a 
cluster, "
                 + "we should set this value")
             .add("zeppelin.spark.useHiveContext",
-                getSystemDefault("ZEPPELIN_SPARK_USEHIVECONTEXT", 
+                getSystemDefault("ZEPPELIN_SPARK_USEHIVECONTEXT",
                     "zeppelin.spark.useHiveContext", "true"),
                 "Use HiveContext instead of SQLContext if it is true.")
             .add("zeppelin.spark.maxResult",

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d4a2ab43/tajo/src/main/java/org/apache/zeppelin/tajo/TajoInterpreter.java
----------------------------------------------------------------------
diff --git a/tajo/src/main/java/org/apache/zeppelin/tajo/TajoInterpreter.java 
b/tajo/src/main/java/org/apache/zeppelin/tajo/TajoInterpreter.java
index d197903..716a32a 100644
--- a/tajo/src/main/java/org/apache/zeppelin/tajo/TajoInterpreter.java
+++ b/tajo/src/main/java/org/apache/zeppelin/tajo/TajoInterpreter.java
@@ -17,18 +17,25 @@
  */
 package org.apache.zeppelin.tajo;
 
-import java.sql.*;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
 import java.util.List;
 import java.util.Properties;
 
-import org.apache.zeppelin.interpreter.*;
 import org.apache.commons.lang.StringUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
+import org.apache.zeppelin.interpreter.Interpreter;
+import org.apache.zeppelin.interpreter.InterpreterContext;
+import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
+import org.apache.zeppelin.interpreter.InterpreterResult;
 import org.apache.zeppelin.interpreter.InterpreterResult.Code;
 import org.apache.zeppelin.scheduler.Scheduler;
 import org.apache.zeppelin.scheduler.SchedulerFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Tajo interpreter for Zeppelin.
@@ -45,7 +52,7 @@ public class TajoInterpreter extends Interpreter {
 
   static {
     Interpreter.register(
-      "tajo",
+      "tql",
       "tajo",
       TajoInterpreter.class.getName(),
       new InterpreterPropertyBuilder()

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d4a2ab43/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
index b6d0257..3f3503c 100644
--- 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
+++ 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
@@ -267,7 +267,8 @@ public abstract class Interpreter {
 
   public static void register(String name, String group, String className,
       Map<String, InterpreterProperty> properties) {
-    registeredInterpreters.put(name, new RegisteredInterpreter(name, group, 
className, properties));
+    registeredInterpreters.put(group + "." + name, new RegisteredInterpreter(
+        name, group, className, properties));
   }
 
   public static RegisteredInterpreter 
findRegisteredInterpreterByClassName(String className) {
@@ -278,6 +279,4 @@ public abstract class Interpreter {
     }
     return null;
   }
-
-
 }

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d4a2ab43/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java
 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java
index b1fd7b9..509a064 100644
--- 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java
+++ 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java
@@ -22,6 +22,7 @@ import java.util.LinkedList;
 import java.util.List;
 
 import org.apache.zeppelin.interpreter.Interpreter;
+import org.apache.zeppelin.interpreter.Interpreter.RegisteredInterpreter;
 import org.apache.zeppelin.interpreter.InterpreterException;
 import org.apache.zeppelin.interpreter.InterpreterFactory;
 import org.apache.zeppelin.interpreter.InterpreterGroup;
@@ -79,29 +80,71 @@ public class NoteInterpreterLoader {
       return null;
     }
 
-    if (replName == null) {
+    if (replName == null || replName.trim().length() == 0) {
       return settings.get(0).getInterpreterGroup().getFirst();
     }
 
     if (Interpreter.registeredInterpreters == null) {
       return null;
     }
-    Interpreter.RegisteredInterpreter registeredInterpreter
-      = Interpreter.registeredInterpreters.get(replName);
-    if (registeredInterpreter == null || registeredInterpreter.getClassName() 
== null) {
-      throw new InterpreterException(replName + " interpreter not found");
-    }
-    String interpreterClassName = registeredInterpreter.getClassName();
 
-    for (InterpreterSetting setting : settings) {
-      InterpreterGroup intpGroup = setting.getInterpreterGroup();
+    String[] replNameSplit = replName.split("\\.");
+    String group = null;
+    String name = null;
+    if (replNameSplit.length == 2) {
+      group = replNameSplit[0];
+      name = replNameSplit[1];
+
+      Interpreter.RegisteredInterpreter registeredInterpreter = 
Interpreter.registeredInterpreters
+          .get(group + "." + name);
+      if (registeredInterpreter == null
+          || registeredInterpreter.getClassName() == null) {
+        throw new InterpreterException(replName + " interpreter not found");
+      }
+      String interpreterClassName = registeredInterpreter.getClassName();
+
+      for (InterpreterSetting setting : settings) {
+        InterpreterGroup intpGroup = setting.getInterpreterGroup();
+        for (Interpreter interpreter : intpGroup) {
+          if (interpreterClassName.equals(interpreter.getClassName())) {
+            return interpreter;
+          }
+        }
+      }
+    } else {
+      // first assume replName is 'name' of interpreter. ('groupName' is 
ommitted)
+      // search 'name' from first (default) interpreter group
+      InterpreterGroup intpGroup = settings.get(0).getInterpreterGroup();
       for (Interpreter interpreter : intpGroup) {
-        if (interpreterClassName.equals(interpreter.getClassName())) {
+        RegisteredInterpreter intp = Interpreter
+            .findRegisteredInterpreterByClassName(interpreter.getClassName());
+        if (intp == null) {
+          continue;
+        }
+
+        if (intp.getName().equals(replName)) {
+          return interpreter;
+        }
+      }
+
+
+      // next, assume replName is 'group' of interpreter ('name' is ommitted)
+      // search interpreter group and return first interpreter.
+      for (InterpreterSetting setting : settings) {
+        intpGroup = setting.getInterpreterGroup();
+        Interpreter interpreter = intpGroup.get(0);
+        RegisteredInterpreter intp = Interpreter
+            .findRegisteredInterpreterByClassName(interpreter.getClassName());
+        if (intp == null) {
+          continue;
+        }
+
+        if (intp.getGroup().equals(replName)) {
           return interpreter;
         }
       }
     }
 
-    return null;
+    throw new InterpreterException(replName + " interpreter not found");
   }
 }

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d4a2ab43/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 63aef0d..d7f4ab1 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
@@ -48,7 +48,7 @@ public class InterpreterFactoryTest {
     tmpDir.mkdirs();
     new File(tmpDir, "conf").mkdirs();
 
-         MockInterpreter1.register("mock1", 
"org.apache.zeppelin.interpreter.mock.MockInterpreter1");
+    MockInterpreter1.register("mock1", 
"org.apache.zeppelin.interpreter.mock.MockInterpreter1");
          MockInterpreter2.register("mock2", 
"org.apache.zeppelin.interpreter.mock.MockInterpreter2");
 
          System.setProperty(ConfVars.ZEPPELIN_HOME.getVarName(), 
tmpDir.getAbsolutePath());

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d4a2ab43/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java
 
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java
new file mode 100644
index 0000000..89901e5
--- /dev/null
+++ 
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java
@@ -0,0 +1,74 @@
+/*
+ * 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.mock;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+
+import org.apache.zeppelin.interpreter.Interpreter;
+import org.apache.zeppelin.interpreter.InterpreterContext;
+import org.apache.zeppelin.interpreter.InterpreterResult;
+import org.apache.zeppelin.scheduler.Scheduler;
+import org.apache.zeppelin.scheduler.SchedulerFactory;
+
+public class MockInterpreter11 extends Interpreter{
+  Map<String, Object> vars = new HashMap<String, Object>();
+
+  public MockInterpreter11(Properties property) {
+    super(property);
+  }
+
+  @Override
+  public void open() {
+  }
+
+  @Override
+  public void close() {
+  }
+
+  @Override
+  public InterpreterResult interpret(String st, InterpreterContext context) {
+    return new InterpreterResult(InterpreterResult.Code.SUCCESS, "repl11: 
"+st);
+  }
+
+  @Override
+  public void cancel(InterpreterContext context) {
+  }
+
+  @Override
+  public FormType getFormType() {
+    return FormType.SIMPLE;
+  }
+
+  @Override
+  public int getProgress(InterpreterContext context) {
+    return 0;
+  }
+
+  @Override
+  public Scheduler getScheduler() {
+    return 
SchedulerFactory.singleton().createOrGetFIFOScheduler("test_"+this.hashCode());
+  }
+
+  @Override
+  public List<String> completion(String buf, int cursor) {
+    return null;
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d4a2ab43/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java
 
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java
new file mode 100644
index 0000000..2a4e17f
--- /dev/null
+++ 
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java
@@ -0,0 +1,84 @@
+package org.apache.zeppelin.notebook;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.zeppelin.conf.ZeppelinConfiguration;
+import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
+import org.apache.zeppelin.interpreter.InterpreterFactory;
+import org.apache.zeppelin.interpreter.InterpreterOption;
+import org.apache.zeppelin.interpreter.mock.MockInterpreter1;
+import org.apache.zeppelin.interpreter.mock.MockInterpreter11;
+import org.apache.zeppelin.interpreter.mock.MockInterpreter2;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class NoteInterpreterLoaderTest {
+
+  private File tmpDir;
+  private ZeppelinConfiguration conf;
+  private InterpreterFactory factory;
+
+  @Before
+  public void setUp() throws Exception {
+    tmpDir = new 
File(System.getProperty("java.io.tmpdir")+"/ZeppelinLTest_"+System.currentTimeMillis());
+    tmpDir.mkdirs();
+    new File(tmpDir, "conf").mkdirs();
+
+    System.setProperty(ConfVars.ZEPPELIN_HOME.getVarName(), 
tmpDir.getAbsolutePath());
+    System.setProperty(ConfVars.ZEPPELIN_INTERPRETERS.getVarName(), 
"org.apache.zeppelin.interpreter.mock.MockInterpreter1,org.apache.zeppelin.interpreter.mock.MockInterpreter11,org.apache.zeppelin.interpreter.mock.MockInterpreter2");
+
+    conf = ZeppelinConfiguration.create();
+
+    MockInterpreter1.register("mock1", "group1", 
"org.apache.zeppelin.interpreter.mock.MockInterpreter1");
+    MockInterpreter11.register("mock11", "group1", 
"org.apache.zeppelin.interpreter.mock.MockInterpreter11");
+    MockInterpreter2.register("mock2", "group2", 
"org.apache.zeppelin.interpreter.mock.MockInterpreter2");
+
+    factory = new InterpreterFactory(conf, new InterpreterOption(false), null);
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    delete(tmpDir);
+  }
+
+  @Test
+  public void testGetInterpreter() throws IOException {
+    NoteInterpreterLoader loader = new NoteInterpreterLoader(factory);
+    loader.setNoteId("note");
+    loader.setInterpreters(factory.getDefaultInterpreterSettingList());
+
+    // when there're no interpreter selection directive
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", 
loader.get(null).getClassName());
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", 
loader.get("").getClassName());
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", 
loader.get(" ").getClassName());
+
+    // when group name is omitted
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter11", 
loader.get("mock11").getClassName());
+
+    // when 'name' is ommitted
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", 
loader.get("group1").getClassName());
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter2", 
loader.get("group2").getClassName());
+
+    // when nothing is ommitted
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", 
loader.get("group1.mock1").getClassName());
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter11", 
loader.get("group1.mock11").getClassName());
+    assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter2", 
loader.get("group2.mock2").getClassName());
+  }
+
+  private void delete(File file){
+    if(file.isFile()) file.delete();
+    else if(file.isDirectory()){
+      File [] files = file.listFiles();
+      if(files!=null && files.length>0){
+        for(File f : files){
+          delete(f);
+        }
+      }
+      file.delete();
+    }
+  }
+}

Reply via email to