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/09/20 07:18:21 UTC

zeppelin git commit: ZEPPELIN-1368. interpreter-setting.json may be loaded mutliple times

Repository: zeppelin
Updated Branches:
  refs/heads/master ba12ea3ed -> 9b8421806


ZEPPELIN-1368. interpreter-setting.json may be loaded mutliple times

### What is this PR for?
here're several ways to load interpreter-setting.json, but for now we will load it multiple times. It is supposed to load only once. We should load it by the following orders
         * 1. Register it from path {ZEPPELIN_HOME}/interpreter/{interpreter_name}/ interpreter-setting.json
         * 2. Register it from interpreter-setting.json in classpath {ZEPPELIN_HOME}/interpreter/{interpreter_name}
         * 3. Register it by Interpreter.register

### What type of PR is it?
[Bug Fix]

### Todos
* [ ] - Task

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

### How should this be tested?
Check the log that each interpreter is registered once. And also modify file interpreter/spark/interpreter-setting.json to make pyspark as the default interpreter and it works. Before this PR, it doesn't work, because it would be override by interpreter-setting.json in `interpreter/spark/zeppelin-spark_2.10-0.7.0-SNAPSHOT.jar`

### Screenshots (if appropriate)
![image](https://cloud.githubusercontent.com/assets/164491/18621557/a4510966-7e57-11e6-8c9a-80697ebf2600.png)

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

8266d12 [Jeff Zhang] ZEPPELIN-1368. interpreter-setting.json may be loaded mutliple times


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

Branch: refs/heads/master
Commit: 9b8421806038ff026ebab5d97f0b5d7d6b1b103c
Parents: ba12ea3
Author: Jeff Zhang <zj...@apache.org>
Authored: Mon Sep 19 10:47:56 2016 +0800
Committer: Jongyoul Lee <jo...@apache.org>
Committed: Tue Sep 20 16:18:11 2016 +0900

----------------------------------------------------------------------
 .../zeppelin/interpreter/Interpreter.java       |  6 +-
 .../interpreter/InterpreterFactory.java         | 64 ++++++++++++--------
 2 files changed, 42 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9b842180/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 42caafd..6d7d660 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
@@ -338,9 +338,9 @@ public abstract class Interpreter {
   @Deprecated
   public static void register(String name, String group, String className,
       boolean defaultInterpreter, Map<String, InterpreterProperty> properties) {
-    logger.error("Static initialization is deprecated. You should change it to use " +
-                     "interpreter-setting.json in your jar or " +
-                     "interpreter/{interpreter}/interpreter-setting.json");
+    logger.warn("Static initialization is deprecated for interpreter {}, You should change it " +
+                     "to use interpreter-setting.json in your jar or " +
+                     "interpreter/{interpreter}/interpreter-setting.json", name);
     register(new RegisteredInterpreter(name, group, className, defaultInterpreter, properties));
   }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9b842180/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 7732a45..5545e9b 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
@@ -171,32 +171,42 @@ public class InterpreterFactory implements InterpreterGroupFactory {
           })) {
         String interpreterDirString = interpreterDir.toString();
 
-        registerInterpreterFromPath(interpreterDirString, interpreterJson);
-
-        registerInterpreterFromResource(cl, interpreterDirString, interpreterJson);
-
-        /*
-         * TODO(jongyoul)
-         * - Remove these codes below because of legacy code
-         * - Support ThreadInterpreter
+        /**
+         * Register interpreter by the following ordering
+         * 1. Register it from path {ZEPPELIN_HOME}/interpreter/{interpreter_name}/
+         *    interpreter-setting.json
+         * 2. Register it from interpreter-setting.json in classpath
+         *    {ZEPPELIN_HOME}/interpreter/{interpreter_name}
+         * 3. Register it by Interpreter.register
          */
-        URLClassLoader ccl = new URLClassLoader(recursiveBuildLibList(interpreterDir.toFile()), cl);
-        for (String className : interpreterClassList) {
-          try {
-            // Load classes
-            Class.forName(className, true, ccl);
-            Set<String> interpreterKeys = Interpreter.registeredInterpreters.keySet();
-            for (String interpreterKey : interpreterKeys) {
-              if (className
-                  .equals(Interpreter.registeredInterpreters.get(interpreterKey).getClassName())) {
-                Interpreter.registeredInterpreters.get(interpreterKey)
-                    .setPath(interpreterDirString);
-                logger.info("Interpreter " + interpreterKey + " found. class=" + className);
-                cleanCl.put(interpreterDirString, ccl);
+        if (!registerInterpreterFromPath(interpreterDirString, interpreterJson)) {
+          if (!registerInterpreterFromResource(cl, interpreterDirString, interpreterJson)) {
+            /*
+             * TODO(jongyoul)
+             * - Remove these codes below because of legacy code
+             * - Support ThreadInterpreter
+            */
+            URLClassLoader ccl = new URLClassLoader(
+                    recursiveBuildLibList(interpreterDir.toFile()), cl);
+            for (String className : interpreterClassList) {
+              try {
+                // Load classes
+                Class.forName(className, true, ccl);
+                Set<String> interpreterKeys = Interpreter.registeredInterpreters.keySet();
+                for (String interpreterKey : interpreterKeys) {
+                  if (className
+                          .equals(Interpreter.registeredInterpreters.get(interpreterKey)
+                                  .getClassName())) {
+                    Interpreter.registeredInterpreters.get(interpreterKey)
+                            .setPath(interpreterDirString);
+                    logger.info("Interpreter " + interpreterKey + " found. class=" + className);
+                    cleanCl.put(interpreterDirString, ccl);
+                  }
+                }
+              } catch (Throwable t) {
+                // nothing to do
               }
             }
-          } catch (Throwable t) {
-            // nothing to do
           }
         }
       }
@@ -277,7 +287,7 @@ public class InterpreterFactory implements InterpreterGroupFactory {
     return properties;
   }
 
-  private void registerInterpreterFromResource(ClassLoader cl, String interpreterDir,
+  private boolean registerInterpreterFromResource(ClassLoader cl, String interpreterDir,
       String interpreterJson) throws IOException, RepositoryException {
     URL[] urls = recursiveBuildLibList(new File(interpreterDir));
     ClassLoader tempClassLoader = new URLClassLoader(urls, cl);
@@ -289,10 +299,12 @@ public class InterpreterFactory implements InterpreterGroupFactory {
       List<RegisteredInterpreter> registeredInterpreterList =
           getInterpreterListFromJson(inputStream);
       registerInterpreters(registeredInterpreterList, interpreterDir);
+      return true;
     }
+    return false;
   }
 
-  private void registerInterpreterFromPath(String interpreterDir, String interpreterJson)
+  private boolean registerInterpreterFromPath(String interpreterDir, String interpreterJson)
       throws IOException, RepositoryException {
 
     Path interpreterJsonPath = Paths.get(interpreterDir, interpreterJson);
@@ -301,7 +313,9 @@ public class InterpreterFactory implements InterpreterGroupFactory {
       List<RegisteredInterpreter> registeredInterpreterList =
           getInterpreterListFromJson(interpreterJsonPath);
       registerInterpreters(registeredInterpreterList, interpreterDir);
+      return true;
     }
+    return false;
   }
 
   private List<RegisteredInterpreter> getInterpreterListFromJson(Path filename)