You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by cl...@apache.org on 2017/04/28 05:16:56 UTC

zeppelin git commit: [ZEPPELIN-2431] Corrected deletion of notes by incorrect interpreter.json

Repository: zeppelin
Updated Branches:
  refs/heads/master c8cd1cf50 -> 874403256


[ZEPPELIN-2431] Corrected deletion of notes by incorrect interpreter.json

## What is this PR for?

We sometimes can not delete a note, or we will be accompanied by an NPE for deleting a note.

This problem occurs when:
When interpreter.json 's note binding is wrong, or there is a problem.
If you are configuring an interpreter that is not through zeppelin's user interface.
As a result, it happens when synchronization of notes deletion and setting retention is not normal.
Therefore, we should add handling for note deletion and exception handling for nonexistent interpreter bindings.
It reduces the synchronization problem of interpreter.json.

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

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

### How should this be tested?
1. zeppelin stop
2. edit con/interpreter.json and `interpreterBindings`
  fix any notes or incorrect information.
  for example
  ```
    },
  "interpreterBindings": {
    "2CFS9YSM5": [
      "2CFRR1D3TINVALIDINVALIDINVALID", <-- edit
      "2CFZ1JKUR",
      "2CEAJK1VW",
      "2CGSESWBH",
      "2CERNPGW5",
   }
  ```
3. zeppelin start
4. You can try remove invalid interpreter bind note on web. (on example = `2CFS9YSM5`)

result :
If the modifications to this PR are not reflected,
It will not be deleted or an error will appear on the server.
Also, the interpreterBindings information in interpreter.json does not respond to delete events.

### Screenshots (if appropriate)
problem animation
![cantremove](https://cloud.githubusercontent.com/assets/10525473/25327785/7031f960-2910-11e7-88c7-d322da21290c.gif)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: CloverHearts <cl...@gmail.com>

Closes #2278 from cloverhearts/fix/invalidsyncInterpreterJson and squashes the following commits:

35da524c [CloverHearts] notebook interpreter binding synchronization process


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

Branch: refs/heads/master
Commit: 8744032563718dfe9aac0c9b968254fcdca7b004
Parents: c8cd1cf
Author: CloverHearts <cl...@gmail.com>
Authored: Mon Apr 24 16:16:47 2017 +0900
Committer: CloverHearts <cl...@gmail.com>
Committed: Fri Apr 28 14:16:44 2017 +0900

----------------------------------------------------------------------
 .../zeppelin/interpreter/InterpreterSettingManager.java      | 8 ++++++--
 .../src/main/java/org/apache/zeppelin/notebook/Notebook.java | 6 +++++-
 2 files changed, 11 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/87440325/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 bebbf35..1b05a76 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
@@ -887,14 +887,18 @@ public class InterpreterSettingManager {
     saveToFile();
   }
 
-  public void removeNoteInterpreterSettingBinding(String user, String noteId) {
+  public void removeNoteInterpreterSettingBinding(String user, String noteId) throws IOException {
     synchronized (interpreterSettings) {
       List<String> settingIds = (interpreterBindings.containsKey(noteId) ?
           interpreterBindings.remove(noteId) :
           Collections.<String>emptyList());
       for (String settingId : settingIds) {
-        this.removeInterpretersForNote(get(settingId), user, noteId);
+        InterpreterSetting setting = get(settingId);
+        if (setting != null) {
+          this.removeInterpretersForNote(setting, user, noteId);
+        }
       }
+      saveToFile();
     }
   }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/87440325/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 de48bef..720a3be 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
@@ -338,7 +338,11 @@ public class Notebook implements NoteEventListener {
       note = notes.remove(id);
       folders.removeNote(note);
     }
-    interpreterSettingManager.removeNoteInterpreterSettingBinding(subject.getUser(), id);
+    try {
+      interpreterSettingManager.removeNoteInterpreterSettingBinding(subject.getUser(), id);
+    } catch (IOException e) {
+      logger.error(e.toString(), e);
+    }
     noteSearchService.deleteIndexDocs(note);
     notebookAuthorization.removeNote(id);