Skip to content

[ZEPPELIN-4870] Improve parsing of the paragraph properties#3799

Closed
alexott wants to merge 1 commit intoapache:masterfrom
alexott:ZEPPELIN-4870
Closed

[ZEPPELIN-4870] Improve parsing of the paragraph properties#3799
alexott wants to merge 1 commit intoapache:masterfrom
alexott:ZEPPELIN-4870

Conversation

@alexott
Copy link
Contributor

@alexott alexott commented Jun 11, 2020

What is this PR for?

We can provide properties that are local to the paragraph, that could be used to pass an additional information for interpreter that could affect its behavior. Unfortunately existing parsing functionality relies on the fact that key/value pairs need to be separated by , character, and doesn't handle values with special characters (,, =, ...) inside, like this:

%cassandra(locale=ruRU, timeFormat="E, d MMM yy", floatPrecision = 5, outputFormat=cql)

This PR changes the parsing logic to perform character-by-character parsing, and handling of the quoted keys & values, escaping of the special characters, etc.

What type of PR is it?

Improvement

What is the Jira issue?

How should this be tested?

localProperties.put(kv[0].trim(), kv[0].trim());
} else {
localProperties.put(kv[0].trim(), kv[1].trim());
int startPos = headingSpace.length() + intpText.length() + 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the local property extraction logics into a separate function for more readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zjffdu done. If you ok with overall change, I can push it to master, and rework my changes regarding formatting in Cassandra...

@zjffdu
Copy link
Contributor

zjffdu commented Jun 12, 2020

Overall LGTM, one last question about this PR, what happens in frontend if user specify invalid properties ?

@alexott
Copy link
Contributor Author

alexott commented Jun 12, 2020

Overall LGTM, one last question about this PR, what happens in frontend if user specify invalid properties ?

Interpreter should throw an exception... The parsing itself just do the general checks that syntax is correct etc., the rest is responsibility of the interpreter - we'll generalize common properties, like, time format, etc. over the time - I'll file a JIRA for it.

@zjffdu
Copy link
Contributor

zjffdu commented Jun 12, 2020

Would the frontend display the proper error message ?

@alexott
Copy link
Contributor Author

alexott commented Jun 12, 2020

Should be, at least I saw them in current implementation of Cassandra interpreter, when I specified incorrect time format, etc.

@zjffdu
Copy link
Contributor

zjffdu commented Jun 12, 2020

Thanks for the confirmation @alexott , +1 from me

@asfgit asfgit closed this in 1d1b058 Jun 12, 2020
asfgit pushed a commit that referenced this pull request Jun 12, 2020
### What is this PR for?

We can provide properties that are local to the paragraph, that could be used to pass an additional information for interpreter that could affect its behavior.  Unfortunately existing parsing functionality relies on the fact that key/value pairs need to be separated by `,` character, and doesn't handle values with special characters (`,`, `=`, ...) inside, like this:

```
%cassandra(locale=ruRU, timeFormat="E, d MMM yy", floatPrecision = 5, outputFormat=cql)
```

This PR changes the parsing logic to perform character-by-character parsing, and handling of the quoted keys & values, escaping of the special characters, etc.

### What type of PR is it?

Improvement

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-4870

### How should this be tested?
* https://travis-ci.org/github/alexott/zeppelin/builds/697522260
* additional unit tests were added

Author: Alex Ott <alexott@gmail.com>

Closes #3799 from alexott/ZEPPELIN-4870 and squashes the following commits:

5fb6ee8 [Alex Ott] [ZEPPELIN-4870] Improve parsing of the paragraph properties

(cherry picked from commit 1d1b058)
Signed-off-by: Alex Ott <alexott@apache.org>
@alexott alexott deleted the ZEPPELIN-4870 branch June 18, 2020 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants