You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2020/10/18 14:42:08 UTC
[zeppelin] branch master updated: [ZEPPELIN-5077] Run all
paragraphs does not stop on failing paragraph
This is an automated email from the ASF dual-hosted git repository.
zjffdu 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 1197227 [ZEPPELIN-5077] Run all paragraphs does not stop on failing paragraph
1197227 is described below
commit 1197227154cb456154da78cbf60d95405285a8a7
Author: Timo Olkkonen <ti...@gmail.com>
AuthorDate: Fri Oct 16 10:24:02 2020 +0300
[ZEPPELIN-5077] Run all paragraphs does not stop on failing paragraph
### What is this PR for?
If a paragraph fails on something other than exception in Zeppelin, notebook will continue running paragraphs after it. This is unintuitive and regression from previous version.
To fix it, return "false" from running a paragraph, if it the actual user code fails. Also, after a failing "run all" operation, we need to broadcast paragraph states to client, so they wont stay in pending state after a failed run.
### What type of PR is it?
Bug Fix
### Todos
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-5077
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
Author: Timo Olkkonen <ti...@gmail.com>
Closes #3933 from olkkoti/ZEPPELIN-5077 and squashes the following commits:
5c0f9be59 [Timo Olkkonen] Stop runAllParagraphs if paragraph result code is ERROR.
aa4d0797b [Timo Olkkonen] Add comment on why we need to broadcast paragraph states after failed runall.
b9ae304ff [Timo Olkkonen] Add a test to see that paragraph after a failing one is not run in runAll.
5e5a595bc [Timo Olkkonen] If running all paragraphs fails, broadcast paragraphs states to client.
---
.../zeppelin/integration/ParagraphActionsIT.java | 41 ++++++++++++++++++++++
.../apache/zeppelin/service/NotebookService.java | 8 ++++-
.../org/apache/zeppelin/socket/NotebookServer.java | 13 +++++--
3 files changed, 59 insertions(+), 3 deletions(-)
diff --git a/zeppelin-integration/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java b/zeppelin-integration/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java
index a3e08b1..b7e6cd2 100644
--- a/zeppelin-integration/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java
+++ b/zeppelin-integration/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java
@@ -238,6 +238,47 @@ public class ParagraphActionsIT extends AbstractZeppelinIT {
}
}
+ @Test
+ public void testRunAllUserCodeFail() throws Exception {
+ try {
+ createNewNote();
+ waitForParagraph(1, "READY");
+ setTextOfParagraph(1, "syntax error");
+ driver.findElement(By.xpath(getParagraphXPath(1) + "//span[@class='icon-settings']")).click();
+ driver.findElement(By.xpath(getParagraphXPath(1) + "//ul/li/a[@ng-click=\"insertNew('below')\"]"))
+ .click();
+ waitForParagraph(2, "READY");
+ setTextOfParagraph(2, "println (\"abcd\")");
+
+
+ driver.findElement(By.xpath(".//*[@id='main']//button[contains(@ng-click, 'runAllParagraphs')]")).sendKeys(Keys.ENTER);
+ ZeppelinITUtils.sleep(1000, false);
+ driver.findElement(By.xpath("//div[@class='modal-dialog'][contains(.,'Run all paragraphs?')]" +
+ "//div[@class='modal-footer']//button[contains(.,'OK')]")).click();
+ ZeppelinITUtils.sleep(2000, false);
+
+
+ collector.checkThat("First paragraph status is ",
+ getParagraphStatus(1), CoreMatchers.equalTo("ERROR")
+ );
+ collector.checkThat("Second paragraph status is ",
+ getParagraphStatus(2), CoreMatchers.equalTo("READY")
+ );
+
+ String xpathToOutputField = getParagraphXPath(2) + "//div[contains(@id,\"_text\")]";
+ collector.checkThat("Second paragraph output is ",
+ driver.findElements(By.xpath(xpathToOutputField)).size(),
+ CoreMatchers.equalTo(0));
+
+
+ driver.navigate().refresh();
+ ZeppelinITUtils.sleep(3000, false);
+ deleteTestNotebook(driver);
+ } catch (Exception e) {
+ handleException("Exception in ParagraphActionsIT while testRunAllUserCodeFail ", e);
+ }
+ }
+
// @Test
public void testRunOnSelectionChange() throws Exception {
try {
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
index 2bc3c46..050b846 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
@@ -20,6 +20,7 @@ package org.apache.zeppelin.service;
import static org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_HOMESCREEN;
+import static org.apache.zeppelin.interpreter.InterpreterResult.Code.ERROR;
import com.google.common.base.Strings;
import java.io.IOException;
@@ -49,7 +50,6 @@ import org.apache.zeppelin.notebook.AuthorizationService;
import org.apache.zeppelin.notebook.repo.NotebookRepoWithVersionControl;
import org.apache.zeppelin.notebook.scheduler.SchedulerService;
import org.apache.zeppelin.common.Message;
-import org.apache.zeppelin.rest.SessionManager;
import org.apache.zeppelin.rest.exception.BadRequestException;
import org.apache.zeppelin.rest.exception.ForbiddenException;
import org.apache.zeppelin.rest.exception.NoteNotFoundException;
@@ -426,6 +426,12 @@ public class NotebookService {
// stop execution when one paragraph fails.
return false;
}
+ // also stop execution when user code in a paragraph fails
+ Paragraph p = note.getParagraph(paragraphId);
+ InterpreterResult result = p.getReturn();
+ if (result.code() == ERROR) {
+ return false;
+ }
} catch (Exception e) {
throw new IOException("Fail to run paragraph json: " + raw);
}
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 6c8e039..45e1abe 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
@@ -1488,8 +1488,17 @@ public class NotebookServer extends WebSocketServlet
new TypeToken<List<Map<String, Object>>>() {
}.getType());
- getNotebookService().runAllParagraphs(noteId, paragraphs, getServiceContext(fromMessage),
- new WebSocketServiceCallback<Paragraph>(conn));
+ if (!getNotebookService().runAllParagraphs(noteId, paragraphs, getServiceContext(fromMessage),
+ new WebSocketServiceCallback<Paragraph>(conn))) {
+ // If one paragraph fails, we need to broadcast paragraph states to the client,
+ // or paragraphs not run will stay in PENDING state.
+ Note note = getNotebookService().getNote(noteId, getServiceContext(fromMessage), new SimpleServiceCallback());
+ if (note != null) {
+ for (Paragraph p : note.getParagraphs()) {
+ broadcastParagraph(note, p, null);
+ }
+ }
+ }
}
private void broadcastSpellExecution(NotebookSocket conn,