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/05/19 02:42:02 UTC

[zeppelin] branch branch-0.9 updated: [ZEPPELIN-4805]. Error Stack Trace not received on rest API

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch branch-0.9
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/branch-0.9 by this push:
     new a769966  [ZEPPELIN-4805]. Error Stack Trace not received on rest API
a769966 is described below

commit a769966fd03764383809285d2b916d64170e108b
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Fri May 8 15:01:52 2020 +0800

    [ZEPPELIN-4805]. Error Stack Trace not received on rest API
    
    ### What is this PR for?
    
     This PR is to fix the issue that no error message returned of running paragraph.
    
    ### What type of PR is it?
    [Bug Fix ]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-4805
    
    ### How should this be tested?
    * CI pass
    
    ### 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: Jeff Zhang <zj...@apache.org>
    
    Closes #3772 from zjffdu/ZEPPELIN-4805 and squashes the following commits:
    
    e8ed2f017 [Jeff Zhang] [ZEPPELIN-4805]. Error Stack Trace not received on rest API
    
    (cherry picked from commit 69722ff4baeab5b0ece76f9abf8fb14ae0980e5b)
    Signed-off-by: Jeff Zhang <zj...@apache.org>
---
 zeppelin-server/conf/notebook-authorization.json   | 16 +++++++++++++++
 .../org/apache/zeppelin/server/ZeppelinServer.java | 12 +++++++----
 .../apache/zeppelin/service/NotebookService.java   | 21 +++++++++++++++++--
 .../apache/zeppelin/rest/NotebookRestApiTest.java  | 24 ++++++++++++++++++++++
 .../zeppelin/service/NotebookServiceTest.java      |  2 +-
 .../java/org/apache/zeppelin/notebook/Note.java    |  2 +-
 .../org/apache/zeppelin/notebook/Paragraph.java    |  5 +++++
 7 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/zeppelin-server/conf/notebook-authorization.json b/zeppelin-server/conf/notebook-authorization.json
