Repository: zeppelin Updated Branches: refs/heads/master 74c034edd -> ca27bf5c1
ZEPPELIN-999 Support alias for JDBC properties ### What is this PR for? In case of using JdbcInterpreter, you should use %jdbc(prefix) if you set multiple configurations. This PR makes you use %prefix only. ### What type of PR is it? [Improvement] ### Todos * [x] - Change %prefix to %jdbc(prefix) during running paragraph ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-999 ### How should this be tested? ### Screenshots (if appropriate) <img width="906" alt="screen shot 2016-06-15 at 12 42 32 am" src="https://cloud.githubusercontent.com/assets/3612566/16049304/25db79f6-3292-11e6-876a-287bbbc50f50.png";> <img width="886" alt="screen shot 2016-06-15 at 12 42 49 am" src="https://cloud.githubusercontent.com/assets/3612566/16049313/31c2097e-3292-11e6-8c91-13d71360f25f.png";> ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Jongyoul Lee <[email protected]> Closes #1012 from jongyoul/ZEPPELIN-999 and squashes the following commits: 0774cca [Jongyoul Lee] Fixed noteTest 6d0293f [Jongyoul Lee] - Added some test cases 37c4810 [Jongyoul Lee] - Fixed some exception to returning null - Added effective text to interpret it actually - Made ZeppelinConfiguration transient 4ca7d81 [Jongyoul Lee] Added logic to change from %property to %jdbc(property) Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/ca27bf5c Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/ca27bf5c Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/ca27bf5c Branch: refs/heads/master Commit: ca27bf5c11ad29070eb392f04ea4867d992313fa Parents: 74c034e Author: Jongyoul Lee <[email protected]> Authored: Wed Jun 15 01:10:20 2016 +0900 Committer: Jongyoul Lee <[email protected]> Committed: Thu Jun 16 14:56:07 2016 +0900 ---------------------------------------------------------------------- .../zeppelin/conf/ZeppelinConfiguration.java | 9 +- .../java/org/apache/zeppelin/notebook/Note.java | 15 +++- .../notebook/NoteInterpreterLoader.java | 2 +- .../org/apache/zeppelin/notebook/Paragraph.java | 16 +++- .../org/apache/zeppelin/notebook/NoteTest.java | 94 ++++++++++++++++++++ .../apache/zeppelin/notebook/ParagraphTest.java | 25 ++++++ 6 files changed, 153 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca27bf5c/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java index 5094901..a7d4498 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java @@ -71,7 +71,7 @@ public class ZeppelinConfiguration extends XMLConfiguration { *url = ZeppelinConfiguration.class.getResource(ZEPPELIN_SITE_XML); * @throws ConfigurationException */ - public static ZeppelinConfiguration create() { + public static synchronized ZeppelinConfiguration create() { if (conf != null) { return conf; } @@ -415,6 +415,10 @@ public class ZeppelinConfiguration extends XMLConfiguration { return getString(ConfVars.ZEPPELIN_WEBSOCKET_MAX_TEXT_MESSAGE_SIZE); } + public boolean getUseJdbcAlias() { + return getBoolean(ConfVars.ZEPPELIN_USE_JDBC_ALIAS); + } + public Map<String, String> dumpConfigurations(ZeppelinConfiguration conf, ConfigurationKeyPredicate predicate) { Map<String, String> configurations = new HashMap<>(); @@ -535,7 +539,8 @@ public class ZeppelinConfiguration extends XMLConfiguration { ZEPPELIN_ALLOWED_ORIGINS("zeppelin.server.allowed.origins", "*"), ZEPPELIN_ANONYMOUS_ALLOWED("zeppelin.anonymous.allowed", true), ZEPPELIN_CREDENTIALS_PERSIST("zeppelin.credentials.persist", true), - ZEPPELIN_WEBSOCKET_MAX_TEXT_MESSAGE_SIZE("zeppelin.websocket.max.text.message.size", "1024000"); + ZEPPELIN_WEBSOCKET_MAX_TEXT_MESSAGE_SIZE("zeppelin.websocket.max.text.message.size", "1024000"), + ZEPPELIN_USE_JDBC_ALIAS("zeppelin.use.jdbc.alias", true); private String varName; http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca27bf5c/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java index 80f2d70..d2feb04 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java @@ -25,6 +25,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.display.AngularObject; import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.display.Input; @@ -63,6 +64,8 @@ public class Note implements Serializable, JobListener { private String name = ""; private String id; + private transient ZeppelinConfiguration conf = ZeppelinConfiguration.create(); + @SuppressWarnings("rawtypes") Map<String, List<AngularObject>> angularObjects = new HashMap<>(); @@ -407,9 +410,17 @@ public class Note implements Serializable, JobListener { Paragraph p = getParagraph(paragraphId); p.setNoteReplLoader(replLoader); p.setListener(jobListenerFactory.getParagraphJobListener(this)); - Interpreter intp = replLoader.get(p.getRequiredReplName()); + String requiredReplName = p.getRequiredReplName(); + Interpreter intp = replLoader.get(requiredReplName); if (intp == null) { - throw new InterpreterException("Interpreter " + p.getRequiredReplName() + " not found"); + // TODO(jongyoul): Make "%jdbc" configurable from JdbcInterpreter + if (conf.getUseJdbcAlias() && null != (intp = replLoader.get("jdbc"))) { + String pText = p.getText().replaceFirst(requiredReplName, "jdbc(" + requiredReplName + ")"); + logger.debug("New paragraph: {}", pText); + p.setEffectiveText(pText); + } else { + throw new InterpreterException("Interpreter " + requiredReplName + " not found"); + } } if (p.getConfig().get("enabled") == null || (Boolean) p.getConfig().get("enabled")) { intp.getScheduler().submit(p); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca27bf5c/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java index b1719c5..d2a1e84 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java @@ -188,6 +188,6 @@ public class NoteInterpreterLoader { } } - throw new InterpreterException(replName + " interpreter not found"); + return null; } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca27bf5c/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java index 36d466b..86af539 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java @@ -51,6 +51,7 @@ public class Paragraph extends Job implements Serializable, Cloneable { private transient NoteInterpreterLoader replLoader; private transient Note note; private transient AuthenticationInfo authenticationInfo; + private transient String effectiveText; String title; String text; @@ -106,6 +107,14 @@ public class Paragraph extends Job implements Serializable, Cloneable { this.dateUpdated = new Date(); } + public void setEffectiveText(String effectiveText) { + this.effectiveText = effectiveText; + } + + public String getEffectiveText() { + return effectiveText; + } + public AuthenticationInfo getAuthenticationInfo() { return authenticationInfo; } @@ -137,7 +146,7 @@ public class Paragraph extends Job implements Serializable, Cloneable { } public String getRequiredReplName() { - return getRequiredReplName(text); + return getRequiredReplName(null != effectiveText ? effectiveText : text); } public static String getRequiredReplName(String text) { @@ -165,8 +174,8 @@ public class Paragraph extends Job implements Serializable, Cloneable { } } - private String getScriptBody() { - return getScriptBody(text); + public String getScriptBody() { + return getScriptBody(null != effectiveText ? effectiveText : text); } public static String getScriptBody(String text) { @@ -295,6 +304,7 @@ public class Paragraph extends Job implements Serializable, Cloneable { } } finally { InterpreterContext.remove(); + effectiveText = null; } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca27bf5c/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java new file mode 100644 index 0000000..33a7ef2 --- /dev/null +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java @@ -0,0 +1,94 @@ +/* + * 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.notebook; + +import org.apache.zeppelin.interpreter.Interpreter; +import org.apache.zeppelin.notebook.repo.NotebookRepo; +import org.apache.zeppelin.scheduler.Scheduler; +import org.apache.zeppelin.search.SearchService; +import org.apache.zeppelin.user.Credentials; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +@RunWith(MockitoJUnitRunner.class) +public class NoteTest { + @Mock + NotebookRepo repo; + + @Mock + NoteInterpreterLoader replLoader; + + @Mock + JobListenerFactory jobListenerFactory; + + @Mock + SearchService index; + + @Mock + Credentials credentials; + + @Mock + Interpreter interpreter; + + @Mock + Scheduler scheduler; + + @Test + public void runNormalTest() { + when(replLoader.get("spark")).thenReturn(interpreter); + when(interpreter.getScheduler()).thenReturn(scheduler); + + String pText = "%spark sc.version"; + Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + Paragraph p = note.addParagraph(); + p.setText(pText); + note.run(p.getId()); + + ArgumentCaptor<Paragraph> pCaptor = ArgumentCaptor.forClass(Paragraph.class); + verify(scheduler, only()).submit(pCaptor.capture()); + verify(replLoader, only()).get("spark"); + + assertEquals("Paragraph text", pText, pCaptor.getValue().getText()); + } + + @Test + public void runJdbcTest() { + when(replLoader.get("mysql")).thenReturn(null); + when(replLoader.get("jdbc")).thenReturn(interpreter); + when(interpreter.getScheduler()).thenReturn(scheduler); + + String pText = "%mysql show databases"; + Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + Paragraph p = note.addParagraph(); + p.setText(pText); + note.run(p.getId()); + + ArgumentCaptor<Paragraph> pCaptor = ArgumentCaptor.forClass(Paragraph.class); + verify(scheduler, only()).submit(pCaptor.capture()); + verify(replLoader, times(2)).get(anyString()); + + assertEquals("Change paragraph text", "%jdbc(mysql) show databases", pCaptor.getValue().getEffectiveText()); + assertEquals("Change paragraph text", pText, pCaptor.getValue().getText()); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca27bf5c/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java index e08fdf8..833eef3 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java @@ -27,6 +27,7 @@ import org.apache.zeppelin.display.AngularObject; import org.apache.zeppelin.display.AngularObjectBuilder; import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.display.Input; +import org.apache.zeppelin.interpreter.Interpreter; import org.junit.Test; import java.util.HashMap; @@ -70,6 +71,30 @@ public class ParagraphTest { } @Test + public void effectiveTextTest() { + NoteInterpreterLoader noteInterpreterLoader = mock(NoteInterpreterLoader.class); + Interpreter interpreter = mock(Interpreter.class); + + Paragraph p = new Paragraph(null, null, null, noteInterpreterLoader); + p.setText("%h2 show databases"); + p.setEffectiveText("%jdbc(h2) show databases"); + assertEquals("Get right replName", "jdbc", p.getRequiredReplName()); + assertEquals("Get right scriptBody", "(h2) show databases", p.getScriptBody()); + + when(noteInterpreterLoader.get("jdbc")).thenReturn(interpreter); + when(interpreter.getFormType()).thenReturn(Interpreter.FormType.NATIVE); + + try { + p.jobRun(); + } catch (Throwable throwable) { + // Do nothing + } + + assertEquals("Erase effective Text", "h2", p.getRequiredReplName()); + assertEquals("Erase effective Text", "show databases", p.getScriptBody()); + } + + @Test public void should_extract_variable_from_angular_object_registry() throws Exception { //Given final String noteId = "noteId";
![https://cloud.githubusercontent.com/assets/3612566/16049304/25db79f6-3292-11e6-876a-287bbbc50f50.png"](https://cloud.githubusercontent.com/assets/3612566/16049304/25db79f6-3292-11e6-876a-287bbbc50f50.png"){kind=link}
![https://cloud.githubusercontent.com/assets/3612566/16049313/31c2097e-3292-11e6-8c91-13d71360f25f.png"](https://cloud.githubusercontent.com/assets/3612566/16049313/31c2097e-3292-11e6-8c91-13d71360f25f.png"){kind=link}