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 2017/10/29 00:13:49 UTC
zeppelin git commit: ZEPPELIN-3005. Refine the error message when
interpreter is not binded to note
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 <zj...@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 <zj...@apache.org>
Authored: Sun Oct 8 09:08:34 2017 +0800
Committer: Jeff Zhang <zj...@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());
+ }
}
}