Skip to content

[ZEPPELIN-1412] add support multiline for pythonErrorIn method on python interpreter#1407

Closed
cloverhearts wants to merge 1 commit intoapache:masterfrom
cloverhearts:dev/ZEPPELIN-1412
Closed

[ZEPPELIN-1412] add support multiline for pythonErrorIn method on python interpreter#1407
cloverhearts wants to merge 1 commit intoapache:masterfrom
cloverhearts:dev/ZEPPELIN-1412

Conversation

@cloverhearts
Copy link
Member

What is this PR for?

currently, has not support multiline exception text on python interpreter.
for example:

Exception: blabla

is error.

but

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
Exception: test exception

is sucess (now)

to resolve this issue.

What type of PR is it?

Bug Fix

Todos

  • modification pythonErrorIn method
  • add test case

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1412?jql=project%20%3D%20ZEPPELIN%20AND%20status%20%3D%20Open

How should this be tested?

added test case.

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

@bzz
Copy link
Member

bzz commented Sep 6, 2016

Thank you for improvements, @cloverhearts Does it work with both, python2 and python3 ?

Looks good to me.

Although as discussed under ZEPPELIN-1360 - this approach have performance implications and this code most probably is going to be removed under ZEPPELIN-1325.

Merging to master if there is no further discussions

@cloverhearts
Copy link
Member Author

cloverhearts commented Sep 6, 2016

@bzz
thank you.
This code works with both Python 3 and 2.
In this code, the part that related to the Python version does not exist except raise Exception() and print (" Exception ( "test exception"))entry in the test code.
Thank you for your answer.

boolean isError = false;
String[] outputMultiline = output.split("\n");
Matcher errorMatcher;
for (String row : outputMultiline) {
Copy link
Member

Choose a reason for hiding this comment

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

should it look at only the last line?

Copy link
Member Author

Choose a reason for hiding this comment

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

@felixcheung Sorry, Can you please explain in more detail?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry guys, I missed GH notifications on this discussion and merged it :\

@asfgit asfgit closed this in 66d5811 Sep 7, 2016
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.

3 participants