Repository: zeppelin
Updated Branches:
  refs/heads/master 0f81b6d61 -> 23c5cac82


ZEPPELIN-3029. Cannot delete an interpreter

### What is this PR for?

Only copy interpreterSetting from interpreterSettingTemplates when zeppelin is 
started the first time (interpreter.json doesn't exist)

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

### Todos
* [ ] - Task

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

### How should this be tested?
* Manually verified.  Delete an interpreter, and then restart zeppelin to check.

### 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 <zjf...@apache.org>

Closes #2648 from zjffdu/ZEPPELIN-3029 and squashes the following commits:

255e607 [Jeff Zhang] Address comments
4865aba [Jeff Zhang] ZEPPELIN-3029. Cannot delete an interpreter


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

Branch: refs/heads/master
Commit: 23c5cac8245aea16bfc0568cf2e5c2ae6dfd0e6d
Parents: 0f81b6d
Author: Jeff Zhang <zjf...@apache.org>
Authored: Wed Nov 8 19:39:16 2017 +0800
Committer: Jeff Zhang <zjf...@apache.org>
Committed: Fri Nov 10 13:53:09 2017 +0800

----------------------------------------------------------------------
 .../interpreter/ClassloaderInterpreter.java     | 261 -------------------
 .../interpreter/InterpreterSetting.java         |  22 +-
 .../interpreter/InterpreterSettingManager.java  |  50 ++--
 .../zeppelin/interpreter/MiniHadoopCluster.java |   1 +
 .../src/test/resources/conf/interpreter.json    |  54 ++++
 5 files changed, 91 insertions(+), 297 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/ClassloaderInterpreter.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/ClassloaderInterpreter.java
 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/ClassloaderInterpreter.java
deleted file mode 100644
index f8afa45..0000000
--- 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/ClassloaderInterpreter.java
+++ /dev/null
@@ -1,261 +0,0 @@
-/*
- * 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;
-
-import java.net.URL;
-import java.util.List;
-import java.util.Properties;
-
-import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
-import org.apache.zeppelin.scheduler.Scheduler;
-
-/**
- * Add to the classpath interpreters.
- *
- */
-public class ClassloaderInterpreter
-    extends Interpreter
-    implements WrappedInterpreter {
-
-  private ClassLoader cl;
-  private Interpreter intp;
-
-  public ClassloaderInterpreter(Interpreter intp, ClassLoader cl) {
-    super(new Properties());
-    this.cl = cl;
-    this.intp = intp;
-  }
-
-  @Override
-  public Interpreter getInnerInterpreter() {
-    return intp;
-  }
-
-  public ClassLoader getClassloader() {
-    return cl;
-  }
-
-  @Override
-  public InterpreterResult interpret(String st, InterpreterContext context)
-      throws InterpreterException {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.interpret(st, context);
-    } catch (InterpreterException e) {
-      throw e;
-    } catch (Exception e) {
-      throw new InterpreterException(e);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-
-  @Override
-  public void open() throws InterpreterException {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      intp.open();
-    } catch (Exception e) {
-      throw new InterpreterException(e);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public void close() throws InterpreterException {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      intp.close();
-    } catch (Exception e) {
-      throw new InterpreterException(e);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public void cancel(InterpreterContext context) throws InterpreterException {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      intp.cancel(context);
-    } catch (Exception e) {
-      throw new InterpreterException(e);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public FormType getFormType() throws InterpreterException {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.getFormType();
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public int getProgress(InterpreterContext context) throws 
InterpreterException {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.getProgress(context);
-    } catch (Exception e) {
-      throw new InterpreterException(e);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public Scheduler getScheduler() {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.getScheduler();
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public List<InterpreterCompletion> completion(String buf, int cursor,
-      InterpreterContext interpreterContext) throws InterpreterException {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      List completion = intp.completion(buf, cursor, interpreterContext);
-      return completion;
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-
-  @Override
-  public String getClassName() {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.getClassName();
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public void setInterpreterGroup(InterpreterGroup interpreterGroup) {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      intp.setInterpreterGroup(interpreterGroup);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public InterpreterGroup getInterpreterGroup() {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.getInterpreterGroup();
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public void setClassloaderUrls(URL [] urls) {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      intp.setClassloaderUrls(urls);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public URL [] getClassloaderUrls() {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.getClassloaderUrls();
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public void setProperties(Properties properties) {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      intp.setProperties(properties);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public Properties getProperties() {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.getProperties();
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-
-  @Override
-  public String getProperty(String key) {
-    ClassLoader oldcl = Thread.currentThread().getContextClassLoader();
-    Thread.currentThread().setContextClassLoader(cl);
-    try {
-      return intp.getProperty(key);
-    } finally {
-      cl = Thread.currentThread().getContextClassLoader();
-      Thread.currentThread().setContextClassLoader(oldcl);
-    }
-  }
-}

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
index 0596cc5..944672c 100644
--- 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
+++ 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
@@ -309,30 +309,37 @@ public class InterpreterSetting {
     return interpreterSettingManager;
   }
 
-  public void setAngularObjectRegistryListener(AngularObjectRegistryListener
+  public InterpreterSetting 
setAngularObjectRegistryListener(AngularObjectRegistryListener
                                                    
angularObjectRegistryListener) {
     this.angularObjectRegistryListener = angularObjectRegistryListener;
+    return this;
   }
 
-  public void setAppEventListener(ApplicationEventListener appEventListener) {
+  public InterpreterSetting setAppEventListener(ApplicationEventListener 
appEventListener) {
     this.appEventListener = appEventListener;
+    return this;
   }
 
-  public void 
setRemoteInterpreterProcessListener(RemoteInterpreterProcessListener
+  public InterpreterSetting 
setRemoteInterpreterProcessListener(RemoteInterpreterProcessListener
                                                       
remoteInterpreterProcessListener) {
     this.remoteInterpreterProcessListener = remoteInterpreterProcessListener;
+    return this;
   }
 
-  public void setDependencyResolver(DependencyResolver dependencyResolver) {
+  public InterpreterSetting setDependencyResolver(DependencyResolver 
dependencyResolver) {
     this.dependencyResolver = dependencyResolver;
+    return this;
   }
 
-  public void setInterpreterSettingManager(InterpreterSettingManager 
interpreterSettingManager) {
+  public InterpreterSetting setInterpreterSettingManager(
+      InterpreterSettingManager interpreterSettingManager) {
     this.interpreterSettingManager = interpreterSettingManager;
+    return this;
   }
 
-  public void setLifecycleManager(LifecycleManager lifecycleManager) {
+  public InterpreterSetting setLifecycleManager(LifecycleManager 
lifecycleManager) {
     this.lifecycleManager = lifecycleManager;
+    return this;
   }
 
   public LifecycleManager getLifecycleManager() {
@@ -511,8 +518,9 @@ public class InterpreterSetting {
     return conf;
   }
 
-  public void setConf(ZeppelinConfiguration conf) {
+  public InterpreterSetting setConf(ZeppelinConfiguration conf) {
     this.conf = conf;
+    return this;
   }
 
   public List<Dependency> getDependencies() {

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/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 fb86954..0b7efd5 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
@@ -165,6 +165,18 @@ public class InterpreterSettingManager {
     init();
   }
 
+
+  private void initInterpreterSetting(InterpreterSetting interpreterSetting) {
+    interpreterSetting.setConf(conf)
+        .setInterpreterSettingManager(this)
+        .setAngularObjectRegistryListener(angularObjectRegistryListener)
+        .setRemoteInterpreterProcessListener(remoteInterpreterProcessListener)
+        .setAppEventListener(appEventListener)
+        .setDependencyResolver(dependencyResolver)
+        .setLifecycleManager(lifecycleManager)
+        .postProcessing();
+  }
+
   /**
    * Load interpreter setting from interpreter-setting.json
    */
@@ -172,6 +184,11 @@ public class InterpreterSettingManager {
     if (!Files.exists(interpreterSettingPath)) {
       // nothing to read
       LOGGER.warn("Interpreter Setting file {} doesn't exist", 
interpreterSettingPath);
+      for (InterpreterSetting interpreterSettingTemplate : 
interpreterSettingTemplates.values()) {
+        InterpreterSetting interpreterSetting = new 
InterpreterSetting(interpreterSettingTemplate);
+        initInterpreterSetting(interpreterSetting);
+        interpreterSettings.put(interpreterSetting.getId(), 
interpreterSetting);
+      }
       return;
     }
 
@@ -179,17 +196,10 @@ public class InterpreterSettingManager {
       InterpreterInfoSaving infoSaving = 
InterpreterInfoSaving.loadFromFile(interpreterSettingPath);
       //TODO(zjffdu) still ugly (should move all to InterpreterInfoSaving)
       for (InterpreterSetting savedInterpreterSetting : 
infoSaving.interpreterSettings.values()) {
-        savedInterpreterSetting.setConf(conf);
-        savedInterpreterSetting.setInterpreterSettingManager(this);
-        
savedInterpreterSetting.setAngularObjectRegistryListener(angularObjectRegistryListener);
-        savedInterpreterSetting.setRemoteInterpreterProcessListener(
-            remoteInterpreterProcessListener);
-        savedInterpreterSetting.setAppEventListener(appEventListener);
-        savedInterpreterSetting.setDependencyResolver(dependencyResolver);
-        savedInterpreterSetting.setLifecycleManager(lifecycleManager);
         
savedInterpreterSetting.setProperties(InterpreterSetting.convertInterpreterProperties(
             savedInterpreterSetting.getProperties()
         ));
+        initInterpreterSetting(savedInterpreterSetting);
 
         InterpreterSetting interpreterSettingTemplate =
             
interpreterSettingTemplates.get(savedInterpreterSetting.getGroup());
@@ -377,14 +387,7 @@ public class InterpreterSettingManager {
         interpreterSettingTemplate);
 
     InterpreterSetting interpreterSetting = new 
InterpreterSetting(interpreterSettingTemplate);
-    
interpreterSetting.setAngularObjectRegistryListener(angularObjectRegistryListener);
-    
interpreterSetting.setRemoteInterpreterProcessListener(remoteInterpreterProcessListener);
-    interpreterSetting.setAppEventListener(appEventListener);
-    interpreterSetting.setDependencyResolver(dependencyResolver);
-    interpreterSetting.setInterpreterSettingManager(this);
-    interpreterSetting.setLifecycleManager(lifecycleManager);
-    interpreterSetting.postProcessing();
-    interpreterSettings.put(interpreterSetting.getId(), interpreterSetting);
+    initInterpreterSetting(interpreterSetting);
   }
 
   @VisibleForTesting
@@ -640,13 +643,7 @@ public class InterpreterSettingManager {
     setting.appendDependencies(dependencies);
     setting.setInterpreterOption(option);
     setting.setProperties(p);
-    setting.setAppEventListener(appEventListener);
-    
setting.setRemoteInterpreterProcessListener(remoteInterpreterProcessListener);
-    setting.setDependencyResolver(dependencyResolver);
-    setting.setAngularObjectRegistryListener(angularObjectRegistryListener);
-    setting.setLifecycleManager(lifecycleManager);
-    setting.setInterpreterSettingManager(this);
-    setting.postProcessing();
+    initInterpreterSetting(setting);
     interpreterSettings.put(setting.getId(), setting);
     saveToFile();
     return setting;
@@ -655,12 +652,7 @@ public class InterpreterSettingManager {
   @VisibleForTesting
   public void addInterpreterSetting(InterpreterSetting interpreterSetting) {
     interpreterSettingTemplates.put(interpreterSetting.getName(), 
interpreterSetting);
-    interpreterSetting.setAppEventListener(appEventListener);
-    interpreterSetting.setDependencyResolver(dependencyResolver);
-    interpreterSetting.setLifecycleManager(lifecycleManager);
-    
interpreterSetting.setAngularObjectRegistryListener(angularObjectRegistryListener);
-    
interpreterSetting.setRemoteInterpreterProcessListener(remoteInterpreterProcessListener);
-    interpreterSetting.setInterpreterSettingManager(this);
+    initInterpreterSetting(interpreterSetting);
     interpreterSettings.put(interpreterSetting.getId(), interpreterSetting);
   }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java
 
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java
index 619d01a..b0799ae 100644
--- 
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java
+++ 
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java
@@ -44,6 +44,7 @@ public class MiniHadoopCluster {
 
     // start MiniYarnCluster
     YarnConfiguration baseConfig = new YarnConfiguration(hadoopConf);
+    
baseConfig.set("yarn.nodemanager.disk-health-checker.max-disk-utilization-per-disk-percentage",
 "95");
     this.yarnCluster = new MiniYARNCluster(getClass().getName(), 2,
         1, 1);
     yarnCluster.init(baseConfig);

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-zengine/src/test/resources/conf/interpreter.json
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/resources/conf/interpreter.json 
b/zeppelin-zengine/src/test/resources/conf/interpreter.json
index 45e1d60..0c5e829 100644
--- a/zeppelin-zengine/src/test/resources/conf/interpreter.json
+++ b/zeppelin-zengine/src/test/resources/conf/interpreter.json
@@ -66,6 +66,60 @@
         "users": [],
         "isUserImpersonate": false
       }
+    },
+
+    "2C4BJDRRZ" : {
+      "group": "mock1",
+      "name": "mock1",
+      "className": "org.apache.zeppelin.interpreter.mock.MockInterpreter1",
+      "properties": {
+      },
+      "option": {
+        "remote": true,
+        "port": -1,
+        "perNote": "shared",
+        "perUser": "shared",
+        "isExistingProcess": false,
+        "setPermission": false,
+        "users": [],
+        "isUserImpersonate": false
+      }
+    },
+
+    "2C3PTPMUH" : {
+      "group": "mock2",
+      "name": "mock2",
+      "className": "org.apache.zeppelin.interpreter.mock.MockInterpreter2",
+      "properties": {
+      },
+      "option": {
+        "remote": true,
+        "port": -1,
+        "perNote": "shared",
+        "perUser": "isolated",
+        "isExistingProcess": false,
+        "setPermission": false,
+        "users": [],
+        "isUserImpersonate": false
+      }
+    },
+
+    "2C5DCRVGM" : {
+      "group": "mock_resource_pool",
+      "name": "mock_resource_pool",
+      "className": 
"org.apache.zeppelin.interpreter.remote.mock.MockInterpreterResourcePool",
+      "properties": {
+      },
+      "option": {
+        "remote": true,
+        "port": -1,
+        "perNote": "shared",
+        "perUser": "shared",
+        "isExistingProcess": false,
+        "setPermission": false,
+        "users": [],
+        "isUserImpersonate": false
+      }
     }
   },
   "interpreterBindings": {

Reply via email to