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 2017/04/28 10:02:27 UTC
zeppelin git commit: [ZEPPELIN-2216] general solution to precode.
refactoring jdbc precode
Repository: zeppelin
Updated Branches:
refs/heads/master d82f42def -> 2c504c44d
[ZEPPELIN-2216] general solution to precode. refactoring jdbc precode
### What is this PR for?
General solution to execute precode. Refactoring jdbc precode using general solution. Task contains to subtasks: executeAfterOpen, executeBeforeClose. executeBeforeClose not done because we need the context so there is a solution only for executeAfterOpen.
### What type of PR is it?
Feature | Refactoring
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-2216
### How should this be tested?
1. Add parameter zeppelin.PySparkInterpreter.precode `someVar='text'`
2. Execute
```
%pyspark
print(someVar)
```
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
Author: Tinkoff DWH <ti...@gmail.com>
Closes #2221 from tinkoff-dwh/ZEPPELIN-2216 and squashes the following commits:
1e3f3f7 [Tinkoff DWH] [ZEPPELIN-2216] fix path
e4cf72f [Tinkoff DWH] [ZEPPELIN-2216] added tests
5a482a0 [Tinkoff DWH] [ZEPPELIN-2216] fix tests
3977722 [Tinkoff DWH] Merge remote-tracking branch 'origin/master' into ZEPPELIN-2216
c0436a2 [Tinkoff DWH] [ZEPPELIN-2216] general solution to precode. refactoring jdbc precode
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/2c504c44
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/2c504c44
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/2c504c44
Branch: refs/heads/master
Commit: 2c504c44d9f87f383f23fad4075aeabcfec20585
Parents: d82f42d
Author: Tinkoff DWH <ti...@gmail.com>
Authored: Mon Apr 24 12:18:55 2017 +0500
Committer: Lee moon soo <mo...@apache.org>
Committed: Fri Apr 28 03:02:23 2017 -0700
----------------------------------------------------------------------
.../img/screenshots/interpreter_precode.png | Bin 0 -> 8327 bytes
docs/interpreter/jdbc.md | 2 +-
docs/manual/interpreters.md | 8 ++
.../apache/zeppelin/jdbc/JDBCInterpreter.java | 22 +++---
.../zeppelin/jdbc/JDBCInterpreterTest.java | 26 ++++---
.../zeppelin/interpreter/Interpreter.java | 13 ++++
.../interpreter/LazyOpenInterpreter.java | 8 +-
.../remote/RemoteInterpreterServer.java | 15 ++--
.../remote/RemoteInterpreterTest.java | 73 +++++++++++++++++++
9 files changed, 136 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/docs/assets/themes/zeppelin/img/screenshots/interpreter_precode.png
----------------------------------------------------------------------
diff --git a/docs/assets/themes/zeppelin/img/screenshots/interpreter_precode.png b/docs/assets/themes/zeppelin/img/screenshots/interpreter_precode.png
new file mode 100644
index 0000000..61b79c3
Binary files /dev/null and b/docs/assets/themes/zeppelin/img/screenshots/interpreter_precode.png differ
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/docs/interpreter/jdbc.md
----------------------------------------------------------------------
diff --git a/docs/interpreter/jdbc.md b/docs/interpreter/jdbc.md
index 2c1d2f7..f66ad6d 100644
--- a/docs/interpreter/jdbc.md
+++ b/docs/interpreter/jdbc.md
@@ -121,7 +121,7 @@ The JDBC interpreter properties are defined by default like below.
<tr>
<td>default.precode</td>
<td></td>
- <td>Some SQL which executes while opening connection</td>
+ <td>Some SQL which executes every time after initialization of the interpreter (see [Binding mode](../manual/interpreters.md#interpreter-binding-mode))</td>
</tr>
<tr>
<td>default.completer.schemaFilters</td>
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/docs/manual/interpreters.md
----------------------------------------------------------------------
diff --git a/docs/manual/interpreters.md b/docs/manual/interpreters.md
index 916d591..51b4991 100644
--- a/docs/manual/interpreters.md
+++ b/docs/manual/interpreters.md
@@ -113,3 +113,11 @@ interpreter.start()
The above code will start interpreter thread inside your process. Once the interpreter is started you can configure zeppelin to connect to RemoteInterpreter by checking **Connect to existing process** checkbox and then provide **Host** and **Port** on which interpreter process is listening as shown in the image below:
<img src="../assets/themes/zeppelin/img/screenshots/existing_interpreter.png" width="450px">
+
+## Precode
+
+Snippet of code (language of interpreter) that executes after initialization of the interpreter depends on [Binding mode](#interpreter-binding-mode). To configure add parameter with class of interpreter (`zeppelin.<ClassName>.precode`) except JDBCInterpreter ([JDBC precode](../interpreter/jdbc.md#usage-precode)).
+
+<img src="../assets/themes/zeppelin/img/screenshots/interpreter_precode.png" width="800px">
+
+
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
----------------------------------------------------------------------
diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
index cc2c55b..95b2a1a 100644
--- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
+++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
@@ -332,9 +332,6 @@ public class JDBCInterpreter extends Interpreter {
if (!getJDBCConfiguration(user).isConnectionInDBDriverPool(propertyKey)) {
createConnectionPool(url, user, propertyKey, properties);
- try (Connection connection = DriverManager.getConnection(jdbcDriver)) {
- executePrecode(connection, propertyKey);
- }
}
return DriverManager.getConnection(jdbcDriver);
}
@@ -572,18 +569,19 @@ public class JDBCInterpreter extends Interpreter {
return queries;
}
- private void executePrecode(Connection connection, String propertyKey) throws SQLException {
- String precode = getProperty(String.format(PRECODE_KEY_TEMPLATE, propertyKey));
- if (StringUtils.isNotBlank(precode)) {
- precode = StringUtils.trim(precode);
- logger.debug("Run SQL precode '{}'", precode);
- try (Statement statement = connection.createStatement()) {
- statement.execute(precode);
- if (!connection.getAutoCommit()) {
- connection.commit();
+ public InterpreterResult executePrecode(InterpreterContext interpreterContext) {
+ InterpreterResult interpreterResult = null;
+ for (String propertyKey : basePropretiesMap.keySet()) {
+ String precode = getProperty(String.format("%s.precode", propertyKey));
+ if (StringUtils.isNotBlank(precode)) {
+ interpreterResult = executeSql(propertyKey, precode, interpreterContext);
+ if (interpreterResult.code() != Code.SUCCESS) {
+ break;
}
}
}
+
+ return interpreterResult;
}
private InterpreterResult executeSql(String propertyKey, String sql,
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
----------------------------------------------------------------------
diff --git a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
index 8fbd805..7c2eef3 100644
--- a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
+++ b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
@@ -400,17 +400,18 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter {
properties.setProperty("default.url", getJdbcConnection());
properties.setProperty("default.user", "");
properties.setProperty("default.password", "");
- properties.setProperty(DEFAULT_PRECODE, "SET @testVariable=1");
+ properties.setProperty(DEFAULT_PRECODE, "create table test_precode (id int); insert into test_precode values (1);");
JDBCInterpreter jdbcInterpreter = new JDBCInterpreter(properties);
jdbcInterpreter.open();
+ jdbcInterpreter.executePrecode(interpreterContext);
- String sqlQuery = "select @testVariable";
+ String sqlQuery = "select *from test_precode";
InterpreterResult interpreterResult = jdbcInterpreter.interpret(sqlQuery, interpreterContext);
assertEquals(InterpreterResult.Code.SUCCESS, interpreterResult.code());
assertEquals(InterpreterResult.Type.TABLE, interpreterResult.message().get(0).getType());
- assertEquals("@TESTVARIABLE\n1\n", interpreterResult.message().get(0).getData());
+ assertEquals("ID\n1\n", interpreterResult.message().get(0).getData());
}
@Test
@@ -420,13 +421,15 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter {
properties.setProperty("default.url", getJdbcConnection());
properties.setProperty("default.user", "");
properties.setProperty("default.password", "");
- properties.setProperty(DEFAULT_PRECODE, "incorrect command");
+ properties.setProperty(DEFAULT_PRECODE, "select 1");
+ properties.setProperty("incorrect.driver", "org.h2.Driver");
+ properties.setProperty("incorrect.url", getJdbcConnection());
+ properties.setProperty("incorrect.user", "");
+ properties.setProperty("incorrect.password", "");
+ properties.setProperty(String.format(PRECODE_KEY_TEMPLATE, "incorrect"), "incorrect command");
JDBCInterpreter jdbcInterpreter = new JDBCInterpreter(properties);
jdbcInterpreter.open();
-
- String sqlQuery = "select 1";
-
- InterpreterResult interpreterResult = jdbcInterpreter.interpret(sqlQuery, interpreterContext);
+ InterpreterResult interpreterResult = jdbcInterpreter.executePrecode(interpreterContext);
assertEquals(InterpreterResult.Code.ERROR, interpreterResult.code());
assertEquals(InterpreterResult.Type.TEXT, interpreterResult.message().get(0).getType());
@@ -439,17 +442,18 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter {
properties.setProperty("anotherPrefix.url", getJdbcConnection());
properties.setProperty("anotherPrefix.user", "");
properties.setProperty("anotherPrefix.password", "");
- properties.setProperty(String.format(PRECODE_KEY_TEMPLATE, "anotherPrefix"), "SET @testVariable=2");
+ properties.setProperty(String.format(PRECODE_KEY_TEMPLATE, "anotherPrefix"), "create table test_precode_2 (id int); insert into test_precode_2 values (2);");
JDBCInterpreter jdbcInterpreter = new JDBCInterpreter(properties);
jdbcInterpreter.open();
+ jdbcInterpreter.executePrecode(interpreterContext);
- String sqlQuery = "(anotherPrefix) select @testVariable";
+ String sqlQuery = "(anotherPrefix) select *from test_precode_2";
InterpreterResult interpreterResult = jdbcInterpreter.interpret(sqlQuery, interpreterContext);
assertEquals(InterpreterResult.Code.SUCCESS, interpreterResult.code());
assertEquals(InterpreterResult.Type.TABLE, interpreterResult.message().get(0).getType());
- assertEquals("@TESTVARIABLE\n2\n", interpreterResult.message().get(0).getData());
+ assertEquals("ID\n2\n", interpreterResult.message().get(0).getData());
}
@Test
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/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 a327b55..e426d94 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
@@ -65,6 +65,19 @@ public abstract class Interpreter {
public abstract void close();
/**
+ * Run precode if exists.
+ */
+ @ZeppelinApi
+ public InterpreterResult executePrecode(InterpreterContext interpreterContext) {
+ String simpleName = this.getClass().getSimpleName();
+ String precode = getProperty(String.format("zeppelin.%s.precode", simpleName));
+ if (StringUtils.isNotBlank(precode)) {
+ return interpret(precode, interpreterContext);
+ }
+ return null;
+ }
+
+ /**
* Run code and return result, in synchronous way.
*
* @param st statements to run
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/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 f1cbef8..96f88ee 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
@@ -21,7 +21,6 @@ import java.net.URL;
import java.util.List;
import java.util.Properties;
-import org.apache.zeppelin.interpreter.remote.RemoteInterpreter;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.scheduler.Scheduler;
@@ -74,6 +73,11 @@ public class LazyOpenInterpreter
}
@Override
+ public InterpreterResult executePrecode(InterpreterContext interpreterContext) {
+ return intp.executePrecode(interpreterContext);
+ }
+
+ @Override
public void close() {
synchronized (intp) {
if (opened == true) {
@@ -157,7 +161,7 @@ public class LazyOpenInterpreter
public void setClassloaderUrls(URL [] urls) {
intp.setClassloaderUrls(urls);
}
-
+
@Override
public void registerHook(String noteId, String event, String cmd) {
intp.registerHook(noteId, event, cmd);
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/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 6c43813..8f40ec4 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
@@ -481,19 +481,24 @@ public class RemoteInterpreterServer
try {
InterpreterContext.set(context);
+ InterpreterResult result = null;
+
// Open the interpreter instance prior to calling interpret().
// This is necessary because the earliest we can register a hook
// is from within the open() method.
LazyOpenInterpreter lazy = (LazyOpenInterpreter) interpreter;
if (!lazy.isOpen()) {
lazy.open();
+ result = lazy.executePrecode(context);
}
- // Add hooks to script from registry.
- // Global scope first, followed by notebook scope
- processInterpreterHooks(null);
- processInterpreterHooks(context.getNoteId());
- InterpreterResult result = interpreter.interpret(script, context);
+ if (result == null || result.code() == Code.SUCCESS) {
+ // Add hooks to script from registry.
+ // Global scope first, followed by notebook scope
+ processInterpreterHooks(null);
+ processInterpreterHooks(context.getNoteId());
+ result = interpreter.interpret(script, context);
+ }
// data from context.out is prepended to InterpreterResult if both defined
context.out.flush();
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2c504c44/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
index 2914bb4..ffcb8d5 100644
--- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
@@ -171,6 +171,79 @@ public class RemoteInterpreterTest {
}
@Test
+ public void testExecuteIncorrectPrecode() throws TTransportException, IOException {
+ Properties p = new Properties();
+ p.put("zeppelin.MockInterpreterA.precode", "fail test");
+ intpGroup.put("note", new LinkedList<Interpreter>());
+
+ RemoteInterpreter intpA = createMockInterpreterA(p);
+
+ intpGroup.get("note").add(intpA);
+
+ intpA.setInterpreterGroup(intpGroup);
+
+ RemoteInterpreterProcess process = intpA.getInterpreterProcess();
+
+ intpA.open();
+
+ InterpreterResult result = intpA.interpret("1",
+ new InterpreterContext(
+ "note",
+ "id",
+ null,
+ "title",
+ "text",
+ new AuthenticationInfo(),
+ new HashMap<String, Object>(),
+ new GUI(),
+ new AngularObjectRegistry(intpGroup.getId(), null),
+ new LocalResourcePool("pool1"),
+ new LinkedList<InterpreterContextRunner>(), null));
+
+
+
+ intpA.close();
+ assertEquals(Code.ERROR, result.code());
+ }
+
+ @Test
+ public void testExecuteCorrectPrecode() throws TTransportException, IOException {
+ Properties p = new Properties();
+ p.put("zeppelin.MockInterpreterA.precode", "2");
+ intpGroup.put("note", new LinkedList<Interpreter>());
+
+ RemoteInterpreter intpA = createMockInterpreterA(p);
+
+ intpGroup.get("note").add(intpA);
+
+ intpA.setInterpreterGroup(intpGroup);
+
+ RemoteInterpreterProcess process = intpA.getInterpreterProcess();
+
+ intpA.open();
+
+ InterpreterResult result = intpA.interpret("1",
+ new InterpreterContext(
+ "note",
+ "id",
+ null,
+ "title",
+ "text",
+ new AuthenticationInfo(),
+ new HashMap<String, Object>(),
+ new GUI(),
+ new AngularObjectRegistry(intpGroup.getId(), null),
+ new LocalResourcePool("pool1"),
+ new LinkedList<InterpreterContextRunner>(), null));
+
+
+
+ intpA.close();
+ assertEquals(Code.SUCCESS, result.code());
+ assertEquals("1", result.message().get(0).getData());
+ }
+
+ @Test
public void testRemoteInterperterErrorStatus() throws TTransportException, IOException {
Properties p = new Properties();