You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2016/02/26 00:10:45 UTC

incubator-zeppelin git commit: [ZEPPELIN-691] Enable user to change shell interpreter command timeout property

Repository: incubator-zeppelin
Updated Branches:
  refs/heads/master ea9f13930 -> d5d918f52


[ZEPPELIN-691] Enable user to change shell interpreter command timeout property

### What is this PR for?
Although Shell Interpreter has already had [command timeout property](https://github.com/apache/incubator-zeppelin/blob/master/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java#L49), users can not change this property via Zeppelin UI. So I added some code lines to `ShellInterpreter.java` so that users can change this value at the interpreter setting tab.

### What type of PR is it?
Improvement

### Todos
* [x] - Enable user to change shell interpreter command timeout property
* [x] - Fix a dead link in `interpreter.java`

### Is there a relevant Jira issue?
[ZEPPELIN-691](https://issues.apache.org/jira/browse/ZEPPELIN-691)

### How should this be tested?
After applying this PR, you can make `%sh` interpreter at the Zeppelin interpreter setting section.
  1. Change `shell.command.timeout.millisecs` 600000(default value) -> 5000 so that you can check quickly
  2. Run

  * **Success Case**

  ```
  %sh
  date && sleep 3 && date
  ```

  then you can get a result as below

  ```
  Mon Feb 22 18:25:00 KST 2016
  Mon Feb 22 18:25:03 KST 2016
  ExitValue: 143
  ```

  * **Error Case(because of timeout property)**

  ```
  %sh
  date && sleep 6 && date
  ```

  then you can get a result as below

  ```
  Mon Feb 22 18:25:00 KST 2016
  Paragraph received a SIGTERM.
  ExitValue: 143
  ```

### Screenshots (if appropriate)
**Before**
![screen shot 2016-02-22 at 7 01 32 pm](https://cloud.githubusercontent.com/assets/10060731/13215178/efbcd26a-d996-11e5-96dd-1fc01a6c96ea.png)

**After**
![screen shot 2016-02-22 at 6 57 29 pm](https://cloud.githubusercontent.com/assets/10060731/13215151/ccbdd62e-d996-11e5-81e5-0eda4de7603d.png)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: AhyoungRyu <fb...@hanmail.net>

Closes #738 from AhyoungRyu/shell-timeout and squashes the following commits:

f98ff16 [AhyoungRyu] Move comma to the upper line
3b4faec [AhyoungRyu] Fix reference link in interpreter.java
b16211d [AhyoungRyu] Enable user to change shell interpreter timeout property


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

Branch: refs/heads/master
Commit: d5d918f52c4b5e3d4e7103546f3fff788c8e76b6
Parents: ea9f139
Author: AhyoungRyu <fb...@hanmail.net>
Authored: Mon Feb 22 22:59:24 2016 +0900
Committer: Lee moon soo <mo...@apache.org>
Committed: Thu Feb 25 15:14:20 2016 -0800

----------------------------------------------------------------------
 .../apache/zeppelin/shell/ShellInterpreter.java | 23 +++++++++++++++++---
 .../zeppelin/interpreter/Interpreter.java       |  2 +-
 2 files changed, 21 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d5d918f5/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java
----------------------------------------------------------------------
diff --git a/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java b/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java
index 9a0434f..6133c32 100644
--- a/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java
+++ b/shell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java
@@ -32,6 +32,7 @@ import org.apache.commons.exec.Executor;
 import org.apache.commons.exec.PumpStreamHandler;
 import org.apache.zeppelin.interpreter.Interpreter;
 import org.apache.zeppelin.interpreter.InterpreterContext;
+import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
 import org.apache.zeppelin.interpreter.InterpreterResult;
 import org.apache.zeppelin.interpreter.InterpreterResult.Code;
 import org.apache.zeppelin.scheduler.Job;
@@ -46,10 +47,22 @@ import org.slf4j.LoggerFactory;
 public class ShellInterpreter extends Interpreter {
   Logger logger = LoggerFactory.getLogger(ShellInterpreter.class);
   private static final String EXECUTOR_KEY = "executor";
-  int commandTimeOut = 600000;
+  public static final String SHELL_COMMAND_TIMEOUT = "shell.command.timeout.millisecs";
+  public static final String DEFAULT_COMMAND_TIMEOUT = "600000";
+  int commandTimeOut;
 
   static {
-    Interpreter.register("sh", ShellInterpreter.class.getName());
+    Interpreter.register(
+            "sh",
+            "sh",
+            ShellInterpreter.class.getName(),
+            new InterpreterPropertyBuilder()
+              .add(
+                SHELL_COMMAND_TIMEOUT,
+                DEFAULT_COMMAND_TIMEOUT,
+                "Shell command time out in millisecs. Default = 600000")
+              .build()
+    );
   }
 
   public ShellInterpreter(Properties property) {
@@ -57,7 +70,11 @@ public class ShellInterpreter extends Interpreter {
   }
 
   @Override
-  public void open() {}
+  public void open() {
+    logger.info("Command timeout is set as:", SHELL_COMMAND_TIMEOUT);
+
+    commandTimeOut = Integer.valueOf(getProperty(SHELL_COMMAND_TIMEOUT));
+  }
 
   @Override
   public void close() {}

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/d5d918f5/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
index 014b13e..a8c4508 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
@@ -35,7 +35,7 @@ import org.slf4j.LoggerFactory;
  * If you want to implement new Zeppelin interpreter, extend this class
  *
  * Please see,
- * http://zeppelin.incubator.apache.org/docs/development/writingzeppelininterpreter.html
+ * https://zeppelin.incubator.apache.org/docs/latest/development/writingzeppelininterpreter.html
  *
  * open(), close(), interpreter() is three the most important method you need to implement.
  * cancel(), getProgress(), completion() is good to have