new file mode 100644
index 0000000..1e3767c
--- /dev/null
+++ b/zeppelin-server/conf/notebook-authorization.json
@@ -0,0 +1,16 @@
+{
+  "authInfo": {
+    "2F8GB7HN7": {
+      "readers": [],
+      "owners": [],
+      "writers": [],
+      "runners": []
+    },
+    "2F9W6X5GY": {
+      "readers": [],
+      "owners": [],
+      "writers": [],
+      "runners": []
+    }
+  }
+}
\ No newline at end of file
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
index fa8e6b8..d41ebb1 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
@@ -287,11 +287,15 @@ public class ZeppelinServer extends ResourceConfig {
             () -> {
               LOG.info("Shutting down Zeppelin Server ... ");
               try {
-                jettyWebServer.stop();
-                if (!conf.isRecoveryEnabled()) {
-                  sharedServiceLocator.getService(InterpreterSettingManager.class).close();
+                if (jettyWebServer != null) {
+                  jettyWebServer.stop();
+                }
+                if (sharedServiceLocator != null) {
+                  if (!conf.isRecoveryEnabled()) {
+                    sharedServiceLocator.getService(InterpreterSettingManager.class).close();
+                  }
+                  sharedServiceLocator.getService(Notebook.class).close();
                 }
-                sharedServiceLocator.getService(Notebook.class).close();
                 Thread.sleep(3000);
               } catch (Exception e) {
                 LOG.error("Error while stopping servlet container", e);
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 9901586..bc7788b 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
@@ -279,6 +279,23 @@ public class NotebookService {
     }
   }
 
+  /**
+   * Executes given paragraph with passed paragraph info like noteId, paragraphId, title, text and etc.
+   *
+   * @param noteId
+   * @param paragraphId
+   * @param title
+   * @param text
+   * @param params
+   * @param config
+   * @param failIfDisabled
+   * @param blocking
+   * @param context
+   * @param callback
+   * @return return true only when paragraph execution finished, it could end with succeed or error due to user code.
+   * return false when paragraph execution fails due to zeppelin internal issue.
+   * @throws IOException
+   */
   public boolean runParagraph(String noteId,
                               String paragraphId,
                               String title,
@@ -334,9 +351,9 @@ public class NotebookService {
 
     try {
       notebook.saveNote(note, context.getAutheInfo());
-      boolean result = note.run(p.getId(), blocking, context.getAutheInfo().getUser());
+      note.run(p.getId(), blocking, context.getAutheInfo().getUser());
       callback.onSuccess(p, context);
-      return result;
+      return true;
     } catch (Exception ex) {
       LOGGER.error("Exception from run", ex);
       p.setReturn(new InterpreterResult(InterpreterResult.Code.ERROR, ex.getMessage()), ex);
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 7498822..328a1b2 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
@@ -24,6 +24,7 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 
 import com.google.gson.Gson;
+import com.google.gson.internal.StringMap;
 import com.google.gson.reflect.TypeToken;
 
 import org.apache.commons.httpclient.methods.GetMethod;
@@ -40,6 +41,7 @@ import org.junit.Test;
 import org.junit.runners.MethodSorters;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -160,6 +162,28 @@ public class NotebookRestApiTest extends AbstractTestRestApi {
       // Check if the paragraph is emptied
       assertEquals(title, p.getTitle());
       assertEquals(text, p.getText());
+
+      // run invalid code
+      text = "%sh\n invalid_cmd";
+      p.setTitle(title);
+      p.setText(text);
+
+      post = httpPost("/notebook/run/" + note1.getId() + "/" + p.getId(), "");
+      assertEquals(500, post.getStatusCode());
+      resp = gson.fromJson(post.getResponseBodyAsString(),
+              new TypeToken<Map<String, Object>>() {}.getType());
+      assertEquals("INTERNAL_SERVER_ERROR", resp.get("status"));
+      StringMap stringMap = (StringMap) resp.get("body");
+      assertEquals("ERROR", stringMap.get("code"));
+      List<StringMap> interpreterResults = (List<StringMap>) stringMap.get("msg");
+      assertTrue(interpreterResults.get(0).toString(),
+              interpreterResults.get(0).get("data").toString().contains("invalid_cmd: command not found"));
+      post.releaseConnection();
+      assertNotEquals(p.getStatus(), Job.Status.READY);
+
+      // Check if the paragraph is emptied
+      assertEquals(title, p.getTitle());
+      assertEquals(text, p.getText());
     } finally {
       // cleanup
       if (null != note1) {
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
index 3b7e2f2..a463617 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
@@ -388,7 +388,7 @@ public class NotebookServiceTest {
     reset(callback);
     runStatus = notebookService.runParagraph(note1.getId(), p.getId(), "my_title", "invalid_code",
         new HashMap<>(), new HashMap<>(), false, true, context, callback);
-    assertFalse(runStatus);
+    assertTrue(runStatus);
     // TODO(zjffdu) Enable it after ZEPPELIN-3699
     // assertNotNull(p.getResult());
     verify(callback).onSuccess(p, context);
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 0af557c..ce70491 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
@@ -780,7 +780,7 @@ public class Note implements JsonSerializable {
   }
 
   /**
-   * Run a single paragraph
+   * Run a single paragraph. Return true only when paragraph run successfully.
    *
    * @param paragraphId
    * @param blocking
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java
index da4c7c6..0ff95c3 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java
@@ -347,6 +347,11 @@ public class Paragraph extends JobWithProgressPoller<InterpreterResult> implemen
     return checkEmptyConfig && Strings.isNullOrEmpty(scriptText) && localProperties.isEmpty();
   }
 
+  /**
+   * Return true only when paragraph run successfully with state of FINISHED.
+   * @param blocking
+   * @return
+   */
   public boolean execute(boolean blocking) {
     try {
       this.interpreter = getBindedInterpreter();