Repository: zeppelin
Updated Branches:
  refs/heads/master f236f1ccd -> 5991a3577


[ZEPPELIN-1306] Interpreter restarts on a note.

### What is this PR for?

This PR is for usability of restarting interpreter.
### What type of PR is it?

Improvement
### What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1306
### How should this be tested?

You can restart interpreter on the interpreter binding page on a note.
and can only restart `scoped` and `isolated` interpreter.
Please refer to screen shot.
### Screenshots (if appropriate)

![restartintp](https://cloud.githubusercontent.com/assets/3348133/17474018/b2739462-5d8e-11e6-81bb-da15544547a5.gif)
### Questions:
- Does the licenses files need update? no
- Is there breaking changes for older versions? no
- Does this needs documentation? no

Author: astroshim <[email protected]>

Closes #1302 from astroshim/ZEPPELIN-1306 and squashes the following commits:

186a361 [astroshim] rollback noteId check null
f38231c [astroshim] noteId Preconditions
af8a2d8 [astroshim] modify null check to Preconditions
72f3e4f [astroshim] fix testcase.
4dda64b [astroshim] rebase
a626188 [astroshim] rebase and fixing testcase
fc1c819 [astroshim] Merge branch 'ZEPPELIN-1306' of 
https://github.com/astroshim/zeppelin into ZEPPELIN-1306
499aa6b [astroshim] add PySparkInterpreter testcase
e8e0c17 [astroshim] rebase
36f5642 [astroshim] add InterpreterForNote testcase
fa45bb5 [astroshim] remove debug log
5c4b32a [astroshim] Merge branch 'master' into ZEPPELIN-1306
4c4339f [astroshim] Merge branch 'master' into ZEPPELIN-1306
72a2259 [astroshim] add testcase and fix the doc.
3127154 [astroshim] Merge branch 'master' into ZEPPELIN-1306
0453402 [astroshim] Merge branch 'master' into ZEPPELIN-1306
ecfa7fb [astroshim] remove dud code.
78fdd74 [astroshim] update alert message.
aa30b39 [astroshim] inactive link when interpreter is not selected.
123882d [astroshim] change button to icon
a980500 [astroshim] update html
f9ea386 [astroshim] fix code style
2cea65b [astroshim] change method name
50c6acf [astroshim] delete unnecessary code
29966a0 [astroshim] change UI and restart interpreter process
322d427 [astroshim] Merge branch 'master' into ZEPPELIN-1306
e698e6f [astroshim] change method name
03bdd05 [astroshim] Merge branch 'master' into ZEPPELIN-1306
4dbe05a [astroshim] change argument string to boolean
439b361 [astroshim] fix js code style.
5ad4503 [astroshim] Interpreter restarts on a note.


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

Branch: refs/heads/master
Commit: 5991a35774607b05e8eebe64242e12b7847ca72b
Parents: f236f1c
Author: astroshim <[email protected]>
Authored: Tue Oct 25 15:48:26 2016 +0900
Committer: Jongyoul Lee <[email protected]>
Committed: Mon Oct 31 14:07:36 2016 +0900

----------------------------------------------------------------------
 docs/rest-api/rest-interpreter.md               | 10 ++++
 .../zeppelin/rest/InterpreterRestApi.java       | 12 +++-
 .../rest/message/RestartInterpreterRequest.java | 33 +++++++++++
 .../zeppelin/rest/InterpreterRestApiTest.java   | 52 +++++++++++++++++
 .../src/app/notebook/notebook.controller.js     | 36 ++++++++++++
 zeppelin-web/src/app/notebook/notebook.css      |  8 +++
 zeppelin-web/src/app/notebook/notebook.html     | 35 +++++++-----
 .../interpreter/InterpreterFactory.java         | 18 +++++-
 .../apache/zeppelin/notebook/NotebookTest.java  | 60 ++++++++++++++++++++
 9 files changed, 246 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/docs/rest-api/rest-interpreter.md
----------------------------------------------------------------------
diff --git a/docs/rest-api/rest-interpreter.md 
b/docs/rest-api/rest-interpreter.md
index 75b0d90..4f85e69 100644
--- a/docs/rest-api/rest-interpreter.md
+++ b/docs/rest-api/rest-interpreter.md
@@ -401,6 +401,16 @@ The role of registered interpreters, settings and 
interpreters group are describ
       <td> 500 </td>
     </tr>
     <tr>
+      <td>Sample JSON input (Optional)</td>
+      <td>
+        <pre>
+{
+  "noteId": "2AVQJVC8N"
+}
+        </pre>
+      </td>
+    </tr>
+    <tr>
       <td>Sample JSON response</td>
       <td>
         <code>{"status":"OK"}</code>

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java
 
b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java
index 5b1773b..c9094eb 100644
--- 
a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java
+++ 
b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java
@@ -33,6 +33,7 @@ import javax.ws.rs.core.Response.Status;
 
 import com.google.gson.Gson;
 import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.zeppelin.rest.message.RestartInterpreterRequest;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonatype.aether.repository.RemoteRepository;
@@ -150,10 +151,15 @@ public class InterpreterRestApi {
   @PUT
   @Path("setting/restart/{settingId}")
   @ZeppelinApi
-  public Response restartSetting(@PathParam("settingId") String settingId) {
-    logger.info("Restart interpreterSetting {}", settingId);
+  public Response restartSetting(String message, @PathParam("settingId") 
String settingId) {
+    logger.info("Restart interpreterSetting {}, msg={}", settingId, message);
+
     try {
-      interpreterFactory.restart(settingId);
+      RestartInterpreterRequest request = gson.fromJson(message, 
RestartInterpreterRequest.class);
+
+      String noteId = request == null ? null : request.getNoteId();
+      interpreterFactory.restart(settingId, noteId);
+
     } catch (InterpreterException e) {
       logger.error("Exception in InterpreterRestApi while restartSetting ", e);
       return new JsonResponse<>(Status.NOT_FOUND, e.getMessage(), 
ExceptionUtils.getStackTrace(e))

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/RestartInterpreterRequest.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/RestartInterpreterRequest.java
 
b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/RestartInterpreterRequest.java
new file mode 100644
index 0000000..f28be61
--- /dev/null
+++ 
b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/RestartInterpreterRequest.java
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.rest.message;
+
+/**
+ * RestartInterpreter rest api request message
+ */
+public class RestartInterpreterRequest {
+  String noteId;
+
+  public RestartInterpreterRequest() {
+
+  }
+
+  public String getNoteId() {
+    return noteId;
+  }
+}

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
 
b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
index 6d4fb2c..6ea9107 100644
--- 
a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
+++ 
b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
@@ -25,6 +25,7 @@ import org.apache.commons.httpclient.methods.DeleteMethod;
 import org.apache.commons.httpclient.methods.GetMethod;
 import org.apache.commons.httpclient.methods.PostMethod;
 import org.apache.commons.httpclient.methods.PutMethod;
+import org.apache.zeppelin.interpreter.InterpreterOption;
 import org.apache.zeppelin.interpreter.InterpreterSetting;
 import org.apache.zeppelin.notebook.Note;
 import org.apache.zeppelin.notebook.Paragraph;
@@ -201,6 +202,57 @@ public class InterpreterRestApiTest extends 
AbstractTestRestApi {
   }
 
   @Test
+  public void testRestartInterpreterPerNote() throws IOException, 
InterruptedException {
+    // create new note
+    Note note = ZeppelinServer.notebook.createNote(anonymous);
+    note.addParagraph();
+    Paragraph p = note.getLastParagraph();
+    Map config = p.getConfig();
+    config.put("enabled", true);
+
+    // run markdown paragraph.
+    p.setConfig(config);
+    p.setText("%md markdown");
+    p.setAuthenticationInfo(anonymous);
+    note.run(p.getId());
+    while (p.getStatus() != Status.FINISHED) {
+      Thread.sleep(100);
+    }
+    assertEquals("<p>markdown</p>\n", p.getResult().message());
+
+    // get md interpreter
+    InterpreterSetting mdIntpSetting = null;
+    for (InterpreterSetting setting : 
ZeppelinServer.notebook.getInterpreterFactory().getInterpreterSettings(note.getId()))
 {
+      if (setting.getName().equals("md")) {
+        mdIntpSetting = setting;
+        break;
+      }
+    }
+
+    String jsonRequest = "{\"noteId\":\"" + note.getId() + "\"}";
+
+    // Restart isolated mode of Interpreter for note.
+    mdIntpSetting.getOption().setPerNote(InterpreterOption.ISOLATED);
+    PutMethod put = httpPut("/interpreter/setting/restart/" + 
mdIntpSetting.getId(), jsonRequest);
+    assertThat("isolated interpreter restart:", put, isAllowed());
+    put.releaseConnection();
+
+    // Restart scoped mode of Interpreter for note.
+    mdIntpSetting.getOption().setPerNote(InterpreterOption.SCOPED);
+    put = httpPut("/interpreter/setting/restart/" + mdIntpSetting.getId(), 
jsonRequest);
+    assertThat("scoped interpreter restart:", put, isAllowed());
+    put.releaseConnection();
+
+    // Restart shared mode of Interpreter for note.
+    mdIntpSetting.getOption().setPerNote(InterpreterOption.SHARED);
+    put = httpPut("/interpreter/setting/restart/" + mdIntpSetting.getId(), 
jsonRequest);
+    assertThat("shared interpreter restart:", put, isAllowed());
+    put.releaseConnection();
+
+    ZeppelinServer.notebook.removeNote(note.getId(), anonymous);
+  }
+
+  @Test
   public void testListRepository() throws IOException {
     GetMethod get = httpGet("/interpreter/repository");
     assertThat(get, isAllowed());

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/zeppelin-web/src/app/notebook/notebook.controller.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js 
b/zeppelin-web/src/app/notebook/notebook.controller.js
index 5203a1d..dbb7b94 100644
--- a/zeppelin-web/src/app/notebook/notebook.controller.js
+++ b/zeppelin-web/src/app/notebook/notebook.controller.js
@@ -651,6 +651,42 @@
       $scope.permissions.writers = angular.element('#selectWriters').val();
     }
 
+    $scope.restartInterpreter = function(interpeter) {
+      var thisConfirm = BootstrapDialog.confirm({
+        closable: false,
+        closeByBackdrop: false,
+        closeByKeyboard: false,
+        title: '',
+        message: 'Do you want to restart ' + interpeter.name + ' interpreter?',
+        callback: function(result) {
+          if (result) {
+            var payload  = {
+              'noteId': $scope.note.id
+            };
+
+            thisConfirm.$modalFooter.find('button').addClass('disabled');
+            thisConfirm.$modalFooter.find('button:contains("OK")')
+              .html('<i class="fa fa-circle-o-notch fa-spin"></i> Saving 
Setting');
+
+            $http.put(baseUrlSrv.getRestApiBase() + 
'/interpreter/setting/restart/' + interpeter.id, payload)
+              .success(function(data, status, headers, config) {
+              var index = _.findIndex($scope.interpreterSettings, {'id': 
interpeter.id});
+              $scope.interpreterSettings[index] = data.body;
+              thisConfirm.close();
+            }).error(function(data, status, headers, config) {
+              thisConfirm.close();
+              console.log('Error %o %o', status, data.message);
+              BootstrapDialog.show({
+                title: 'Error restart interpreter.',
+                message: data.message
+              });
+            });
+            return false;
+          }
+        }
+      });
+    };
+
     $scope.savePermissions = function() {
       convertPermissionsToArray();
       $http.put(baseUrlSrv.getRestApiBase() + '/notebook/' + $scope.note.id + 
'/permissions',

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/zeppelin-web/src/app/notebook/notebook.css
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/notebook.css 
b/zeppelin-web/src/app/notebook/notebook.css
index c11544f..1f78a7e 100644
--- a/zeppelin-web/src/app/notebook/notebook.css
+++ b/zeppelin-web/src/app/notebook/notebook.css
@@ -312,3 +312,11 @@
   min-width: 150px;
   max-width: 50%;
 }
+
+
+.inactivelink {
+  color: darkgrey;
+  cursor: default;
+  pointer-events: none;
+}
+

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/zeppelin-web/src/app/notebook/notebook.html
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/notebook.html 
b/zeppelin-web/src/app/notebook/notebook.html
index 4f775f4..a508ed6 100644
--- a/zeppelin-web/src/app/notebook/notebook.html
+++ b/zeppelin-web/src/app/notebook/notebook.html
@@ -32,20 +32,27 @@ limitations under the License.
       <div class="interpreterSettings"
            as-sortable="interpreterSelectionListeners" 
data-ng-model="interpreterBindings">
         <div data-ng-repeat="item in interpreterBindings" as-sortable-item>
-          <div as-sortable-item-handle
-               ng-click="item.selected = !item.selected"
-               class="btn"
-               ng-class="{'btn-info': item.selected, 'btn-default': 
!item.selected}">
-            <font style="font-size:16px">{{item.name}}</font>
-            <small>
-              <span style="display:inline-block" ng-repeat="intp in 
item.interpreters">
-                <span ng-show="!$first">, </span>
-                %<span ng-show="!$parent.$first || $first">{{item.name}}</span
-                ><span ng-show="(!$parent.$first || $first) && !$first">.</span
-                ><span ng-show="!$first">{{intp.name}}</span>
-                <span ng-show="$parent.$first && $first">(default)</span>
-              </span>
-            </small>
+          <div>
+            <a ng-click="restartInterpreter(item)" 
+               ng-class="{'inactivelink': !item.selected}"
+               tooltip="Restart">
+              <span class="glyphicon glyphicon-refresh btn-md"></span>
+            </a>&nbsp
+            <div as-sortable-item-handle
+                 ng-click="item.selected = !item.selected"
+                 class="btn"
+                 ng-class="{'btn-info': item.selected, 'btn-default': 
!item.selected}">
+              <font style="font-size:16px">{{item.name}}</font>
+              <small>
+                <span style="display:inline-block" ng-repeat="intp in 
item.interpreters">
+                  <span ng-show="!$first">, </span>
+                  %<span ng-show="!$parent.$first || 
$first">{{item.name}}</span
+                  ><span ng-show="(!$parent.$first || $first) && 
!$first">.</span
+                  ><span ng-show="!$first">{{intp.name}}</span>
+                  <span ng-show="$parent.$first && $first">(default)</span>
+                </span>
+              </small>
+            </div>
           </div>
         </div>
       </div>

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/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 63e6bee..7711b9a 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
@@ -955,13 +955,29 @@ public class InterpreterFactory implements 
InterpreterGroupFactory {
     }
   }
 
+  private boolean noteIdIsExist(String noteId) {
+    return noteId == null ? false : true;
+  }
+
+  public void restart(String settingId, String noteId) {
+    InterpreterSetting intpsetting = interpreterSettings.get(settingId);
+    Preconditions.checkNotNull(intpsetting);
+
+    if (noteIdIsExist(noteId) &&
+      intpsetting.getOption().isProcess()) {
+      intpsetting.closeAndRemoveInterpreterGroup(noteId);
+      return;
+    }
+    restart(settingId);
+  }
+
   public void restart(String id) {
     synchronized (interpreterSettings) {
       InterpreterSetting intpsetting = interpreterSettings.get(id);
       // Check if dependency in specified path is changed
       // If it did, overwrite old dependency jar with new one
-      copyDependenciesFromLocalPath(intpsetting);
       if (intpsetting != null) {
+        copyDependenciesFromLocalPath(intpsetting);
 
         stopJobAllInterpreter(intpsetting);
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5991a357/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
----------------------------------------------------------------------
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 2bb6112..c590c16 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
@@ -763,6 +763,66 @@ public class NotebookTest implements JobListenerFactory{
     notebook.removeNote(note2.getId(), anonymous);
   }
 
+
+  @Test
+  public void testPerNoteSessionInterpreter() throws IOException {
+    // create two notes
+    Note note1  = notebook.createNote(anonymous);
+    Paragraph p1 = note1.addParagraph();
+
+    Note note2  = notebook.createNote(anonymous);
+    Paragraph p2 = note2.addParagraph();
+
+    p1.setText("getId");
+    p1.setAuthenticationInfo(anonymous);
+    p2.setText("getId");
+    p2.setAuthenticationInfo(anonymous);
+
+    // shared mode.
+    note1.run(p1.getId());
+    note2.run(p2.getId());
+
+    while (p1.getStatus() != Status.FINISHED) Thread.yield();
+    while (p2.getStatus() != Status.FINISHED) Thread.yield();
+
+    assertEquals(p1.getResult().message(), p2.getResult().message());
+
+    // restart interpreter with scoped mode enabled
+    for (InterpreterSetting setting : 
notebook.getInterpreterFactory().getInterpreterSettings(note1.getId())) {
+      setting.getOption().setPerNote(InterpreterOption.SCOPED);
+      notebook.getInterpreterFactory().restart(setting.getId(), note1.getId());
+      notebook.getInterpreterFactory().restart(setting.getId(), note2.getId());
+    }
+
+    // run per note session enabled
+    note1.run(p1.getId());
+    note2.run(p2.getId());
+
+    while (p1.getStatus() != Status.FINISHED) Thread.yield();
+    while (p2.getStatus() != Status.FINISHED) Thread.yield();
+
+    assertNotEquals(p1.getResult().message(), p2.getResult().message());
+
+    // restart interpreter with isolated mode enabled
+    for (InterpreterSetting setting : 
notebook.getInterpreterFactory().getInterpreterSettings(note1.getId())) {
+      setting.getOption().setPerNote(InterpreterOption.ISOLATED);
+      notebook.getInterpreterFactory().restart(setting.getId(), note1.getId());
+      notebook.getInterpreterFactory().restart(setting.getId(), note2.getId());
+    }
+
+    // run per note process enabled
+    note1.run(p1.getId());
+    note2.run(p2.getId());
+
+    while (p1.getStatus() != Status.FINISHED) Thread.yield();
+    while (p2.getStatus() != Status.FINISHED) Thread.yield();
+
+    assertNotEquals(p1.getResult().message(), p2.getResult().message());
+
+    notebook.removeNote(note1.getId(), anonymous);
+    notebook.removeNote(note2.getId(), anonymous);
+  }
+
   @Test
   public void testPerSessionInterpreterCloseOnUnbindInterpreterSetting() 
throws IOException {
     // create a notes

Reply via email to