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 2018/03/16 09:51:57 UTC
zeppelin git commit: ZEPPELIN-3343. Interpreter Hook is broken
Repository: zeppelin
Updated Branches:
refs/heads/master c6f7f2bdb -> 90b27cb67
ZEPPELIN-3343. Interpreter Hook is broken
### What is this PR for?
Currently the interpreter hook mechanism is broken. This PR fix this issue and also add unit test for interpreter hook, (including global hook and note level hook)
### What type of PR is it?
[Bug Fix | Improvement | Refactoring]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3343
### How should this be tested?
* CI pass
### 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 <zj...@apache.org>
Closes #2875 from zjffdu/ZEPPELIN-3343 and squashes the following commits:
3c55f07 [Jeff Zhang] ZEPPELIN-3343. Interpreter Hook is broken
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/90b27cb6
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/90b27cb6
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/90b27cb6
Branch: refs/heads/master
Commit: 90b27cb678e94084a2dee70b616969424b5999c6
Parents: c6f7f2b
Author: Jeff Zhang <zj...@apache.org>
Authored: Fri Mar 16 16:05:31 2018 +0800
Committer: Jeff Zhang <zj...@apache.org>
Committed: Fri Mar 16 17:51:50 2018 +0800
----------------------------------------------------------------------
.../zeppelin/python/PythonInterpreter.java | 6 +-
.../resources/grpc/python/zeppelin_python.py | 23 ++++
.../main/resources/python/zeppelin_python.py | 24 ++++
.../zeppelin/spark/PySparkInterpreter.java | 7 +-
.../main/resources/python/zeppelin_pyspark.py | 12 ++
.../interpreter/BaseZeppelinContext.java | 85 +++++++-----
.../zeppelin/interpreter/Interpreter.java | 4 +-
.../interpreter/InterpreterContext.java | 10 ++
.../interpreter/InterpreterHookRegistry.java | 70 +++++-----
.../interpreter/InvalidHookException.java | 29 ++++
.../interpreter/LazyOpenInterpreter.java | 4 +-
.../remote/RemoteInterpreterServer.java | 21 ++-
.../interpreter/BaseZeppelinContextTest.java | 133 +++++++++++++++++++
.../InterpreterHookRegistryTest.java | 54 ++++----
.../zeppelin/rest/ZeppelinSparkClusterTest.java | 31 ++++-
15 files changed, 403 insertions(+), 110 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
----------------------------------------------------------------------
diff --git a/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java b/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
index 1864409..f646de3 100644
--- a/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
+++ b/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
@@ -259,7 +259,11 @@ public class PythonInterpreter extends Interpreter implements ExecuteResultHandl
// Add matplotlib display hook
InterpreterGroup intpGroup = getInterpreterGroup();
if (intpGroup != null && intpGroup.getInterpreterHookRegistry() != null) {
- registerHook(HookType.POST_EXEC_DEV, "__zeppelin__._displayhook()");
+ try {
+ registerHook(HookType.POST_EXEC_DEV.getName(), "__zeppelin__._displayhook()");
+ } catch (InvalidHookException e) {
+ throw new InterpreterException(e);
+ }
}
// Add matplotlib display hook
try {
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/python/src/main/resources/grpc/python/zeppelin_python.py
----------------------------------------------------------------------
diff --git a/python/src/main/resources/grpc/python/zeppelin_python.py b/python/src/main/resources/grpc/python/zeppelin_python.py
index 6d7c33e..d76bdf4 100644
--- a/python/src/main/resources/grpc/python/zeppelin_python.py
+++ b/python/src/main/resources/grpc/python/zeppelin_python.py
@@ -113,6 +113,29 @@ class PyZeppelinContext(object):
#)
body_buf.close(); header_buf.close()
+ def registerHook(self, event, cmd, replName=None):
+ if replName is None:
+ self.z.registerHook(event, cmd)
+ else:
+ self.z.registerHook(event, cmd, replName)
+
+ def unregisterHook(self, event, replName=None):
+ if replName is None:
+ self.z.unregisterHook(event)
+ else:
+ self.z.unregisterHook(event, replName)
+
+ def registerNoteHook(self, event, cmd, noteId, replName=None):
+ if replName is None:
+ self.z.registerNoteHook(event, cmd, noteId)
+ else:
+ self.z.registerNoteHook(event, cmd, noteId, replName)
+
+ def unregisterNoteHook(self, event, noteId, replName=None):
+ if replName is None:
+ self.z.unregisterNoteHook(event, noteId)
+ else:
+ self.z.unregisterNoteHook(event, noteId, replName)
# start JVM gateway
client = GatewayClient(address='127.0.0.1', port=${JVM_GATEWAY_PORT})
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/python/src/main/resources/python/zeppelin_python.py
----------------------------------------------------------------------
diff --git a/python/src/main/resources/python/zeppelin_python.py b/python/src/main/resources/python/zeppelin_python.py
index f4ea2c7..cc4cb79 100644
--- a/python/src/main/resources/python/zeppelin_python.py
+++ b/python/src/main/resources/python/zeppelin_python.py
@@ -84,6 +84,30 @@ class PyZeppelinContext(object):
def noteCheckbox(self, name, options, defaultChecked=[]):
return self.z.noteCheckbox(name, self.getDefaultChecked(defaultChecked), self.getParamOptions(options))
+ def registerHook(self, event, cmd, replName=None):
+ if replName is None:
+ self.z.registerHook(event, cmd)
+ else:
+ self.z.registerHook(event, cmd, replName)
+
+ def unregisterHook(self, event, replName=None):
+ if replName is None:
+ self.z.unregisterHook(event)
+ else:
+ self.z.unregisterHook(event, replName)
+
+ def registerNoteHook(self, event, cmd, noteId, replName=None):
+ if replName is None:
+ self.z.registerNoteHook(event, cmd, noteId)
+ else:
+ self.z.registerNoteHook(event, cmd, noteId, replName)
+
+ def unregisterNoteHook(self, event, noteId, replName=None):
+ if replName is None:
+ self.z.unregisterNoteHook(event, noteId)
+ else:
+ self.z.unregisterNoteHook(event, noteId, replName)
+
def getParamOptions(self, options):
javaOptions = gateway.new_array(self.paramOption, len(options))
i = 0
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/spark/interpreter/src/main/java/org/apache/zeppelin/spark/PySparkInterpreter.java
----------------------------------------------------------------------
diff --git a/spark/interpreter/src/main/java/org/apache/zeppelin/spark/PySparkInterpreter.java b/spark/interpreter/src/main/java/org/apache/zeppelin/spark/PySparkInterpreter.java
index f5e4793..bd2a30c 100644
--- a/spark/interpreter/src/main/java/org/apache/zeppelin/spark/PySparkInterpreter.java
+++ b/spark/interpreter/src/main/java/org/apache/zeppelin/spark/PySparkInterpreter.java
@@ -38,6 +38,7 @@ import org.apache.zeppelin.interpreter.InterpreterHookRegistry.HookType;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.InterpreterResultMessage;
+import org.apache.zeppelin.interpreter.InvalidHookException;
import org.apache.zeppelin.interpreter.LazyOpenInterpreter;
import org.apache.zeppelin.interpreter.WrappedInterpreter;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
@@ -152,7 +153,11 @@ public class PySparkInterpreter extends Interpreter implements ExecuteResultHand
// Add matplotlib display hook
InterpreterGroup intpGroup = getInterpreterGroup();
if (intpGroup != null && intpGroup.getInterpreterHookRegistry() != null) {
- registerHook(HookType.POST_EXEC_DEV, "__zeppelin__._displayhook()");
+ try {
+ registerHook(HookType.POST_EXEC_DEV.getName(), "__zeppelin__._displayhook()");
+ } catch (InvalidHookException e) {
+ throw new InterpreterException(e);
+ }
}
DepInterpreter depInterpreter = getDepInterpreter();
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/spark/interpreter/src/main/resources/python/zeppelin_pyspark.py
----------------------------------------------------------------------
diff --git a/spark/interpreter/src/main/resources/python/zeppelin_pyspark.py b/spark/interpreter/src/main/resources/python/zeppelin_pyspark.py
index c10855a..00d8a9a 100644
--- a/spark/interpreter/src/main/resources/python/zeppelin_pyspark.py
+++ b/spark/interpreter/src/main/resources/python/zeppelin_pyspark.py
@@ -133,6 +133,18 @@ class PyZeppelinContext(dict):
else:
self.z.unregisterHook(event, replName)
+ def registerNoteHook(self, event, cmd, noteId, replName=None):
+ if replName is None:
+ self.z.registerNoteHook(event, cmd, noteId)
+ else:
+ self.z.registerNoteHook(event, cmd, noteId, replName)
+
+ def unregisterNoteHook(self, event, noteId, replName=None):
+ if replName is None:
+ self.z.unregisterNoteHook(event, noteId)
+ else:
+ self.z.unregisterNoteHook(event, noteId, replName)
+
def getHook(self, event, replName=None):
if replName is None:
return self.z.getHook(event)
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/BaseZeppelinContext.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/BaseZeppelinContext.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/BaseZeppelinContext.java
index 33d8626..139edd1 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/BaseZeppelinContext.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/BaseZeppelinContext.java
@@ -202,9 +202,6 @@ public abstract class BaseZeppelinContext {
this.noteGui = noteGui;
}
- private void restartInterpreter() {
- }
-
public InterpreterContext getInterpreterContext() {
return interpreterContext;
}
@@ -743,15 +740,10 @@ public abstract class BaseZeppelinContext {
* Get the interpreter class name from name entered in paragraph
* @param replName if replName is a valid className, return that instead.
*/
- public String getClassNameFromReplName(String replName) {
- for (String name : getInterpreterClassMap().keySet()) {
- if (replName.equals(name)) {
- return replName;
- }
- }
-
- if (replName.contains("spark.")) {
- replName = replName.replace("spark.", "");
+ private String getClassNameFromReplName(String replName) {
+ String[] splits = replName.split(".");
+ if (splits.length > 1) {
+ replName = splits[splits.length - 1];
}
return getInterpreterClassMap().get(replName);
}
@@ -763,10 +755,9 @@ public abstract class BaseZeppelinContext {
* @param replName Name of the interpreter
*/
@Experimental
- public void registerHook(String event, String cmd, String replName) {
- String noteId = interpreterContext.getNoteId();
+ public void registerHook(String event, String cmd, String replName) throws InvalidHookException {
String className = getClassNameFromReplName(replName);
- hooks.register(noteId, className, event, cmd);
+ hooks.register(null, className, event, cmd);
}
/**
@@ -775,55 +766,79 @@ public abstract class BaseZeppelinContext {
* @param cmd The code to be executed by the interpreter on given event
*/
@Experimental
- public void registerHook(String event, String cmd) {
- String className = interpreterContext.getInterpreterClassName();
- registerHook(event, cmd, className);
+ public void registerHook(String event, String cmd) throws InvalidHookException {
+ String replClassName = interpreterContext.getInterpreterClassName();
+ hooks.register(null, replClassName, event, cmd);
+ }
+
+ /**
+ *
+ * @param event
+ * @param cmd
+ * @param noteId
+ * @throws InvalidHookException
+ */
+ @Experimental
+ public void registerNoteHook(String event, String cmd, String noteId)
+ throws InvalidHookException {
+ String replClassName = interpreterContext.getInterpreterClassName();
+ hooks.register(noteId, replClassName, event, cmd);
+ }
+
+ @Experimental
+ public void registerNoteHook(String event, String cmd, String noteId, String replName)
+ throws InvalidHookException {
+ String className = getClassNameFromReplName(replName);
+ hooks.register(noteId, className, event, cmd);
}
/**
- * Get the hook code
+ * Unbind code from given hook event and given repl
+ *
* @param event The type of event to hook to (pre_exec, post_exec)
* @param replName Name of the interpreter
*/
@Experimental
- public String getHook(String event, String replName) {
- String noteId = interpreterContext.getNoteId();
+ public void unregisterHook(String event, String replName) {
String className = getClassNameFromReplName(replName);
- return hooks.get(noteId, className, event);
+ hooks.unregister(null, className, event);
}
/**
- * getHook() wrapper for current repl
+ * unregisterHook() wrapper for current repl
* @param event The type of event to hook to (pre_exec, post_exec)
*/
@Experimental
- public String getHook(String event) {
- String className = interpreterContext.getInterpreterClassName();
- return getHook(event, className);
+ public void unregisterHook(String event) {
+ unregisterHook(event, interpreterContext.getReplName());
}
/**
- * Unbind code from given hook event
+ * Unbind code from given hook event and given note
+ *
+ * @param noteId The id of note
* @param event The type of event to hook to (pre_exec, post_exec)
- * @param replName Name of the interpreter
*/
@Experimental
- public void unregisterHook(String event, String replName) {
- String noteId = interpreterContext.getNoteId();
- String className = getClassNameFromReplName(replName);
+ public void unregisterNoteHook(String noteId, String event) {
+ String className = interpreterContext.getInterpreterClassName();
hooks.unregister(noteId, className, event);
}
+
/**
- * unregisterHook() wrapper for current repl
+ * Unbind code from given hook event, given note and given repl
+ * @param noteId The id of note
* @param event The type of event to hook to (pre_exec, post_exec)
+ * @param replName Name of the interpreter
*/
@Experimental
- public void unregisterHook(String event) {
- String className = interpreterContext.getInterpreterClassName();
- unregisterHook(event, className);
+ public void unregisterNoteHook(String noteId, String event, String replName) {
+ String className = getClassNameFromReplName(replName);
+ hooks.unregister(noteId, className, event);
}
+
/**
* Add object into resource pool
* @param name
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/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 52cc161..7b591e7 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
@@ -240,7 +240,7 @@ public abstract class Interpreter {
* @param cmd The code to be executed by the interpreter on given event
*/
@Experimental
- public void registerHook(String noteId, String event, String cmd) {
+ public void registerHook(String noteId, String event, String cmd) throws InvalidHookException {
InterpreterHookRegistry hooks = interpreterGroup.getInterpreterHookRegistry();
String className = getClassName();
hooks.register(noteId, className, event, cmd);
@@ -253,7 +253,7 @@ public abstract class Interpreter {
* @param cmd The code to be executed by the interpreter on given event
*/
@Experimental
- public void registerHook(String event, String cmd) {
+ public void registerHook(String event, String cmd) throws InvalidHookException {
registerHook(null, event, cmd);
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterContext.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterContext.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterContext.java
index 6ff90a3..e4518a4 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterContext.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterContext.java
@@ -88,6 +88,16 @@ public class InterpreterContext {
return this;
}
+ public Builder setInterpreterClassName(String intpClassName) {
+ context.interpreterClassName = intpClassName;
+ return this;
+ }
+
+ public Builder setReplName(String replName) {
+ context.replName = replName;
+ return this;
+ }
+
public InterpreterContext build() {
return context;
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterHookRegistry.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterHookRegistry.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterHookRegistry.java
index 9df76f1..83917ec 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterHookRegistry.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterHookRegistry.java
@@ -18,41 +18,29 @@
package org.apache.zeppelin.interpreter;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
/**
- * The InterpreterinterpreterHookRegistry specifies code to be conditionally executed by an
+ * The InterpreterHookRegistry specifies code to be conditionally executed by an
* interpreter. The constants defined in this class denote currently
* supported events. Each instance is bound to a single InterpreterGroup.
* Scope is determined on a per-note basis (except when null for global scope).
*/
public class InterpreterHookRegistry {
- public static final String GLOBAL_KEY = "_GLOBAL_";
- private String interpreterId;
+ static final String GLOBAL_KEY = "_GLOBAL_";
+
+ // Scope (noteId/global scope) -> (ClassName -> (EventType -> Hook Code))
private Map<String, Map<String, Map<String, String>>> registry = new HashMap<>();
- /**
- * hookRegistry constructor.
- *
- * @param interpreterId The Id of the InterpreterGroup instance to bind to
- */
- public InterpreterHookRegistry(final String interpreterId) {
- this.interpreterId = interpreterId;
- }
-
- /**
- * Get the interpreterGroup id this instance is bound to
- */
- public String getInterpreterId() {
- return interpreterId;
- }
-
+
/**
* Adds a note to the registry
*
* @param noteId The Id of the Note instance to add
*/
- public void addNote(String noteId) {
+ private void addNote(String noteId) {
synchronized (registry) {
if (registry.get(noteId) == null) {
registry.put(noteId, new HashMap<String, Map<String, String>>());
@@ -66,7 +54,7 @@ public class InterpreterHookRegistry {
* @param noteId The note id
* @param className The name of the interpreter repl to map the hooks to
*/
- public void addRepl(String noteId, String className) {
+ private void addRepl(String noteId, String className) {
synchronized (registry) {
addNote(noteId);
if (registry.get(noteId).get(className) == null) {
@@ -84,19 +72,15 @@ public class InterpreterHookRegistry {
* @param cmd Code to be executed by the interpreter
*/
public void register(String noteId, String className,
- String event, String cmd) throws IllegalArgumentException {
+ String event, String cmd) throws InvalidHookException {
synchronized (registry) {
+ if (!HookType.ValidEvents.contains(event)) {
+ throw new InvalidHookException("event " + event + " is not valid hook event");
+ }
if (noteId == null) {
noteId = GLOBAL_KEY;
}
addRepl(noteId, className);
- if (!event.equals(HookType.POST_EXEC) && !event.equals(HookType.PRE_EXEC) &&
- !event.equals(HookType.POST_EXEC_DEV) && !event.equals(HookType.PRE_EXEC_DEV)) {
- throw new IllegalArgumentException("Must be " + HookType.POST_EXEC + ", " +
- HookType.POST_EXEC_DEV + ", " +
- HookType.PRE_EXEC + " or " +
- HookType.PRE_EXEC_DEV);
- }
registry.get(noteId).get(className).put(event, cmd);
}
}
@@ -138,18 +122,36 @@ public class InterpreterHookRegistry {
/**
* Container for hook event type constants
*/
- public static final class HookType {
+ public enum HookType {
+
// Execute the hook code PRIOR to main paragraph code execution
- public static final String PRE_EXEC = "pre_exec";
+ PRE_EXEC("pre_exec"),
// Execute the hook code AFTER main paragraph code execution
- public static final String POST_EXEC = "post_exec";
+ POST_EXEC("post_exec"),
// Same as above but reserved for interpreter developers, in order to allow
// notebook users to use the above without overwriting registry settings
// that are initialized directly in subclasses of Interpreter.
- public static final String PRE_EXEC_DEV = "pre_exec_dev";
- public static final String POST_EXEC_DEV = "post_exec_dev";
+ PRE_EXEC_DEV("pre_exec_dev"),
+ POST_EXEC_DEV("post_exec_dev");
+
+ private String name;
+
+ HookType(String name) {
+ this.name = name;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public static Set<String> ValidEvents = new HashSet();
+ static {
+ for (HookType type : values()) {
+ ValidEvents.add(type.getName());
+ }
+ }
}
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InvalidHookException.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InvalidHookException.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InvalidHookException.java
new file mode 100644
index 0000000..9b44726
--- /dev/null
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InvalidHookException.java
@@ -0,0 +1,29 @@
+/*
+ * 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.interpreter;
+
+/**
+ * Exception for invalid hook
+ */
+public class InvalidHookException extends Exception {
+
+ public InvalidHookException(String message) {
+ super(message);
+ }
+}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/LazyOpenInterpreter.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/LazyOpenInterpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/LazyOpenInterpreter.java
index ffb8140..7581e67 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/LazyOpenInterpreter.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/LazyOpenInterpreter.java
@@ -165,12 +165,12 @@ public class LazyOpenInterpreter
}
@Override
- public void registerHook(String noteId, String event, String cmd) {
+ public void registerHook(String noteId, String event, String cmd) throws InvalidHookException {
intp.registerHook(noteId, event, cmd);
}
@Override
- public void registerHook(String event, String cmd) {
+ public void registerHook(String event, String cmd) throws InvalidHookException {
intp.registerHook(event, cmd);
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
index 88ac59e..b5c7aef 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
@@ -280,7 +280,7 @@ public class RemoteInterpreterServer extends Thread
if (interpreterGroup == null) {
interpreterGroup = new InterpreterGroup(interpreterGroupId);
angularObjectRegistry = new AngularObjectRegistry(interpreterGroup.getId(), this);
- hookRegistry = new InterpreterHookRegistry(interpreterGroup.getId());
+ hookRegistry = new InterpreterHookRegistry();
resourcePool = new DistributedResourcePool(interpreterGroup.getId(), eventClient);
interpreterGroup.setInterpreterHookRegistry(hookRegistry);
interpreterGroup.setAngularObjectRegistry(angularObjectRegistry);
@@ -563,8 +563,8 @@ public class RemoteInterpreterServer extends Thread
InterpreterHookListener hookListener = new InterpreterHookListener() {
@Override
public void onPreExecute(String script) {
- String cmdDev = interpreter.getHook(noteId, HookType.PRE_EXEC_DEV);
- String cmdUser = interpreter.getHook(noteId, HookType.PRE_EXEC);
+ String cmdDev = interpreter.getHook(noteId, HookType.PRE_EXEC_DEV.getName());
+ String cmdUser = interpreter.getHook(noteId, HookType.PRE_EXEC.getName());
// User defined hook should be executed before dev hook
List<String> cmds = Arrays.asList(cmdDev, cmdUser);
@@ -579,8 +579,8 @@ public class RemoteInterpreterServer extends Thread
@Override
public void onPostExecute(String script) {
- String cmdDev = interpreter.getHook(noteId, HookType.POST_EXEC_DEV);
- String cmdUser = interpreter.getHook(noteId, HookType.POST_EXEC);
+ String cmdDev = interpreter.getHook(noteId, HookType.POST_EXEC_DEV.getName());
+ String cmdUser = interpreter.getHook(noteId, HookType.POST_EXEC.getName());
// User defined hook should be executed after dev hook
List<String> cmds = Arrays.asList(cmdUser, cmdDev);
@@ -616,9 +616,16 @@ public class RemoteInterpreterServer extends Thread
if (result == null || result.code() == Code.SUCCESS) {
// Add hooks to script from registry.
- // Global scope first, followed by notebook scope
- processInterpreterHooks(null);
+ // note scope first, followed by global scope.
+ // Here's the code after hooking:
+ // global_pre_hook
+ // note_pre_hook
+ // script
+ // note_post_hook
+ // global_post_hook
processInterpreterHooks(context.getNoteId());
+ processInterpreterHooks(null);
+ logger.debug("Script after hooks: " + script);
result = interpreter.interpret(script, context);
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/BaseZeppelinContextTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/BaseZeppelinContextTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/BaseZeppelinContextTest.java
new file mode 100644
index 0000000..db9cfd8
--- /dev/null
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/BaseZeppelinContextTest.java
@@ -0,0 +1,133 @@
+/*
+ * 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.interpreter;
+
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+
+public class BaseZeppelinContextTest {
+
+ @Test
+ public void testHooks() throws InvalidHookException {
+ InterpreterHookRegistry hookRegistry = new InterpreterHookRegistry();
+ TestZeppelinContext z = new TestZeppelinContext(hookRegistry, 10);
+ InterpreterContext context = InterpreterContext.builder()
+ .setNoteId("note_1")
+ .setParagraphId("paragraph_1")
+ .setInterpreterClassName("Test1Interpreter")
+ .setReplName("test1")
+ .build();
+ z.setInterpreterContext(context);
+
+ // register global hook for current interpreter
+ z.registerHook(InterpreterHookRegistry.HookType.PRE_EXEC.getName(), "pre_cmd");
+ z.registerHook(InterpreterHookRegistry.HookType.POST_EXEC.getName(), "post_cmd");
+ assertEquals("pre_cmd", hookRegistry.get(null, "Test1Interpreter",
+ InterpreterHookRegistry.HookType.PRE_EXEC.getName()));
+ assertEquals("post_cmd", hookRegistry.get(null, "Test1Interpreter",
+ InterpreterHookRegistry.HookType.POST_EXEC.getName()));
+
+ z.unregisterHook(InterpreterHookRegistry.HookType.PRE_EXEC.getName());
+ z.unregisterHook(InterpreterHookRegistry.HookType.POST_EXEC.getName());
+ assertEquals(null, hookRegistry.get(null, "Test1Interpreter",
+ InterpreterHookRegistry.HookType.PRE_EXEC.getName()));
+ assertEquals(null, hookRegistry.get(null, "Test1Interpreter",
+ InterpreterHookRegistry.HookType.POST_EXEC.getName()));
+
+ // register global hook for interpreter test2
+ z.registerHook(InterpreterHookRegistry.HookType.PRE_EXEC.getName(), "pre_cmd2", "test2");
+ z.registerHook(InterpreterHookRegistry.HookType.POST_EXEC.getName(), "post_cmd2", "test2");
+ assertEquals("pre_cmd2", hookRegistry.get(null, "Test2Interpreter",
+ InterpreterHookRegistry.HookType.PRE_EXEC.getName()));
+ assertEquals("post_cmd2", hookRegistry.get(null, "Test2Interpreter",
+ InterpreterHookRegistry.HookType.POST_EXEC.getName()));
+
+ z.unregisterHook(InterpreterHookRegistry.HookType.PRE_EXEC.getName(), "test2");
+ z.unregisterHook(InterpreterHookRegistry.HookType.POST_EXEC.getName(), "test2");
+ assertEquals(null, hookRegistry.get(null, "Test2Interpreter",
+ InterpreterHookRegistry.HookType.PRE_EXEC.getName()));
+ assertEquals(null, hookRegistry.get(null, "Test2Interpreter",
+ InterpreterHookRegistry.HookType.POST_EXEC.getName()));
+
+ // register hook for note_1 and current interpreter
+ z.registerNoteHook(InterpreterHookRegistry.HookType.PRE_EXEC.getName(), "pre_cmd", "note_1");
+ z.registerNoteHook(InterpreterHookRegistry.HookType.POST_EXEC.getName(), "post_cmd", "note_1");
+ assertEquals("pre_cmd", hookRegistry.get("note_1", "Test1Interpreter",
+ InterpreterHookRegistry.HookType.PRE_EXEC.getName()));
+ assertEquals("post_cmd", hookRegistry.get("note_1", "Test1Interpreter",
+ InterpreterHookRegistry.HookType.POST_EXEC.getName()));
+
+ z.unregisterNoteHook("note_1", InterpreterHookRegistry.HookType.PRE_EXEC.getName(), "test1");
+ z.unregisterNoteHook("note_1", InterpreterHookRegistry.HookType.POST_EXEC.getName(), "test1");
+ assertEquals(null, hookRegistry.get("note_1", "Test1Interpreter",
+ InterpreterHookRegistry.HookType.PRE_EXEC.getName()));
+ assertEquals(null, hookRegistry.get("note_1", "Test1Interpreter",
+ InterpreterHookRegistry.HookType.POST_EXEC.getName()));
+
+ // register hook for note_1 and interpreter test2
+ z.registerNoteHook(InterpreterHookRegistry.HookType.PRE_EXEC.getName(),
+ "pre_cmd2", "note_1", "test2");
+ z.registerNoteHook(InterpreterHookRegistry.HookType.POST_EXEC.getName(),
+ "post_cmd2", "note_1", "test2");
+ assertEquals("pre_cmd2", hookRegistry.get("note_1", "Test2Interpreter",
+ InterpreterHookRegistry.HookType.PRE_EXEC.getName()));
+ assertEquals("post_cmd2", hookRegistry.get("note_1", "Test2Interpreter",
+ InterpreterHookRegistry.HookType.POST_EXEC.getName()));
+
+ z.unregisterNoteHook("note_1", InterpreterHookRegistry.HookType.PRE_EXEC.getName(), "test2");
+ z.unregisterNoteHook("note_1", InterpreterHookRegistry.HookType.POST_EXEC.getName(), "test2");
+ assertEquals(null, hookRegistry.get("note_1", "Test2Interpreter",
+ InterpreterHookRegistry.HookType.PRE_EXEC.getName()));
+ assertEquals(null, hookRegistry.get("note_1", "Test2Interpreter",
+ InterpreterHookRegistry.HookType.POST_EXEC.getName()));
+ }
+
+
+ public static class TestZeppelinContext extends BaseZeppelinContext {
+
+ public TestZeppelinContext(InterpreterHookRegistry hooks, int maxResult) {
+ super(hooks, maxResult);
+ }
+
+ @Override
+ public Map<String, String> getInterpreterClassMap() {
+ Map<String, String> map = new HashMap<>();
+ map.put("test1", "Test1Interpreter");
+ map.put("test2", "Test2Interpreter");
+ return map;
+ }
+
+ @Override
+ public List<Class> getSupportedClasses() {
+ return null;
+ }
+
+ @Override
+ protected String showData(Object obj) {
+ return null;
+ }
+ }
+
+
+}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterHookRegistryTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterHookRegistryTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterHookRegistryTest.java
index eab8a28..2381dc3 100644
--- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterHookRegistryTest.java
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterHookRegistryTest.java
@@ -19,54 +19,54 @@ package org.apache.zeppelin.interpreter;
import org.junit.Test;
+import static org.apache.zeppelin.interpreter.InterpreterHookRegistry.HookType.POST_EXEC;
+import static org.apache.zeppelin.interpreter.InterpreterHookRegistry.HookType.POST_EXEC_DEV;
+import static org.apache.zeppelin.interpreter.InterpreterHookRegistry.HookType.PRE_EXEC;
+import static org.apache.zeppelin.interpreter.InterpreterHookRegistry.HookType.PRE_EXEC_DEV;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
public class InterpreterHookRegistryTest {
@Test
- public void testBasic() {
- final String PRE_EXEC = InterpreterHookRegistry.HookType.PRE_EXEC;
- final String POST_EXEC = InterpreterHookRegistry.HookType.POST_EXEC;
- final String PRE_EXEC_DEV = InterpreterHookRegistry.HookType.PRE_EXEC_DEV;
- final String POST_EXEC_DEV = InterpreterHookRegistry.HookType.POST_EXEC_DEV;
+ public void testBasic() throws InvalidHookException {
final String GLOBAL_KEY = InterpreterHookRegistry.GLOBAL_KEY;
final String noteId = "note";
final String className = "class";
final String preExecHook = "pre";
final String postExecHook = "post";
- InterpreterHookRegistry registry = new InterpreterHookRegistry("intpId");
+ InterpreterHookRegistry registry = new InterpreterHookRegistry();
// Test register()
- registry.register(noteId, className, PRE_EXEC, preExecHook);
- registry.register(noteId, className, POST_EXEC, postExecHook);
- registry.register(noteId, className, PRE_EXEC_DEV, preExecHook);
- registry.register(noteId, className, POST_EXEC_DEV, postExecHook);
+ registry.register(noteId, className, PRE_EXEC.getName(), preExecHook);
+ registry.register(noteId, className, POST_EXEC.getName(), postExecHook);
+ registry.register(noteId, className, PRE_EXEC_DEV.getName(), preExecHook);
+ registry.register(noteId, className, POST_EXEC_DEV.getName(), postExecHook);
// Test get()
- assertEquals(registry.get(noteId, className, PRE_EXEC), preExecHook);
- assertEquals(registry.get(noteId, className, POST_EXEC), postExecHook);
- assertEquals(registry.get(noteId, className, PRE_EXEC_DEV), preExecHook);
- assertEquals(registry.get(noteId, className, POST_EXEC_DEV), postExecHook);
+ assertEquals(registry.get(noteId, className, PRE_EXEC.getName()), preExecHook);
+ assertEquals(registry.get(noteId, className, POST_EXEC.getName()), postExecHook);
+ assertEquals(registry.get(noteId, className, PRE_EXEC_DEV.getName()), preExecHook);
+ assertEquals(registry.get(noteId, className, POST_EXEC_DEV.getName()), postExecHook);
// Test Unregister
- registry.unregister(noteId, className, PRE_EXEC);
- registry.unregister(noteId, className, POST_EXEC);
- registry.unregister(noteId, className, PRE_EXEC_DEV);
- registry.unregister(noteId, className, POST_EXEC_DEV);
- assertNull(registry.get(noteId, className, PRE_EXEC));
- assertNull(registry.get(noteId, className, POST_EXEC));
- assertNull(registry.get(noteId, className, PRE_EXEC_DEV));
- assertNull(registry.get(noteId, className, POST_EXEC_DEV));
+ registry.unregister(noteId, className, PRE_EXEC.getName());
+ registry.unregister(noteId, className, POST_EXEC.getName());
+ registry.unregister(noteId, className, PRE_EXEC_DEV.getName());
+ registry.unregister(noteId, className, POST_EXEC_DEV.getName());
+ assertNull(registry.get(noteId, className, PRE_EXEC.getName()));
+ assertNull(registry.get(noteId, className, POST_EXEC.getName()));
+ assertNull(registry.get(noteId, className, PRE_EXEC_DEV.getName()));
+ assertNull(registry.get(noteId, className, POST_EXEC_DEV.getName()));
// Test Global Scope
- registry.register(null, className, PRE_EXEC, preExecHook);
- assertEquals(registry.get(GLOBAL_KEY, className, PRE_EXEC), preExecHook);
+ registry.register(null, className, PRE_EXEC.getName(), preExecHook);
+ assertEquals(registry.get(GLOBAL_KEY, className, PRE_EXEC.getName()), preExecHook);
}
- @Test(expected = IllegalArgumentException.class)
- public void testValidEventCode() {
- InterpreterHookRegistry registry = new InterpreterHookRegistry("intpId");
+ @Test(expected = InvalidHookException.class)
+ public void testValidEventCode() throws InvalidHookException {
+ InterpreterHookRegistry registry = new InterpreterHookRegistry();
// Test that only valid event codes ("pre_exec", "post_exec") are accepted
registry.register("foo", "bar", "baz", "whatever");
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/90b27cb6/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
index 14eb0d9..a440419 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
@@ -379,7 +379,36 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi {
}
@Test
- public void pySparkDepLoaderTest() throws IOException, InterpreterException {
+ public void testZeppelinContextHook() throws IOException {
+ Note note = ZeppelinServer.notebook.createNote(anonymous);
+
+ // register global hook & note1 hook
+ Paragraph p1 = note.addNewParagraph(anonymous);
+ p1.setText("%python from __future__ import print_function\n" +
+ "z.registerHook('pre_exec', 'print(1)')\n" +
+ "z.registerHook('post_exec', 'print(2)')\n" +
+ "z.registerNoteHook('pre_exec', 'print(3)', '" + note.getId() + "')\n" +
+ "z.registerNoteHook('post_exec', 'print(4)', '" + note.getId() + "')\n");
+
+ Paragraph p2 = note.addNewParagraph(anonymous);
+ p2.setText("%python print(5)");
+
+ note.run(p1.getId(), true);
+ note.run(p2.getId(), true);
+
+ assertEquals(Status.FINISHED, p1.getStatus());
+ assertEquals(Status.FINISHED, p2.getStatus());
+ assertEquals("1\n3\n5\n4\n2\n", p2.getResult().message().get(0).getData());
+
+ Note note2 = ZeppelinServer.notebook.createNote(anonymous);
+ Paragraph p3 = note2.addNewParagraph(anonymous);
+ p3.setText("%python print(6)");
+ note2.run(p3.getId(), true);
+ assertEquals("1\n6\n2\n", p3.getResult().message().get(0).getData());
+ }
+
+ @Test
+ public void pySparkDepLoaderTest() throws IOException {
Note note = ZeppelinServer.notebook.createNote(anonymous);
// restart spark interpreter to make dep loader work