This is an automated email from the ASF dual-hosted git repository.
zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 44a49fd [ZEPPELIN-4555]. paragraph title is disappear after I run the
paragraph
44a49fd is described below
commit 44a49fdf68ecc8d048f1cf026df4a74c792da7ad
Author: Jeff Zhang <zjf...@apache.org>
AuthorDate: Mon Jan 20 12:07:09 2020 +0800
[ZEPPELIN-4555]. paragraph title is disappear after I run the paragraph
### What is this PR for?
This PR is to fix the title disappear issue. The main change is in method
`mergeConfig` where I would always insert the config in
`interpreter-setting.json`
### What type of PR is it?
[Bug Fix ]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-4555
### 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 <zjf...@apache.org>
Closes #3609 from zjffdu/ZEPPELIN-4555 and squashes the following commits:
49af306ef [Jeff Zhang] [ZEPPELIN-4555]. Paragraph title is disappear after
I run the paragraph
---
.../zeppelin/interpreter/InterpreterSetting.java | 17 +++
.../org/apache/zeppelin/notebook/Paragraph.java | 125 +++------------------
.../org/apache/zeppelin/notebook/NoteTest.java | 15 +++
.../org/apache/zeppelin/notebook/NotebookTest.java | 46 ++++----
4 files changed, 74 insertions(+), 129 deletions(-)
diff --git
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
index 4ac73fb..156bcb6 100644
---
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
+++
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
@@ -85,6 +85,10 @@ public class InterpreterSetting {
"language", (Object) "text",
"editOnDblClick", false);
+ public static String PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE =
"runOnSelectionChange";
+ public static String PARAGRAPH_CONFIG_TITLE = "title";
+ public static String PARAGRAPH_CONFIG_CHECK_EMTPY = "checkEmpty";
+
private String id;
private String name;
// the original interpreter setting template name where it is created from
@@ -379,6 +383,19 @@ public class InterpreterSetting {
return null;
}
+ public Map<String, Object> getConfig(String className) {
+ Map<String, Object> configSetting = new HashMap<>();
+ for (InterpreterInfo intpInfo : interpreterInfos) {
+ if (className.equals(intpInfo.getClassName())) {
+ if (intpInfo.getConfig() != null) {
+ configSetting = intpInfo.getConfig();
+ }
+ break;
+ }
+ }
+ return configSetting;
+ }
+
public RecoveryStorage getRecoveryStorage() {
return recoveryStorage;
}
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 8a32214..e74a269 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
@@ -20,7 +20,6 @@ package org.apache.zeppelin.notebook;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
@@ -45,8 +44,6 @@ import org.apache.zeppelin.interpreter.Interpreter;
import org.apache.zeppelin.interpreter.Interpreter.FormType;
import org.apache.zeppelin.interpreter.InterpreterContext;
import org.apache.zeppelin.interpreter.InterpreterException;
-import org.apache.zeppelin.interpreter.InterpreterGroup;
-import org.apache.zeppelin.interpreter.InterpreterInfo;
import org.apache.zeppelin.interpreter.InterpreterNotFoundException;
import org.apache.zeppelin.interpreter.InterpreterOutput;
import org.apache.zeppelin.interpreter.InterpreterOutputListener;
@@ -55,7 +52,6 @@ import org.apache.zeppelin.interpreter.InterpreterResult.Code;
import org.apache.zeppelin.interpreter.InterpreterResultMessage;
import org.apache.zeppelin.interpreter.InterpreterResultMessageOutput;
import org.apache.zeppelin.interpreter.InterpreterSetting;
-import org.apache.zeppelin.interpreter.InterpreterSettingManager;
import org.apache.zeppelin.interpreter.ManagedInterpreterGroup;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
import org.apache.zeppelin.resource.ResourcePool;
@@ -81,7 +77,7 @@ public class Paragraph extends
JobWithProgressPoller<InterpreterResult> implemen
private static Logger LOGGER = LoggerFactory.getLogger(Paragraph.class);
private static Pattern REPL_PATTERN =
Pattern.compile("(\\s*)%([\\w\\.]+)(\\(.*?\\))?.*", Pattern.DOTALL);
-
+
private String title;
// text is composed of intpText and scriptText.
private String text;
@@ -97,7 +93,6 @@ public class Paragraph extends
JobWithProgressPoller<InterpreterResult> implemen
/************** Transient fields which are not serializabled into note json
**************/
private transient String intpText;
- private transient Boolean configSettingNeedUpdate = true;
private transient String scriptText;
private transient Interpreter interpreter;
private transient Note note;
@@ -109,12 +104,7 @@ public class Paragraph extends
JobWithProgressPoller<InterpreterResult> implemen
private Map<String, ParagraphRuntimeInfo> runtimeInfos = new HashMap<>();
private transient List<InterpreterResultMessage> outputBuffer = new
ArrayList<>();
- public static String PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE =
"runOnSelectionChange";
- private static boolean PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE_DEFAULT = true;
- public static String PARAGRAPH_CONFIG_TITLE = "title";
- private static boolean PARAGRAPH_CONFIG_TITLE_DEFAULT = false;
- public static String PARAGRAPH_CONFIG_CHECK_EMTPY = "checkEmpty";
- private static boolean PARAGRAPH_CONFIG_CHECK_EMTPY_DEFAULT = true;
+
@VisibleForTesting
Paragraph() {
@@ -174,9 +164,6 @@ public class Paragraph extends
JobWithProgressPoller<InterpreterResult> implemen
}
private void setIntpText(String newIntptext) {
- if (null == intpText || !this.intpText.equals(newIntptext)) {
- this.configSettingNeedUpdate = true;
- }
this.intpText = newIntptext;
}
@@ -351,66 +338,27 @@ public class Paragraph extends
JobWithProgressPoller<InterpreterResult> implemen
}
public boolean shouldSkipRunParagraph() {
- // Because the user can arbitrarily specify the paragraph's interpreter
- // So need to determine the configuration of the interpreter
- // and the secondary interpreter at runtime.
- Map<String, Object> intpConfig = this.config;
-
- if (!StringUtils.isBlank(intpText)) {
- String[] intpList = intpText.split("\\.");
- if (intpList.length > 0) {
- InterpreterSettingManager intpSettingManager =
note.getInterpreterSettingManager();
- String intpName = intpList[0];
- try {
- InterpreterSetting intpSetting =
intpSettingManager.getInterpreterSettingByName(intpName);
- String intpInfoName = intpName; // e.g %sh
-
- // e.g %sh.terminal
- if (intpList.length == 2) {
- intpInfoName = intpList[1];
- }
- InterpreterInfo interpreterInfo =
intpSetting.getInterpreterInfo(intpInfoName);
- if (null != interpreterInfo && null != interpreterInfo.getConfig()) {
- intpConfig = interpreterInfo.getConfig();
- }
- } catch (RuntimeException e) {
- LOGGER.error(e.getMessage(), e);
- }
- }
- }
-
- // check interpreter-setting.json `config.checkEmpty` is equal false
- Object configCheckEmpty = intpConfig.get(PARAGRAPH_CONFIG_CHECK_EMTPY);
- if (null != configCheckEmpty) {
- boolean checkEmtpy = PARAGRAPH_CONFIG_CHECK_EMTPY_DEFAULT;
- try {
- checkEmtpy = (boolean) configCheckEmpty;
- } catch (ClassCastException e) {
- LOGGER.error(e.getMessage(), e);
- } catch (Exception e) {
- LOGGER.error(e.getMessage(), e);
- }
- if (!checkEmtpy) {
- LOGGER.info("This interpreter config `interpreter-setting.json` set
config.{} = false",
- PARAGRAPH_CONFIG_CHECK_EMTPY);
- return false;
- }
- }
-
+ boolean checkEmptyConfig =
+ (Boolean)
config.getOrDefault(InterpreterSetting.PARAGRAPH_CONFIG_CHECK_EMTPY, true);
// don't skip paragraph when local properties is not empty.
// local properties can customize the behavior of interpreter. e.g.
%r.shiny(type=run)
- return Strings.isNullOrEmpty(scriptText) && localProperties.isEmpty();
+ return checkEmptyConfig && Strings.isNullOrEmpty(scriptText) &&
localProperties.isEmpty();
}
public boolean execute(boolean blocking) {
- if (shouldSkipRunParagraph()) {
- LOGGER.info("Skip to run blank paragraph. {}", getId());
- setStatus(Job.Status.FINISHED);
- return true;
- }
-
try {
this.interpreter = getBindedInterpreter();
+ InterpreterSetting interpreterSetting = ((ManagedInterpreterGroup)
+ interpreter.getInterpreterGroup()).getInterpreterSetting();
+ Map<String, Object> config
+ = interpreterSetting.getConfig(interpreter.getClassName());
+ mergeConfig(config);
+
+ if (shouldSkipRunParagraph()) {
+ LOGGER.info("Skip to run blank paragraph. {}", getId());
+ setStatus(Job.Status.FINISHED);
+ return true;
+ }
setStatus(Status.READY);
if (getConfig().get("enabled") == null || (Boolean)
getConfig().get("enabled")) {
@@ -541,24 +489,6 @@ public class Paragraph extends
JobWithProgressPoller<InterpreterResult> implemen
p.settings.setParams(settings.getParams());
}
- // After the paragraph is executed,
- // need to apply the paragraph to the configuration in the
- // `interpreter-setting.json` config
- if (this.configSettingNeedUpdate) {
- this.configSettingNeedUpdate = false;
- InterpreterSettingManager intpSettingManager
- = this.note.getInterpreterSettingManager();
- if (null != intpSettingManager) {
- InterpreterGroup intpGroup = interpreter.getInterpreterGroup();
- if (null != intpGroup && intpGroup instanceof
ManagedInterpreterGroup) {
- String name = ((ManagedInterpreterGroup)
intpGroup).getInterpreterSetting().getName();
- Map<String, Object> config
- = intpSettingManager.getConfigSetting(name);
- mergeConfig(config);
- }
- }
- }
-
return res;
} finally {
InterpreterContext.remove();
@@ -696,32 +626,9 @@ public class Paragraph extends
JobWithProgressPoller<InterpreterResult> implemen
// Need to delete the existing configuration of this paragraph,
// update with the specified interpreter configuration
public void mergeConfig(Map<String, Object> newConfig) {
- if (null == newConfig || 0 == newConfig.size()) {
- newConfig = getDefaultConfigSetting();
- }
-
- List<String> keysToRemove =
Arrays.asList(PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE,
- PARAGRAPH_CONFIG_TITLE, PARAGRAPH_CONFIG_CHECK_EMTPY);
- for (String removeKey : keysToRemove) {
- if ((false == newConfig.containsKey(removeKey))
- && (true == config.containsKey(removeKey))) {
- this.config.remove(removeKey);
- }
- }
-
this.config.putAll(newConfig);
}
- // default parameters of the interpreter
- private Map<String, Object> getDefaultConfigSetting() {
- Map<String, Object> config = new HashMap<>();
- config.put(PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE,
PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE_DEFAULT);
- config.put(PARAGRAPH_CONFIG_TITLE, PARAGRAPH_CONFIG_TITLE_DEFAULT);
- config.put(PARAGRAPH_CONFIG_CHECK_EMTPY,
PARAGRAPH_CONFIG_CHECK_EMTPY_DEFAULT);
-
- return config;
- }
-
public void setReturn(InterpreterResult value, Throwable t) {
setResult(value);
setException(t);
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
index 0c97726..2727389 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java
@@ -24,11 +24,14 @@ import org.apache.zeppelin.interpreter.Interpreter;
import org.apache.zeppelin.interpreter.InterpreterFactory;
import org.apache.zeppelin.interpreter.InterpreterNotFoundException;
import org.apache.zeppelin.interpreter.InterpreterResult;
+import org.apache.zeppelin.interpreter.InterpreterSetting;
import org.apache.zeppelin.interpreter.InterpreterSettingManager;
+import org.apache.zeppelin.interpreter.ManagedInterpreterGroup;
import org.apache.zeppelin.notebook.repo.NotebookRepo;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.apache.zeppelin.user.Credentials;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
@@ -57,6 +60,12 @@ public class NoteTest {
Interpreter interpreter;
@Mock
+ ManagedInterpreterGroup interpreterGroup;
+
+ @Mock
+ InterpreterSetting interpreterSetting;
+
+ @Mock
Scheduler scheduler;
List<NoteEventListener> noteEventListener = new ArrayList<>();
@@ -69,6 +78,12 @@ public class NoteTest {
private AuthenticationInfo anonymous = new AuthenticationInfo("anonymous");
+ @Before
+ public void setUp() {
+ when(interpreter.getInterpreterGroup()).thenReturn(interpreterGroup);
+
when(interpreterGroup.getInterpreterSetting()).thenReturn(interpreterSetting);
+ }
+
@Test
public void runNormalTest() throws InterpreterNotFoundException {
when(interpreterFactory.getInterpreter(anyString(), anyString(),
eq("spark"), anyString())).thenReturn(interpreter);
diff --git
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
index c010eb4..c5cfb8a 100644
---
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
+++
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
@@ -1108,12 +1108,12 @@ public class NotebookTest extends
AbstractInterpreterTest implements ParagraphJo
// create paragraphs
Paragraph p1 = note.addNewParagraph(anonymous);
Map<String, Object> config = p1.getConfig();
-
assertTrue(config.containsKey(Paragraph.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE));
- assertTrue(config.containsKey(Paragraph.PARAGRAPH_CONFIG_TITLE));
- assertTrue(config.containsKey(Paragraph.PARAGRAPH_CONFIG_CHECK_EMTPY));
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE),
false);
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_TITLE), true);
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_CHECK_EMTPY), false);
+
assertTrue(config.containsKey(InterpreterSetting.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE));
+ assertTrue(config.containsKey(InterpreterSetting.PARAGRAPH_CONFIG_TITLE));
+
assertTrue(config.containsKey(InterpreterSetting.PARAGRAPH_CONFIG_CHECK_EMTPY));
+
assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE),
false);
+ assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_TITLE), true);
+ assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_CHECK_EMTPY),
false);
// The config_test interpreter sets the default parameters
// in interpreter/config_test/interpreter-setting.json
@@ -1123,29 +1123,35 @@ public class NotebookTest extends
AbstractInterpreterTest implements ParagraphJo
// "checkEmpty": true
// },
p1.setText("%config_test sleep 1000");
- note.runAll(AuthenticationInfo.ANONYMOUS, false);
-
- // wait until first paragraph finishes and second paragraph starts
- while (p1.getStatus() != Status.FINISHED) Thread.yield();
+ p1.execute(true);
// Check if the config_test interpreter default parameter takes effect
LOGGER.info("p1.getConfig() = " + p1.getConfig());
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE),
false);
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_TITLE), true);
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_CHECK_EMTPY), false);
+
assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE),
false);
+ assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_TITLE), true);
+ assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_CHECK_EMTPY),
false);
// The mock1 interpreter does not set default parameters
p1.setText("%mock1 sleep 1000");
- note.runAll(AuthenticationInfo.ANONYMOUS, false);
+ p1.execute(true);
- // wait until first paragraph finishes and second paragraph starts
- while (p1.getStatus() != Status.FINISHED) Thread.yield();
+ // mock1 has no config setting in interpreter-setting.json, so keep the
previous config
+ LOGGER.info("changed intp p1.getConfig() = " + p1.getConfig());
+
assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE),
false);
+ assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_TITLE), true);
+ assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_CHECK_EMTPY),
false);
+
+ // user manually change config
+
p1.getConfig().put(InterpreterSetting.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE,
true);
+ p1.getConfig().put(InterpreterSetting.PARAGRAPH_CONFIG_TITLE, false);
+ p1.setText("%mock1 sleep 1000");
+ p1.execute(true);
- // Check if the mock1 interpreter parameter is updated
+ // manually config change take effect after execution
LOGGER.info("changed intp p1.getConfig() = " + p1.getConfig());
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE),
true);
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_TITLE), false);
- assertEquals(config.get(Paragraph.PARAGRAPH_CONFIG_CHECK_EMTPY), true);
+
assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_RUNONSELECTIONCHANGE),
true);
+ assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_TITLE), false);
+ assertEquals(config.get(InterpreterSetting.PARAGRAPH_CONFIG_CHECK_EMTPY),
false);
}
@Test
