You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by al...@apache.org on 2020/06/14 06:47:10 UTC
[zeppelin] branch master updated: [ZEPPELIN-4870][hotfix] fix edge
case, and remove duplicate parser
This is an automated email from the ASF dual-hosted git repository.
alexott 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 d679bb1 [ZEPPELIN-4870][hotfix] fix edge case, and remove duplicate parser
d679bb1 is described below
commit d679bb1b8933367ffd40abd413d3bc291feb5188
Author: Alex Ott <al...@gmail.com>
AuthorDate: Fri Jun 12 16:04:31 2020 +0200
[ZEPPELIN-4870][hotfix] fix edge case, and remove duplicate parser
### What is this PR for?
Tests didn't cover all of the edge cases, that lead to error when parsing specific message.
Also, was found that parsing is also duplicated in the `Paragraph` class - it was replaced with call to `ParagraphTextParser`.
The most visible change is that `%interpreter(some text` will know fail as there is no matching `)` character. But it's works the same if we change it to `%interpreter (some text`...
### What type of PR is it?
Hot Fix
### What is the Jira issue?
* ZEPPELIN-4870
### How should this be tested?
* Added one more test
* https://travis-ci.org/github/alexott/zeppelin/builds/697944179
Author: Alex Ott <al...@gmail.com>
Closes #3800 from alexott/ZEPPELIN-4870-hotfix and squashes the following commits:
5942c5d35 [Alex Ott] [ZEPPELIN-4870][hotfix] fix edge case, and remove duplicate parser
---
.../org/apache/zeppelin/notebook/Paragraph.java | 38 +++-------------------
.../zeppelin/notebook/ParagraphTextParser.java | 15 ++++++---
.../apache/zeppelin/notebook/ParagraphTest.java | 2 +-
.../zeppelin/notebook/ParagraphTextParserTest.java | 12 +++++++
4 files changed, 27 insertions(+), 40 deletions(-)
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 3d70eb6..00cc926 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
@@ -194,40 +194,10 @@ public class Paragraph extends JobWithProgressPoller<InterpreterResult> implemen
// parse text to get interpreter component
if (this.text != null) {
// clean localProperties, otherwise previous localProperties will be used for the next run
- this.localProperties.clear();
- Matcher matcher = REPL_PATTERN.matcher(this.text);
- if (matcher.matches()) {
- String headingSpace = matcher.group(1);
- setIntpText(matcher.group(2));
-
- if (matcher.groupCount() == 3 && matcher.group(3) != null) {
- String localPropertiesText = matcher.group(3);
- String[] splits = localPropertiesText.substring(1, localPropertiesText.length() -1)
- .split(",");
- for (String split : splits) {
- String[] kv = split.split("=");
- if (StringUtils.isBlank(split) || kv.length == 0) {
- continue;
- }
- if (kv.length > 2) {
- throw new RuntimeException("Invalid paragraph properties format: " + split);
- }
- if (kv.length == 1) {
- localProperties.put(kv[0].trim(), kv[0].trim());
- } else {
- localProperties.put(kv[0].trim(), kv[1].trim());
- }
- }
- this.scriptText = this.text.substring(headingSpace.length() + intpText.length() +
- localPropertiesText.length() + 1).trim();
- } else {
- this.scriptText = this.text.substring(headingSpace.length() + intpText.length() + 1).trim();
- }
- config.putAll(localProperties);
- } else {
- setIntpText("");
- this.scriptText = this.text.trim();
- }
+ ParagraphTextParser.ParseResult result = ParagraphTextParser.parse(this.text);
+ localProperties = result.getLocalProperties();
+ setIntpText(result.getIntpText());
+ this.scriptText = result.getScriptText();
}
}
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/ParagraphTextParser.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/ParagraphTextParser.java
index f4742b4..59c990e 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/ParagraphTextParser.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/ParagraphTextParser.java
@@ -122,6 +122,10 @@ public class ParagraphTextParser {
if (insideQuotes) {
sb.append(ch);
} else {
+ if (!parseKey) {
+ throw new RuntimeException(
+ "Invalid paragraph properties format");
+ }
propKey = sb.toString().trim();
sb.delete(0, sb.length());
parseKey = false;
@@ -141,10 +145,10 @@ public class ParagraphTextParser {
} else {
localProperties.put(propKey, sb.toString().trim());
}
+ propKey = null;
+ parseKey = true;
+ sb.delete(0, sb.length());
}
- propKey = null;
- parseKey = true;
- sb.delete(0, sb.length());
break;
}
default:
@@ -169,14 +173,15 @@ public class ParagraphTextParser {
String headingSpace = matcher.group(1);
intpText = matcher.group(2);
int startPos = headingSpace.length() + intpText.length() + 1;
- if (text.charAt(startPos) == '(') {
+ if (startPos < text.length() && text.charAt(startPos) == '(') {
startPos = parseLocalProperties(text, startPos, localProperties);
}
scriptText = text.substring(startPos).trim();
} else {
intpText = "";
- scriptText = text;
+ scriptText = text.trim();
}
+
return new ParseResult(intpText, scriptText, localProperties);
}
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 5dd6301..bc05f7a 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
@@ -77,7 +77,7 @@ public class ParagraphTest extends AbstractInterpreterTest {
public void scriptBodyWithReplName() {
Note note = createNote();
Paragraph paragraph = new Paragraph(note, null);
- paragraph.setText("%test(1234567");
+ paragraph.setText("%test (1234567");
assertEquals("test", paragraph.getIntpText());
assertEquals("(1234567", paragraph.getScriptText());
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTextParserTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTextParserTest.java
index 4636105..f3ef227 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTextParserTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTextParserTest.java
@@ -34,6 +34,18 @@ public class ParagraphTextParserTest {
assertEquals("", parseResult.getScriptText());
}
+
+ @Test
+ public void testCassandra() {
+ ParagraphTextParser.ParseResult parseResult = ParagraphTextParser.parse(
+ "%cassandra(locale=ru_RU, timeFormat=\"E, d MMM yy\", floatPrecision = 5, output=cql)\n"
+ + "select * from system_auth.roles;");
+ assertEquals("cassandra", parseResult.getIntpText());
+ assertEquals(4, parseResult.getLocalProperties().size());
+ assertEquals("E, d MMM yy", parseResult.getLocalProperties().get("timeFormat"));
+ assertEquals("select * from system_auth.roles;", parseResult.getScriptText());
+ }
+
@Test
public void testParagraphTextLocalPropertiesAndText() {
ParagraphTextParser.ParseResult parseResult = ParagraphTextParser.parse("%spark.pyspark(pool=pool_1) sc.version");