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/04/18 09:56:46 UTC

zeppelin git commit: ZEPPELIN-2386. Add parameter for check running current paragraph in ZeppelinContext

Repository: zeppelin
Updated Branches:
  refs/heads/master a7ffc1291 -> c706d453e


ZEPPELIN-2386. Add parameter for check running current paragraph in ZeppelinContext

### What is this PR for?
I can reproduce it via the test code in ZeppelinIT.testAngularDisplay. First run the 4 paragraphs to initiate the state. Then when I click paragraph 0 the first time, paragraph 2 will run correctly. But if I click paragraph 0 again, interpreter would raise the following exception. The cause is that ZeppelinContext's interpreterContext now point to paragraph 2, because paragraph 2 is the last paragraph that runs in this interpreter. So I propose to add parameter for check running current paragraph in ZeppelinContext.

* Add 2 methods in ZeppelinContext.   `run(idx, checkCurrentParagraph) `, 'run(noteId, idx, context, checkCurrentParagraph)'
* Also make some changes for debug logging.

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

### Todos
* [ ] - Task

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

### How should this be tested?
Tested manually and also add integration test.

### Screenshots (if appropriate)

Before
![zeppelin_before](https://cloud.githubusercontent.com/assets/164491/24895529/8514826e-1ec3-11e7-915b-70baf09c297a.gif)

After
![zeppelin_after](https://cloud.githubusercontent.com/assets/164491/24895532/88e87bc0-1ec3-11e7-90ee-a7ca7a3758f8.gif)

### 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 #2240 from zjffdu/ZEPPELIN-2386 and squashes the following commits:

03995a3 [Jeff Zhang] ZEPPELIN-2386. Add parameter to check running current paragraph in ZeppelinContext


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

Branch: refs/heads/master
Commit: c706d453e06a2f8c71fcd61d7e4dddc87cafc0f4
Parents: a7ffc12
Author: Jeff Zhang <zj...@apache.org>
Authored: Tue Apr 11 14:21:53 2017 +0800
Committer: Jeff Zhang <zj...@apache.org>
Committed: Tue Apr 18 17:56:39 2017 +0800

----------------------------------------------------------------------
 .../apache/zeppelin/spark/ZeppelinContext.java  | 60 +++++++++++++++++---
 .../apache/zeppelin/display/AngularObject.java  |  4 +-
 .../apache/zeppelin/socket/NotebookServer.java  |  9 +--
 .../apache/zeppelin/integration/ZeppelinIT.java | 17 +++++-
 4 files changed, 77 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c706d453/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java
----------------------------------------------------------------------
diff --git a/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java b/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java
index e187915..7e1ab70 100644
--- a/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java
+++ b/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java
@@ -312,7 +312,7 @@ public class ZeppelinContext {
    */
   @ZeppelinApi
   public void run(String noteId, String paragraphId) {
-    run(noteId, paragraphId, interpreterContext);
+    run(noteId, paragraphId, interpreterContext, true);
   }
 
   /**
@@ -321,18 +321,38 @@ public class ZeppelinContext {
    */
   @ZeppelinApi
   public void run(String paragraphId) {
+    run(paragraphId, true);
+  }
+
+  /**
+   * Run paragraph by id
+   * @param paragraphId
+   * @param checkCurrentParagraph
+   */
+  @ZeppelinApi
+  public void run(String paragraphId, boolean checkCurrentParagraph) {
     String noteId = interpreterContext.getNoteId();
-    run(noteId, paragraphId, interpreterContext);
+    run(noteId, paragraphId, interpreterContext, checkCurrentParagraph);
   }
 
   /**
    * Run paragraph by id
    * @param noteId
-   * @param context
    */
   @ZeppelinApi
   public void run(String noteId, String paragraphId, InterpreterContext context) {
-    if (paragraphId.equals(context.getParagraphId())) {
+    run(noteId, paragraphId, context, true);
+  }
+
+  /**
+   * Run paragraph by id
+   * @param noteId
+   * @param context
+   */
+  @ZeppelinApi
+  public void run(String noteId, String paragraphId, InterpreterContext context,
+                  boolean checkCurrentParagraph) {
+    if (paragraphId.equals(context.getParagraphId()) && checkCurrentParagraph) {
       throw new InterpreterException("Can not run current Paragraph");
     }
 
@@ -412,24 +432,50 @@ public class ZeppelinContext {
    */
   @ZeppelinApi
   public void run(int idx) {
+    run(idx, true);
+  }
+
+  /**
+   *
+   * @param idx  paragraph index
+   * @param checkCurrentParagraph  check whether you call this run method in the current paragraph.
+   * Set it to false only when you are sure you are not invoking this method to run current
+   * paragraph. Otherwise you would run current paragraph in infinite loop.
+   */
+  public void run(int idx, boolean checkCurrentParagraph) {
     String noteId = interpreterContext.getNoteId();
-    run(noteId, idx, interpreterContext);
+    run(noteId, idx, interpreterContext, checkCurrentParagraph);
   }
 
   /**
    * Run paragraph at index
+   * @param noteId
    * @param idx index starting from 0
    * @param context interpreter context
    */
   public void run(String noteId, int idx, InterpreterContext context) {
+    run(noteId, idx, context, true);
+  }
+
+  /**
+   *
+   * @param noteId
+   * @param idx  paragraph index
+   * @param context interpreter context
+   * @param checkCurrentParagraph check whether you call this run method in the current paragraph.
+   * Set it to false only when you are sure you are not invoking this method to run current
+   * paragraph. Otherwise you would run current paragraph in infinite loop.
+   */
+  public void run(String noteId, int idx, InterpreterContext context,
+                  boolean checkCurrentParagraph) {
     List<InterpreterContextRunner> runners = getInterpreterContextRunner(noteId, context);
     if (idx >= runners.size()) {
       throw new InterpreterException("Index out of bound");
     }
 
     InterpreterContextRunner runner = runners.get(idx);
-    if (runner.getParagraphId().equals(context.getParagraphId())) {
-      throw new InterpreterException("Can not run current Paragraph");
+    if (runner.getParagraphId().equals(context.getParagraphId()) && checkCurrentParagraph) {
+      throw new InterpreterException("Can not run current Paragraph: " + runner.getParagraphId());
     }
 
     runner.run();

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c706d453/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/AngularObject.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/AngularObject.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/AngularObject.java
index 929533b..84ee67e 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/AngularObject.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/AngularObject.java
@@ -34,6 +34,8 @@ import org.slf4j.LoggerFactory;
  * @param <T>
  */
 public class AngularObject<T> {
+  private static final Logger LOGGER = LoggerFactory.getLogger(AngularObject.class);
+
   private String name;
   private T object;
   
@@ -172,7 +174,7 @@ public class AngularObject<T> {
     if (emit) {
       emit();
     }
-
+    LOGGER.debug("Update angular object: " + name + " with value: " + o);
     final Logger logger = LoggerFactory.getLogger(AngularObject.class);
     List<AngularObjectWatcher> ws = new LinkedList<>();
     synchronized (watchers) {

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c706d453/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
index 23d80a4..87a6bca 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
@@ -183,10 +183,11 @@ public class NotebookServer extends WebSocketServlet
     Notebook notebook = notebook();
     try {
       Message messagereceived = deserializeMessage(msg);
-      LOG.debug("RECEIVE << " + messagereceived.op);
-      LOG.debug("RECEIVE PRINCIPAL << " + messagereceived.principal);
-      LOG.debug("RECEIVE TICKET << " + messagereceived.ticket);
-      LOG.debug("RECEIVE ROLES << " + messagereceived.roles);
+      LOG.debug("RECEIVE << " + messagereceived.op +
+          ", RECEIVE PRINCIPAL << " + messagereceived.principal +
+          ", RECEIVE TICKET << " + messagereceived.ticket +
+          ", RECEIVE ROLES << " + messagereceived.roles +
+          ", RECEIVE DATA << " + messagereceived.data);
 
       if (LOG.isTraceEnabled()) {
         LOG.trace("RECEIVE MSG = " + messagereceived);

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c706d453/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ZeppelinIT.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ZeppelinIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ZeppelinIT.java
index 63c3ab1..afdae10 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ZeppelinIT.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ZeppelinIT.java
@@ -137,7 +137,7 @@ public class ZeppelinIT extends AbstractZeppelinIT {
        *   z.run(2, context)
        * }
        */
-      setTextOfParagraph(4, "z.angularWatch(\"myVar\", (before:Object, after:Object, context:org.apache.zeppelin.interpreter.InterpreterContext)=>{ z.run(2)})");
+      setTextOfParagraph(4, "z.angularWatch(\"myVar\", (before:Object, after:Object, context:org.apache.zeppelin.interpreter.InterpreterContext)=>{ z.run(2, false)})");
       runParagraph(4);
       waitForParagraph(4, "FINISHED");
 
@@ -157,6 +157,21 @@ public class ZeppelinIT extends AbstractZeppelinIT {
       waitForText("myVar=3", By.xpath(
               getParagraphXPath(3) + "//div[contains(@id,\"_text\") and @class=\"text\"]"));
 
+
+      /*
+       * Click element, again and see watcher still works
+       */
+      driver.findElement(By.xpath(
+          getParagraphXPath(1) + "//div[@id=\"angularTestButton\"]")).click();
+      // check expected text
+      waitForText("BindingTest_4_", By.xpath(
+          getParagraphXPath(1) + "//div[@id=\"angularTestButton\"]"));
+      waitForParagraph(3, "FINISHED");
+
+      // check expected text by watcher
+      waitForText("myVar=4", By.xpath(
+          getParagraphXPath(3) + "//div[contains(@id,\"_text\") and @class=\"text\"]"));
+
       /*
        * Unbind
        * z.angularUnbind("myVar")