Reamer commented on a change in pull request #4252:
URL: https://github.com/apache/zeppelin/pull/4252#discussion_r780078820
##########
File path:
zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
##########
@@ -352,43 +355,64 @@ public void removeCorruptedNote(String noteId,
AuthenticationInfo subject) throw
authorizationService.removeNoteAuth(noteId);
}
+ /**
+ * Removes the Note with the NoteId.
+ * @param noteId
+ * @param subject
+ * @throws IOException when fail to get it from NotebookRepo
+ */
public void removeNote(String noteId, AuthenticationInfo subject) throws
IOException {
- Note note = getNote(noteId);
- removeNote(note, subject);
+ processNote(noteId,
+ note -> {
+ if (note != null) {
+ removeNote(note, subject);
+ }
+ return null;
+ });
}
/**
- * Get note from NotebookRepo and also initialize it with other properties
that is not
- * persistent in NotebookRepo, such as paragraphJobListener.
+ * Process a note in an eviction aware manner. It initializes the note
+ * with other properties that is not persistent in NotebookRepo, such as
paragraphJobListener.
+ * <p>
+ * Use {@link #processNote(String, boolean, NoteProcessor)} in case you want
to force
+ * a note reload from the {@link #NotebookRepo}.
+ * </p>
* @param noteId
- * @return null if note not found.
- * @throws IOException when fail to get it from NotebookRepo.
+ * @param noteProcessor
+ * @return result of the noteProcessor
+ * @throws IOException when fail to get it from {@link NotebookRepo}
*/
- public Note getNote(String noteId) throws IOException {
- return getNote(noteId, false);
+ public <T> T processNote(String noteId, NoteProcessor<T> noteProcessor)
throws IOException {
+ return processNote(noteId, false, noteProcessor);
}
/**
- * Get note from NotebookRepo and also initialize it with other properties
that is not
- * persistent in NotebookRepo, such as paragraphJobListener.
+ * Process a note in an eviction aware manner. It initializes the note
+ * with other properties that is not persistent in NotebookRepo, such as
paragraphJobListener.
+ *
* @param noteId
- * @return null if note not found.
- * @throws IOException when fail to get it from NotebookRepo.
+ * @param reload
+ * @param noteProcessor
+ * @return result of the noteProcessor
+ * @throws IOException when fail to get it from {@link NotebookRepo}
*/
- public Note getNote(String noteId, boolean reload) throws IOException {
- Note note = noteManager.getNote(noteId, reload);
- if (note == null) {
- return null;
- }
- note.setInterpreterFactory(replFactory);
- note.setInterpreterSettingManager(interpreterSettingManager);
- note.setParagraphJobListener(paragraphJobListener);
- note.setNoteEventListeners(noteEventListeners);
- note.setCredentials(credentials);
- for (Paragraph p : note.getParagraphs()) {
- p.setNote(note);
- }
- return note;
+ public <T> T processNote(String noteId, boolean reload, NoteProcessor<T>
noteProcessor) throws IOException {
+ return noteManager.processNote(noteId, reload, note -> {
+ if (note == null) {
+ return noteProcessor.process(note);
Review comment:
Yes, for several reasons.
To return a correct response as in this
[case](https://github.com/apache/zeppelin/pull/4252/files#diff-017bf51cce0cfa2d227c2a93230a2164bbad6b98de581a54dc4a778b49704497R128).
```java
if (note == null) {
return new JsonResponse<>(Response.Status.NOT_FOUND, "Note " + noteId + "
not found").build();
}
```
Or to work on a callback as in this
[case](https://github.com/apache/zeppelin/pull/4252/files#diff-94da03110f043520a11e2697bd4b6749134c14214ec240b65a61f6b0ed1f0005R66-R77)
```java
return notebook.processNote(noteId,
jobNote -> {
List<NoteJobInfo> notesJobInfo = new ArrayList<>();
if (jobNote == null) {
callback.onFailure(new IOException("Note " + noteId + " not
found"), context);
} else {
notesJobInfo.add(new NoteJobInfo(jobNote));
callback.onSuccess(notesJobInfo, context);
}
return notesJobInfo;
});
```
for logging as in this
[case](https://github.com/apache/zeppelin/pull/4252/files#diff-c8d59c9816d0b26045cd0cd44343eb10dbebd61730fadae84a9b147e3b6226f8R1829-R1831)
```java
getNotebook().processNote(noteId,
note -> {
if (note == null) {
// It is possible the note is removed, but the job is still
running
LOG.warn("Note {} doesn't existed, it maybe deleted.", noteId);
} else {
note.clearParagraphOutput(paragraphId);
Paragraph paragraph = note.getParagraph(paragraphId);
broadcastParagraph(note, paragraph, MSG_ID_NOT_DEFINED);
}
return null;
});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
