This is an automated email from the ASF dual-hosted git repository.
pdallig pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new ab4fdb7 [ZEPPELIN-5188] /note1 race condition CI
ab4fdb7 is described below
commit ab4fdb7247dce413cb33a4bb147d6f1b0874d837
Author: Philipp Dallig <[email protected]>
AuthorDate: Tue Jan 5 11:10:45 2021 +0100
[ZEPPELIN-5188] /note1 race condition CI
### What is this PR for?
This PR should fix a race condition between removing and saving notes in
our CI setup
### What type of PR is it?
- Bug Fix
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-5188
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Author: Philipp Dallig <[email protected]>
Closes #4014 from Reamer/note_1_race_condition and squashes the following
commits:
0b1bc294f [Philipp Dallig] Do not save note, when is was removed
4c35421f8 [Philipp Dallig] Cleanup more code
---
.../java/org/apache/zeppelin/socket/NotebookServer.java | 2 +-
.../org/apache/zeppelin/rest/NotebookRestApiTest.java | 7 +++----
.../src/main/java/org/apache/zeppelin/notebook/Note.java | 15 ++++++++++++---
.../java/org/apache/zeppelin/notebook/NoteManager.java | 8 +++++---
.../main/java/org/apache/zeppelin/notebook/Notebook.java | 2 ++
5 files changed, 23 insertions(+), 11 deletions(-)
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 e1146db..224cbc5 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
@@ -1685,7 +1685,7 @@ public class NotebookServer extends WebSocketServlet
try {
Note note = getNotebook().getNote(noteId);
if (note == null) {
- LOG.warn("Note " + noteId + " note found");
+ LOG.warn("Note {} not found", noteId);
return;
}
Paragraph paragraph = note.getParagraph(paragraphId);
diff --git
a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
index 2b31ee5..214ba7b 100644
---
a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
+++
b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
@@ -42,7 +42,6 @@ import org.junit.runners.MethodSorters;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
-import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -136,7 +135,7 @@ public class NotebookRestApiTest extends
AbstractTestRestApi {
assertEquals("OK", resp.get("status"));
post.close();
p.waitUntilFinished();
- assertNotEquals(p.getStatus(), Job.Status.FINISHED);
+ assertNotEquals(Job.Status.FINISHED, p.getStatus());
} finally {
// cleanup
if (null != note1) {
@@ -167,7 +166,7 @@ public class NotebookRestApiTest extends
AbstractTestRestApi {
new TypeToken<Map<String, Object>>() {}.getType());
assertEquals("OK", resp.get("status"));
post.close();
- assertNotEquals(p.getStatus(), Job.Status.READY);
+ assertNotEquals(Job.Status.READY, p.getStatus());
// Check if the paragraph is emptied
assertEquals(title, p.getTitle());
@@ -189,7 +188,7 @@ public class NotebookRestApiTest extends
AbstractTestRestApi {
assertTrue(interpreterResults.get(0).toString(),
interpreterResults.get(0).get("data").toString().contains("invalid_cmd: command
not found"));
post.close();
- assertNotEquals(p.getStatus(), Job.Status.READY);
+ assertNotEquals(Job.Status.READY, p.getStatus());
// Check if the paragraph is emptied
assertEquals(title, p.getTitle());
diff --git
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
index 26c98d2..10c9422 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
@@ -151,6 +151,7 @@ public class Note implements JsonSerializable {
/********************************** transient fields
******************************************/
private transient boolean loaded = false;
private transient boolean saved = false;
+ private transient boolean removed = false;
private transient InterpreterFactory interpreterFactory;
private transient InterpreterSettingManager interpreterSettingManager;
private transient ParagraphJobListener paragraphJobListener;
@@ -188,7 +189,7 @@ public class Note implements JsonSerializable {
}
public String getParentPath() {
- int pos = path.lastIndexOf("/");
+ int pos = path.lastIndexOf('/');
if (pos == 0) {
return "/";
} else {
@@ -197,7 +198,7 @@ public class Note implements JsonSerializable {
}
private String getName(String path) {
- int pos = path.lastIndexOf("/");
+ int pos = path.lastIndexOf('/');
return path.substring(pos + 1);
}
@@ -329,7 +330,7 @@ public class Note implements JsonSerializable {
this.path = "/" + name;
}
} else {
- int pos = this.path.lastIndexOf("/");
+ int pos = this.path.lastIndexOf('/');
this.path = this.path.substring(0, pos + 1) + this.name;
}
}
@@ -1228,4 +1229,12 @@ public class Note implements JsonSerializable {
public boolean isSaved() {
return saved;
}
+
+ public void setRemoved(boolean removed) {
+ this.removed = removed;
+ }
+
+ public boolean isRemoved() {
+ return removed;
+ }
}
diff --git
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
index 80eb37d..779d173 100644
---
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
+++
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
@@ -177,7 +177,9 @@ public class NoteManager {
* @throws IOException
*/
public void saveNote(Note note, AuthenticationInfo subject) throws
IOException {
- if (note.isLoaded() || !note.isSaved()) {
+ if (note.isRemoved()) {
+ LOGGER.warn("Try to save note: {} when it is removed", note.getId());
+ } else if (note.isLoaded() || !note.isSaved()) {
addOrUpdateNoteNode(note);
this.notebookRepo.save(note, subject);
note.setSaved(true);
@@ -367,12 +369,12 @@ public class NoteManager {
}
private String getFolderName(String notePath) {
- int pos = notePath.lastIndexOf("/");
+ int pos = notePath.lastIndexOf('/');
return notePath.substring(0, pos);
}
private String getNoteName(String notePath) {
- int pos = notePath.lastIndexOf("/");
+ int pos = notePath.lastIndexOf('/');
return notePath.substring(pos + 1);
}
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 94614bb..16170c1 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
@@ -332,6 +332,8 @@ public class Notebook {
public void removeNote(Note note, AuthenticationInfo subject) throws
IOException {
LOGGER.info("Remove note: {}", note.getId());
+ // Set Remove to true to cancel saving this note
+ note.setRemoved(true);
noteManager.removeNote(note.getId(), subject);
fireNoteRemoveEvent(note, subject);
}
