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 <[email protected]> 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 <[email protected]> Authored: Sun Oct 8 09:08:34 2017 +0800 Committer: Jeff Zhang <[email protected]> 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()); + } } }
