Skip to content

ZEPPELIN-3179. Improve error message when IPython is not available#2736

Closed
zjffdu wants to merge 1 commit intoapache:masterfrom
zjffdu:ZEPPELIN-3179
Closed

ZEPPELIN-3179. Improve error message when IPython is not available#2736
zjffdu wants to merge 1 commit intoapache:masterfrom
zjffdu:ZEPPELIN-3179

Conversation

@zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Jan 21, 2018

What is this PR for?

Minor change to improve the error message when IPython is not available. See the following screenshot.

What type of PR is it?

[ Improvement ]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

  • Manually
    screen shot 2018-01-20 at 5 00 25 pm
    screen shot 2018-01-20 at 5 00 19 pm

verified

Screenshots (if appropriate)

Questions:

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

@zjffdu
Copy link
Contributor Author

zjffdu commented Jan 21, 2018

@felixcheung @prabhjyotsingh Could you help review it ? Thanks

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

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

LG except one generic exception path

return true;
} catch (Exception e) {
LOGGER.warn("Fail to checkIPythonPrerequisite", e);
return false;
Copy link
Member

Choose a reason for hiding this comment

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

if there's an exception we should return a generic error string here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

if (!freezeOutput.contains("grpcio=")) {
InterpreterContext.get().out.write("grpcio is not installed\n".getBytes());
return false;
return "grpcio is not installed";
Copy link
Member

Choose a reason for hiding this comment

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

are these return strings going to trace log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it would be logged as well.

@zjffdu
Copy link
Contributor Author

zjffdu commented Jan 24, 2018

More if no more comments

@asfgit asfgit closed this in 8ad8c7d Jan 24, 2018
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