zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From m...@apache.org
Subject zeppelin git commit: [ZEPPELIN-2216] general solution to precode. refactoring jdbc precode
Date Fri, 28 Apr 2017 10:02:27 GMT
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 <tinkoff.dwh@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 <tinkoff.dwh@gmail.com>
Authored: Mon Apr 24 12:18:55 2017 +0500
Committer: Lee moon soo <moon@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();
 


Mime
View raw message