zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jongy...@apache.org
Subject zeppelin git commit: [HOTFIX] Handle removing interpreters while removing note
Date Mon, 20 Mar 2017 02:45:23 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/master 4a9377db9 -> 632815c35


[HOTFIX] Handle removing interpreters while removing note

### What is this PR for?
Removing some interpreters while removing note. In case of "Per note", "Isolated" interpreter
process should be removed if a note be removed. In case of "Scoped" of "Per note", that session
should be removed and if there's no more session in that process, the process should be removed,
too.

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

### Todos
* [x] - Handle removing interpreters when removing note

### What is the Jira issue?
N/A

### How should this be tested?
1. set python interpreter as "per note/scoped"
1. run ```%python print(1)``` in a note
1. remove that note
1. check if python process exists with grep

### 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: Jongyoul Lee <jongyoul@gmail.com>

Closes #2159 from jongyoul/hotfix/remove-note-remove-interpreter-instances and squashes the
following commits:

cb3de97 [Jongyoul Lee] Added method to handle removing interpreters while removing note


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

Branch: refs/heads/master
Commit: 632815c3582a5e91e9fdc8ee3c0473a0d87c268e
Parents: 4a9377d
Author: Jongyoul Lee <jongyoul@gmail.com>
Authored: Mon Mar 20 00:33:50 2017 +0900
Committer: Jongyoul Lee <jongyoul@apache.org>
Committed: Mon Mar 20 11:45:19 2017 +0900

----------------------------------------------------------------------
 .../apache/zeppelin/socket/NotebookServer.java  |   1 +
 .../interpreter/InterpreterSettingManager.java  |  15 +--
 .../org/apache/zeppelin/notebook/Notebook.java  |   7 ++
 .../interpreter/InterpreterSettingTest.java     | 120 +++++++++++++++++++
 4 files changed, 129 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/632815c3/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
index 1a6f0be..23d80a4 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
@@ -1082,6 +1082,7 @@ public class NotebookServer extends WebSocketServlet
     if (note != null && !note.isTrash()){
       fromMessage.put("name", Folder.TRASH_FOLDER_ID + "/" + note.getName());
       renameNote(conn, userAndRoles, notebook, fromMessage, "move");
+      notebook.moveNoteToTrash(note.getId());
     }
   }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/632815c3/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 585456f..1832564 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
