zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zjf...@apache.org
Subject zeppelin git commit: ZEPPELIN-3005. Refine the error message when interpreter is not binded to note
Date Sun, 29 Oct 2017 00:13:49 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/master abc197c2d -> 49ccc29b6


ZEPPELIN-3005. Refine the error message when interpreter is not binded to note

### What is this PR for?
More user-friendly error message.

### What type of PR is it?
[Improvement]

### Todos
* [ ] - Task

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

### How should this be tested?
Unit test added

### 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 <zjffdu@apache.org>

Closes #2629 from zjffdu/ZEPPELIN-3005 and squashes the following commits:

134c960 [Jeff Zhang] ZEPPELIN-3005. Refine the error message when interpreter is not binded
to note


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

Branch: refs/heads/master
Commit: 49ccc29b652acca94e4e819b0b5eb106f05aba6c
Parents: abc197c
Author: Jeff Zhang <zjffdu@apache.org>
Authored: Sun Oct 8 09:08:34 2017 +0800
Committer: Jeff Zhang <zjffdu@apache.org>
Committed: Sun Oct 29 08:13:44 2017 +0800

----------------------------------------------------------------------
 .../zeppelin/interpreter/InterpreterFactory.java  | 18 +++++++-----------
 .../interpreter/ManagedInterpreterGroup.java      |  3 ++-
 .../zeppelin/scheduler/RemoteScheduler.java       |  6 +++---
 .../interpreter/InterpreterFactoryTest.java       | 15 +++++++++++++--
 4 files changed, 25 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/49ccc29b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
index 7233239..911c285 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
@@ -76,10 +76,10 @@ public class InterpreterFactory {
         if (null != interpreter) {
           return interpreter;
         }
+        throw new RuntimeException("No such interpreter: " + replName);
       }
-      return null;
-
-    } else {
+      throw new RuntimeException("Interpreter " + group + " is not binded to this note");
+    } else if (replNameSplit.length == 1){
       // first assume replName is 'name' of interpreter. ('groupName' is ommitted)
       // search 'name' from first (default) interpreter group
       // TODO(jl): Handle with noteId to support defaultInterpreter per note.
@@ -90,19 +90,15 @@ public class InterpreterFactory {
         return interpreter;
       }
 
-      // next, assume replName is 'group' of interpreter ('name' is ommitted)
+      // next, assume replName is 'group' of interpreter ('name' is omitted)
       // search interpreter group and return first interpreter.
       setting = getInterpreterSettingByGroup(settings, replName);
 
       if (null != setting) {
         return setting.getDefaultInterpreter(user, noteId);
-      }
-
-      // Support the legacy way to use it
-      for (InterpreterSetting s : settings) {
-        if (s.getGroup().equals(replName)) {
-          return setting.getDefaultInterpreter(user, noteId);
-        }
+      } else {
+        throw new RuntimeException("Either no interpreter named " + replName + " or it is
not " +
+            "binded to this note");
       }
     }
     //TODO(zjffdu) throw InterpreterException instead of return null

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/49ccc29b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
index a8ae338..2c1e631 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
@@ -82,7 +82,8 @@ public class ManagedInterpreterGroup extends InterpreterGroup {
    * @param sessionId
    */
   public synchronized void close(String sessionId) {
-    LOGGER.info("Close Session: " + sessionId);
+    LOGGER.info("Close Session: " + sessionId + " for interpreter setting: " +
+        interpreterSetting.getName());
     close(sessions.remove(sessionId));
     //TODO(zjffdu) whether close InterpreterGroup if there's no session left in Zeppelin
Server
     if (sessions.isEmpty() && interpreterSetting != null) {

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/49ccc29b/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
index ac9d536..982b84a 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/scheduler/RemoteScheduler.java
@@ -321,14 +321,14 @@ public class RemoteScheduler implements Scheduler {
       if (job.isAborted()) {
         job.setStatus(Status.ABORT);
       } else if (job.getException() != null) {
-        logger.debug("Job ABORT, " + job.getId());
+        logger.debug("Job ABORT, " + job.getId() + ", " + job.getErrorMessage());
         job.setStatus(Status.ERROR);
       } else if (jobResult != null && jobResult instanceof InterpreterResult
           && ((InterpreterResult) jobResult).code() == Code.ERROR) {
-        logger.debug("Job Error, " + job.getId());
+        logger.debug("Job Error, " + job.getId() + ", " + job.getErrorMessage());
         job.setStatus(Status.ERROR);
       } else {
-        logger.debug("Job Finished, " + job.getId());
+        logger.debug("Job Finished, " + job.getId() + ", Result: " + job.getReturn());
         job.setStatus(Status.FINISHED);
       }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/49ccc29b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
index 8f82044..0a762d8 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
@@ -26,6 +26,7 @@ import java.io.IOException;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class InterpreterFactoryTest extends AbstractInterpreterTest {
 
@@ -56,12 +57,22 @@ public class InterpreterFactoryTest extends AbstractInterpreterTest {
   @Test
   public void testUnknownRepl1() throws IOException {
     interpreterSettingManager.setInterpreterBinding("user1", "note1", interpreterSettingManager.getSettingIds());
-    assertNull(interpreterFactory.getInterpreter("user1", "note1", "test.unknown_repl"));
+    try {
+      interpreterFactory.getInterpreter("user1", "note1", "test.unknown_repl");
+      fail("should fail due to no such interpreter");
+    } catch (RuntimeException e) {
+      assertEquals("No such interpreter: test.unknown_repl", e.getMessage());
+    }
   }
 
   @Test
   public void testUnknownRepl2() throws IOException {
     interpreterSettingManager.setInterpreterBinding("user1", "note1", interpreterSettingManager.getSettingIds());
-    assertNull(interpreterFactory.getInterpreter("user1", "note1", "unknown_repl"));
+    try {
+      interpreterFactory.getInterpreter("user1", "note1", "unknown_repl");
+      fail("should fail due to no such interpreter");
+    } catch (RuntimeException e) {
+      assertEquals("Either no interpreter named unknown_repl or it is not binded to this
note", e.getMessage());
+    }
   }
 }


Mime
View raw message