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 <zjf...@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 <zjf...@apache.org>
Authored: Mon Sep 19 10:47:56 2016 +0800
Committer: Jongyoul Lee <jongy...@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)

Reply via email to