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 2022/04/13 01:42:07 UTC
[zeppelin] branch master updated: [ZEPPELIN-5699] Trim space in ConfInterpreter
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 4b269f88fc [ZEPPELIN-5699] Trim space in ConfInterpreter
4b269f88fc is described below
commit 4b269f88fc4a9d59a140b10260064a67382b6916
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Wed Mar 30 11:33:19 2022 +0800
[ZEPPELIN-5699] Trim space in ConfInterpreter
### What is this PR for?
Trim the space for the key & value of ConfInterpreter. Unit test is added.
### What type of PR is it?
[Bug Fix ]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-5669
### 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 #4336 from zjffdu/ZEPPELIN-5699 and squashes the following commits:
a4a471e1bf [Jeff Zhang] [ZEPPELIN-5699] Trim space in ConfInterpreter
---
.../zeppelin/interpreter/ConfInterpreter.java | 4 +-
.../zeppelin/interpreter/ConfInterpreterTest.java | 56 +++++++++++++++++++++-
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java
index f343790862..9c5d02debb 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java
@@ -67,7 +67,9 @@ public class ConfInterpreter extends Interpreter {
finalProperties.putAll(getProperties());
Properties newProperties = new Properties();
newProperties.load(new StringReader(st));
- finalProperties.putAll(newProperties);
+ for (String key : newProperties.stringPropertyNames()) {
+ finalProperties.put(key.trim(), newProperties.getProperty(key).trim());
+ }
LOGGER.debug("Properties for InterpreterGroup: {} is {}", interpreterGroupId, finalProperties);
interpreterSetting.setInterpreterGroupProperties(interpreterGroupId, finalProperties);
return new InterpreterResult(InterpreterResult.Code.SUCCESS);
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/ConfInterpreterTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/ConfInterpreterTest.java
index 41f5d73b00..3a8717ff37 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/ConfInterpreterTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/ConfInterpreterTest.java
@@ -21,8 +21,9 @@ import org.apache.zeppelin.interpreter.remote.RemoteInterpreter;
import org.junit.Before;
import org.junit.Test;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import java.util.Properties;
+
+import static org.junit.Assert.*;
public class ConfInterpreterTest extends AbstractInterpreterTest {
@@ -59,6 +60,57 @@ public class ConfInterpreterTest extends AbstractInterpreterTest {
assertEquals(InterpreterResult.Code.ERROR, result.code);
}
+ @Test
+ public void testPropertyTrim() throws InterpreterException {
+ assertTrue(interpreterFactory.getInterpreter("test.conf", executionContext) instanceof ConfInterpreter);
+ ConfInterpreter confInterpreter = (ConfInterpreter) interpreterFactory.getInterpreter("test.conf", executionContext);
+
+ InterpreterContext context = InterpreterContext.builder()
+ .setNoteId("noteId")
+ .setParagraphId("paragraphId")
+ .build();
+
+ // space before key and space after values
+ InterpreterResult result = confInterpreter.interpret(" property_1 \tnew_value \n new_property \t dummy_value \n", context);
+ assertEquals(InterpreterResult.Code.SUCCESS, result.code);
+
+ assertTrue(interpreterFactory.getInterpreter("test", executionContext) instanceof RemoteInterpreter);
+ RemoteInterpreter remoteInterpreter = (RemoteInterpreter) interpreterFactory.getInterpreter("test", executionContext);
+ remoteInterpreter.interpret("hello world", context);
+ Properties intpProperties = remoteInterpreter.getProperties();
+ // Total 7 properties,
+ // 3 built-in properties (zeppelin.interpreter.output.limit, zeppelin.interpreter.localRepo, zeppelin.interpreter.connection.poolsize)
+ assertEquals(7, intpProperties.size());
+ assertNotNull(intpProperties.getProperty("zeppelin.interpreter.output.limit"));
+ assertNotNull(intpProperties.getProperty("zeppelin.interpreter.localRepo"));
+ assertNotNull(intpProperties.getProperty("zeppelin.interpreter.connection.poolsize"));
+ assertEquals("new_value", intpProperties.getProperty("property_1"));
+ assertEquals("new_value_2", intpProperties.getProperty("property_2"));
+ assertEquals("value_3", intpProperties.getProperty("property_3"));
+ assertEquals("dummy_value", intpProperties.getProperty("new_property"));
+ }
+
+ @Test
+ public void testEmptyValue() throws InterpreterException {
+ ConfInterpreter confInterpreter = (ConfInterpreter) interpreterFactory.getInterpreter("test.conf", executionContext);
+
+ InterpreterContext context = InterpreterContext.builder()
+ .setNoteId("noteId")
+ .setParagraphId("paragraphId")
+ .build();
+
+ InterpreterResult result = confInterpreter.interpret(" property_1\t \n new_property\t \n", context);
+ assertEquals(result.toString(), InterpreterResult.Code.SUCCESS, result.code);
+
+ assertTrue(interpreterFactory.getInterpreter("test", executionContext) instanceof RemoteInterpreter);
+ RemoteInterpreter remoteInterpreter = (RemoteInterpreter) interpreterFactory.getInterpreter("test", executionContext);
+ remoteInterpreter.interpret("hello world", context);
+ assertEquals(7, remoteInterpreter.getProperties().size());
+ assertEquals("", remoteInterpreter.getProperty("property_1"));
+ assertEquals("", remoteInterpreter.getProperty("new_property"));
+ assertEquals("value_3", remoteInterpreter.getProperty("property_3"));
+ }
+
@Test
public void testEmptyConf() throws InterpreterException {
assertTrue(interpreterFactory.getInterpreter("test.conf", executionContext) instanceof ConfInterpreter);