@@ -799,20 +799,7 @@ public class InterpreterSettingManager {
 
   public void removeInterpretersForNote(InterpreterSetting interpreterSetting, String user,
       String noteId) {
-    InterpreterOption option = interpreterSetting.getOption();
-    if (option.isProcess()) {
-      interpreterSetting.closeAndRemoveInterpreterGroupByNoteId(noteId);
-    } else if (option.isSession()) {
-      InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup(user, noteId);
-      String key = getInterpreterSessionKey(user, noteId, interpreterSetting);
-      interpreterGroup.close(key);
-      synchronized (interpreterGroup) {
-        interpreterGroup.remove(key);
-        interpreterGroup.notifyAll(); // notify createInterpreterForNote()
-      }
-      logger.info("Interpreter instance {} for note {} is removed", interpreterSetting.getName(),
-          noteId);
-    }
+    interpreterSetting.closeAndRemoveInterpreterGroup(noteId, "");
   }
 
   public String getInterpreterSessionKey(String user, String noteId, InterpreterSetting setting)
{

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/632815c3/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
index dfef6f4..e7f7e49 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
@@ -322,6 +322,13 @@ public class Notebook implements NoteEventListener {
     }
   }
 
+  public void moveNoteToTrash(String noteId) {
+    for (InterpreterSetting interpreterSetting : interpreterSettingManager
+        .getInterpreterSettings(noteId)) {
+      interpreterSettingManager.removeInterpretersForNote(interpreterSetting, "", noteId);
+    }
+  }
+
   public void removeNote(String id, AuthenticationInfo subject) {
     Preconditions.checkNotNull(subject, "AuthenticationInfo should not be null");
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/632815c3/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java
index 0008751..1aab757 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java
@@ -204,4 +204,124 @@ public class InterpreterSettingTest {
     interpreterSetting.closeAndRemoveInterpreterGroup("note2", "user1");
     assertEquals(0, interpreterSetting.getAllInterpreterGroups().size());
   }
+
+  @Test
+  public void perNoteScopedModeRemoveInterpreterGroupWhenNoteIsRemoved() {
+    InterpreterOption interpreterOption = new InterpreterOption();
+    interpreterOption.setPerNote(InterpreterOption.SCOPED);
+    InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList<InterpreterInfo>(),
new Properties(), new ArrayList<Dependency>(), interpreterOption, "", null);
+
+    interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() {
+      @Override
+      public InterpreterGroup createInterpreterGroup(String interpreterGroupId,
+          InterpreterOption option) {
+        return new InterpreterGroup(interpreterGroupId);
+      }
+    });
+
+    Interpreter mockInterpreter1 = mock(RemoteInterpreter.class);
+    List<Interpreter> interpreterList1 = new ArrayList<>();
+    interpreterList1.add(mockInterpreter1);
+    InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1");
+    interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1);
+
+    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+    assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size());
+
+    // This method will be called when remove note
+    interpreterSetting.closeAndRemoveInterpreterGroup("note1","");
+    assertEquals(0, interpreterSetting.getAllInterpreterGroups().size());
+    // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist
+    assertEquals(0, interpreterSetting.getInterpreterGroup("user1","note1").size());
+  }
+
+  @Test
+  public void perNoteIsolatedModeRemoveInterpreterGroupWhenNoteIsRemoved() {
+    InterpreterOption interpreterOption = new InterpreterOption();
+    interpreterOption.setPerNote(InterpreterOption.ISOLATED);
+    InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList<InterpreterInfo>(),
new Properties(), new ArrayList<Dependency>(), interpreterOption, "", null);
+
+    interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() {
+      @Override
+      public InterpreterGroup createInterpreterGroup(String interpreterGroupId,
+          InterpreterOption option) {
+        return new InterpreterGroup(interpreterGroupId);
+      }
+    });
+
+    Interpreter mockInterpreter1 = mock(RemoteInterpreter.class);
+    List<Interpreter> interpreterList1 = new ArrayList<>();
+    interpreterList1.add(mockInterpreter1);
+    InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1");
+    interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1);
+
+    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+    assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size());
+
+    // This method will be called when remove note
+    interpreterSetting.closeAndRemoveInterpreterGroup("note1","");
+    assertEquals(0, interpreterSetting.getAllInterpreterGroups().size());
+    // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist
+    assertEquals(0, interpreterSetting.getInterpreterGroup("user1","note1").size());
+  }
+
+  @Test
+  public void perUserScopedModeNeverRemoveInterpreterGroupWhenNoteIsRemoved() {
+    InterpreterOption interpreterOption = new InterpreterOption();
+    interpreterOption.setPerUser(InterpreterOption.SCOPED);
+    InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList<InterpreterInfo>(),
new Properties(), new ArrayList<Dependency>(), interpreterOption, "", null);
+
+    interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() {
+      @Override
+      public InterpreterGroup createInterpreterGroup(String interpreterGroupId,
+          InterpreterOption option) {
+        return new InterpreterGroup(interpreterGroupId);
+      }
+    });
+
+    Interpreter mockInterpreter1 = mock(RemoteInterpreter.class);
+    List<Interpreter> interpreterList1 = new ArrayList<>();
+    interpreterList1.add(mockInterpreter1);
+    InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1");
+    interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1);
+
+    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+    assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size());
+
+    // This method will be called when remove note
+    interpreterSetting.closeAndRemoveInterpreterGroup("note1","");
+    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+    // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist
+    assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note1").size());
+  }
+
+  @Test
+  public void perUserIsolatedModeNeverRemoveInterpreterGroupWhenNoteIsRemoved() {
+    InterpreterOption interpreterOption = new InterpreterOption();
+    interpreterOption.setPerUser(InterpreterOption.ISOLATED);
+    InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList<InterpreterInfo>(),
new Properties(), new ArrayList<Dependency>(), interpreterOption, "", null);
+
+    interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() {
+      @Override
+      public InterpreterGroup createInterpreterGroup(String interpreterGroupId,
+          InterpreterOption option) {
+        return new InterpreterGroup(interpreterGroupId);
+      }
+    });
+
+    Interpreter mockInterpreter1 = mock(RemoteInterpreter.class);
+    List<Interpreter> interpreterList1 = new ArrayList<>();
+    interpreterList1.add(mockInterpreter1);
+    InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1");
+    interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1);
+
+    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+    assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size());
+
+    // This method will be called when remove note
+    interpreterSetting.closeAndRemoveInterpreterGroup("note1","");
+    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+    // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist
+    assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note1").size());
+  }
 }


Mime
View raw message