From a4188be5ec162defecefec115532c7e5fca8f0db Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Tue, 31 May 2016 12:05:30 +0900 Subject: [PATCH 1/9] [ZEPPELIN-905] fix failed notebook import bug --- .../main/java/org/apache/zeppelin/notebook/Note.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) 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 01d625abe00..9522cd1a57f 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 @@ -207,17 +207,19 @@ public void addCloneParagraph(Paragraph srcParagraph) { Map config = new HashMap<>(srcParagraph.getConfig()); Map param = new HashMap<>(srcParagraph.settings.getParams()); Map form = new HashMap<>(srcParagraph.settings.getForms()); - Gson gson = new Gson(); - InterpreterResult result = gson.fromJson( - gson.toJson(srcParagraph.getReturn()), - InterpreterResult.class); newParagraph.setConfig(config); newParagraph.settings.setParams(param); newParagraph.settings.setForms(form); newParagraph.setText(srcParagraph.getText()); newParagraph.setTitle(srcParagraph.getTitle()); - newParagraph.setReturn(result, null); + + try { + Gson gson = new Gson(); + String resultJson = gson.toJson(srcParagraph.getReturn()); + InterpreterResult result = gson.fromJson(resultJson, InterpreterResult.class); + newParagraph.setReturn(result, null); + } catch (Exception e) { /*ignore*/ } synchronized (paragraphs) { paragraphs.add(newParagraph); From 1e45a9e2d6f50dbe3c8d7eefa89618aa7496411d Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Tue, 31 May 2016 14:14:12 +0900 Subject: [PATCH 2/9] [ZEPPELIN-905] add test --- .../zeppelin/notebook/NotebookTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java index 53749d1ddea..493c9851165 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java @@ -316,6 +316,28 @@ public void testAutoRestartInterpreterAfterSchedule() throws InterruptedExceptio notebook.refreshCron(note.id()); } + @Test + public void testExportAndImportNote() throws IOException, CloneNotSupportedException, + InterruptedException { + Note note = notebook.createNote(); + note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + + final Paragraph p = note.addParagraph(); + String simpleText = "hello world"; + p.setText(simpleText); + + String exportedNoteJson = notebook.exportNote(note.getId()); + + Note importedNote = notebook.importNote(exportedNoteJson, "Title"); + + Paragraph p2 = importedNote.getParagraphs().get(0); + + // Test + assertEquals(p.getId(), p2.getId()); + assertEquals(p.text, p2.text); + assertEquals(p.getResult().message(), p2.getResult().message()); + } + @Test public void testCloneNote() throws IOException, CloneNotSupportedException, InterruptedException { From c13293fe7076803709aee4b7e6d0f8b1ad9d883e Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Tue, 31 May 2016 19:23:55 +0900 Subject: [PATCH 3/9] fix test, better implementation --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 9 ++------- .../java/org/apache/zeppelin/notebook/NotebookTest.java | 3 +++ 2 files changed, 5 insertions(+), 7 deletions(-) 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 9522cd1a57f..bd98b3dc01f 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 @@ -20,7 +20,6 @@ import java.io.IOException; import java.io.Serializable; import java.util.*; -import java.util.concurrent.Callable; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -40,7 +39,6 @@ import org.apache.zeppelin.scheduler.JobListener; import org.apache.zeppelin.search.SearchService; -import com.google.gson.Gson; import org.apache.zeppelin.user.AuthenticationInfo; import org.apache.zeppelin.user.Credentials; import org.slf4j.Logger; @@ -215,11 +213,8 @@ public void addCloneParagraph(Paragraph srcParagraph) { newParagraph.setTitle(srcParagraph.getTitle()); try { - Gson gson = new Gson(); - String resultJson = gson.toJson(srcParagraph.getReturn()); - InterpreterResult result = gson.fromJson(resultJson, InterpreterResult.class); - newParagraph.setReturn(result, null); - } catch (Exception e) { /*ignore*/ } + newParagraph.setReturn((InterpreterResult) srcParagraph.getReturn(), null); + } catch (ClassCastException e) { /*ignore*/ } synchronized (paragraphs) { paragraphs.add(newParagraph); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java index 493c9851165..fdfacc4fd44 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java @@ -326,6 +326,9 @@ public void testExportAndImportNote() throws IOException, CloneNotSupportedExcep String simpleText = "hello world"; p.setText(simpleText); + note.runAll(); + while(p.isTerminated()==false || p.getResult()==null) Thread.yield(); + String exportedNoteJson = notebook.exportNote(note.getId()); Note importedNote = notebook.importNote(exportedNoteJson, "Title"); From 803e08abb93ad44ef90608d4e3997518eeca2424 Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Wed, 1 Jun 2016 10:20:45 +0900 Subject: [PATCH 4/9] revert implementation --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 bd98b3dc01f..9522cd1a57f 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 @@ -20,6 +20,7 @@ import java.io.IOException; import java.io.Serializable; import java.util.*; +import java.util.concurrent.Callable; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -39,6 +40,7 @@ import org.apache.zeppelin.scheduler.JobListener; import org.apache.zeppelin.search.SearchService; +import com.google.gson.Gson; import org.apache.zeppelin.user.AuthenticationInfo; import org.apache.zeppelin.user.Credentials; import org.slf4j.Logger; @@ -213,8 +215,11 @@ public void addCloneParagraph(Paragraph srcParagraph) { newParagraph.setTitle(srcParagraph.getTitle()); try { - newParagraph.setReturn((InterpreterResult) srcParagraph.getReturn(), null); - } catch (ClassCastException e) { /*ignore*/ } + Gson gson = new Gson(); + String resultJson = gson.toJson(srcParagraph.getReturn()); + InterpreterResult result = gson.fromJson(resultJson, InterpreterResult.class); + newParagraph.setReturn(result, null); + } catch (Exception e) { /*ignore*/ } synchronized (paragraphs) { paragraphs.add(newParagraph); From 32949bc26b31450d0f3e9df83109760eb647d43f Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Wed, 1 Jun 2016 11:41:22 +0900 Subject: [PATCH 5/9] trigger CI build --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9522cd1a57f..e6399c13fe6 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 @@ -219,7 +219,7 @@ public void addCloneParagraph(Paragraph srcParagraph) { String resultJson = gson.toJson(srcParagraph.getReturn()); InterpreterResult result = gson.fromJson(resultJson, InterpreterResult.class); newParagraph.setReturn(result, null); - } catch (Exception e) { /*ignore*/ } + } catch (Exception e) { /* ignore */ } synchronized (paragraphs) { paragraphs.add(newParagraph); From d4f6699804db8f332380b17fd594162fc70d14a1 Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Fri, 3 Jun 2016 12:48:25 +0900 Subject: [PATCH 6/9] log exception --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 e6399c13fe6..1ee654d4979 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 @@ -219,7 +219,10 @@ public void addCloneParagraph(Paragraph srcParagraph) { String resultJson = gson.toJson(srcParagraph.getReturn()); InterpreterResult result = gson.fromJson(resultJson, InterpreterResult.class); newParagraph.setReturn(result, null); - } catch (Exception e) { /* ignore */ } + } catch (Exception e) { + // 'result' part of Note consists of exception, instead of actual interpreter results + logger.info("Paragraph " + srcParagraph.getId() + " has a result with exception."); + } synchronized (paragraphs) { paragraphs.add(newParagraph); From 7bf5d01de085332c009ef31ce18b5eae353ba4b5 Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Fri, 3 Jun 2016 19:02:43 +0900 Subject: [PATCH 7/9] log info -> warn, add message --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1ee654d4979..8bd377dd0a5 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 @@ -221,7 +221,7 @@ public void addCloneParagraph(Paragraph srcParagraph) { newParagraph.setReturn(result, null); } catch (Exception e) { // 'result' part of Note consists of exception, instead of actual interpreter results - logger.info("Paragraph " + srcParagraph.getId() + " has a result with exception."); + logger.warn("Paragraph " + srcParagraph.getId() + " has a result with exception. " + e.getMessage()); } synchronized (paragraphs) { From e7af919e82b54d858f933810d9a23c6dffb06e96 Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Fri, 3 Jun 2016 19:31:19 +0900 Subject: [PATCH 8/9] stylish code --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 8bd377dd0a5..08231e54722 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 @@ -221,7 +221,8 @@ public void addCloneParagraph(Paragraph srcParagraph) { newParagraph.setReturn(result, null); } catch (Exception e) { // 'result' part of Note consists of exception, instead of actual interpreter results - logger.warn("Paragraph " + srcParagraph.getId() + " has a result with exception. " + e.getMessage()); + logger.warn("Paragraph " + srcParagraph.getId() + " has a result with exception. " + + e.getMessage()); } synchronized (paragraphs) { From 69b8c02f6623686c3d23e6692f397468846eae8c Mon Sep 17 00:00:00 2001 From: Mina Lee Date: Sat, 18 Jun 2016 00:38:45 -0700 Subject: [PATCH 9/9] Add test for clone notebook with String type result --- .../org/apache/zeppelin/scheduler/Job.java | 2 +- .../zeppelin/notebook/NotebookTest.java | 29 +++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) 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 00df966de96..2dc1719ebac 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 @@ -255,7 +255,7 @@ public Date getDateFinished() { return dateFinished; } - protected void setResult(Object result) { + public void setResult(Object result) { this.result = result; } } diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java index fdfacc4fd44..d4e59970d01 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java @@ -43,7 +43,6 @@ import org.apache.zeppelin.resource.ResourcePoolUtils; import org.apache.zeppelin.scheduler.Job; import org.apache.zeppelin.scheduler.Job.Status; -import org.apache.zeppelin.scheduler.JobListener; import org.apache.zeppelin.scheduler.SchedulerFactory; import org.apache.zeppelin.search.SearchService; import org.apache.zeppelin.user.Credentials; @@ -327,7 +326,9 @@ public void testExportAndImportNote() throws IOException, CloneNotSupportedExcep p.setText(simpleText); note.runAll(); - while(p.isTerminated()==false || p.getResult()==null) Thread.yield(); + while (p.isTerminated() == false || p.getResult() == null) { + Thread.yield(); + } String exportedNoteJson = notebook.exportNote(note.getId()); @@ -363,6 +364,30 @@ public void testCloneNote() throws IOException, CloneNotSupportedException, assertEquals(cp.getResult().message(), p.getResult().message()); } + @Test + public void testCloneNoteWithExceptionResult() throws IOException, CloneNotSupportedException, + InterruptedException { + Note note = notebook.createNote(); + note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + + final Paragraph p = note.addParagraph(); + p.setText("hello world"); + note.runAll(); + while (p.isTerminated() == false || p.getResult() == null) { + Thread.yield(); + } + // Force paragraph to have String type object + p.setResult("Exception"); + + Note cloneNote = notebook.cloneNote(note.getId(), "clone note with Exception result"); + Paragraph cp = cloneNote.paragraphs.get(0); + + // Keep same ParagraphID + assertEquals(cp.getId(), p.getId()); + assertEquals(cp.text, p.text); + assertNull(cp.getResult()); + } + @Test public void testResourceRemovealOnParagraphNoteRemove() throws IOException { Note note = notebook.createNote();