zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jongy...@apache.org
Subject zeppelin git commit: ZEPPELIN-3432 Fix results object when throwing exception while running job
Date Wed, 02 May 2018 04:04:37 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/master 97086be4e -> e503623de


ZEPPELIN-3432 Fix results object when throwing exception while running job

### What is this PR for?
Handling the issue that `InterpretJob.jobRun()` throws an `InterpreterException`

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

### Todos
* [x] - Change the logic to create `InterpreterResult` when throwing `InterpreterException`

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

### How should this be tested?
1. Enable `zeppelin.spark.sql.stacktrace` option
2. %sql select invalid query

### 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: Jongyoul Lee <jongyoul@gmail.com>

Closes #2950 from jongyoul/ZEPPELIN-3432 and squashes the following commits:

c0bdb1bed [Jongyoul Lee] Revert not to unwrap InterpreterException because found it's not
propagated to Server from Interpreter
039f6a046 [Jongyoul Lee] Fix JobTests
d47baccb4 [Jongyoul Lee] Add getting stack trace logic when error occurs
1631e6007 [Jongyoul Lee] Fix style
47d693520 [Jongyoul Lee] Make `InterpreterJob` a public inner class to instantiate it outside
of the class Set the type of `results` to InterpreterResult when exception occurred


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

Branch: refs/heads/master
Commit: e503623de6c3c9406e33991dab8e3872c7105092
Parents: 97086be
Author: Jongyoul Lee <jongyoul@gmail.com>
Authored: Mon Apr 30 14:57:07 2018 +0900
Committer: Jongyoul Lee <jongyoul@apache.org>
Committed: Wed May 2 13:04:31 2018 +0900

----------------------------------------------------------------------
 .../remote/RemoteInterpreterServer.java         |  8 +-
 .../java/org/apache/zeppelin/scheduler/Job.java |  4 +-
 .../org/apache/zeppelin/scheduler/JobTest.java  | 85 ++++++++++++++++++++
 3 files changed, 93 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/e503623d/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
index 401be36..e4db469 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
@@ -513,7 +513,8 @@ public class RemoteInterpreterServer extends Thread
     }
   }
 
-  class InterpretJob extends Job {
+  // TODO(jl): Need to extract this class from RemoteInterpreterServer to test it
+  public static class InterpretJob extends Job {
 
     private Interpreter interpreter;
     private String script;
@@ -521,7 +522,7 @@ public class RemoteInterpreterServer extends Thread
     private Map<String, Object> infos;
     private Object results;
 
-    InterpretJob(
+    public InterpretJob(
         String jobId,
         String jobName,
         JobListener listener,
@@ -592,7 +593,8 @@ public class RemoteInterpreterServer extends Thread
     }
 
     @Override
-    protected Object jobRun() throws Throwable {
+    // TODO(jl): need to redesign this class
+    public Object jobRun() throws Throwable {
       ClassLoader currentThreadContextClassloader = Thread.currentThread().getContextClassLoader();
       try {
         InterpreterContext.set(context);

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/e503623d/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java
index 579b604..8e5c823 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java
@@ -22,6 +22,8 @@ import java.util.Date;
 import java.util.Map;
 
 import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.zeppelin.interpreter.InterpreterResult;
+import org.apache.zeppelin.interpreter.InterpreterResult.Code;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -200,7 +202,7 @@ public abstract class Job {
   }
 
   private synchronized void completeWithError(Throwable error) {
-    setResult(error.getMessage());
+    setResult(new InterpreterResult(Code.ERROR, getStack(error)));
     setException(error);
     dateFinished = new Date();
   }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/e503623d/zeppelin-interpreter/src/test/java/org/apache/zeppelin/scheduler/JobTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/scheduler/JobTest.java
b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/scheduler/JobTest.java
new file mode 100644
index 0000000..ea80a14
--- /dev/null
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/scheduler/JobTest.java
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.scheduler;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.spy;
+
+import org.apache.zeppelin.interpreter.Interpreter;
+import org.apache.zeppelin.interpreter.InterpreterContext;
+import org.apache.zeppelin.interpreter.InterpreterException;
+import org.apache.zeppelin.interpreter.InterpreterResult;
+import org.apache.zeppelin.interpreter.InterpreterResult.Code;
+import org.apache.zeppelin.interpreter.remote.RemoteInterpreterServer.InterpretJob;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class JobTest {
+
+  @Mock private JobListener mockJobListener;
+  @Mock private Interpreter mockInterpreter;
+  @Mock private InterpreterContext mockInterpreterContext;
+  private InterpretJob spyInterpretJob;
+
+  @Before
+  public void setUp() throws Exception {
+    InterpretJob interpretJob =
+        new InterpretJob(
+            "jobid",
+            "jobName",
+            mockJobListener,
+            10000,
+            mockInterpreter,
+            "script",
+            mockInterpreterContext);
+    spyInterpretJob = spy(interpretJob);
+  }
+
+  @Test
+  public void testNormalCase() throws Throwable {
+
+    InterpreterResult successInterpreterResult =
+        new InterpreterResult(Code.SUCCESS, "success result");
+    doReturn(successInterpreterResult).when(spyInterpretJob).jobRun();
+
+    spyInterpretJob.run();
+
+    assertEquals(successInterpreterResult, spyInterpretJob.getReturn());
+  }
+
+  @Test
+  public void testErrorCase() throws Throwable {
+    String failedMessage = "failed message";
+    InterpreterException interpreterException = new InterpreterException(failedMessage);
+    doThrow(interpreterException).when(spyInterpretJob).jobRun();
+
+    spyInterpretJob.run();
+
+    Object failedResult = spyInterpretJob.getReturn();
+    assertTrue(failedResult instanceof InterpreterResult);
+    assertTrue(
+        ((InterpreterResult) failedResult).message().get(0).getData().contains(failedMessage));
+  }
+}


Mime
View raw message