Skip to content

ZEPPELIN-1716. Error of interpreter not found is not propagated to fr…#1691

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

ZEPPELIN-1716. Error of interpreter not found is not propagated to fr…#1691
zjffdu wants to merge 1 commit intoapache:masterfrom
zjffdu:ZEPPELIN-1716

Conversation

@zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Nov 28, 2016

What is this PR for?

It is introduced in ZEPPELIN-1399, we call InterpreterFactory.getInterpreter before calling Paragraph.jobRun. This PR would ignore the InterpreterException in Paragraph.isValidIntepreter

What type of PR is it?

[Bug Fix | Improvement | Feature | Documentation | Hot Fix | Refactoring]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

Tested manually, see the following screenshot

Screenshots (if appropriate)

Before this PR, only see the following error in log, but not in frontend

ERROR [2016-11-28 15:16:47,378] ({qtp1471868639-17} NotebookServer.java[onMessage]:303) - Can't handle message
org.apache.zeppelin.interpreter.InterpreterException: spark.sparkr interpreter not found
    at org.apache.zeppelin.interpreter.InterpreterFactory.getInterpreter(InterpreterFactory.java:1302)
    at org.apache.zeppelin.notebook.Paragraph.isValidInterpreter(Paragraph.java:596)
    at org.apache.zeppelin.notebook.Paragraph.getMagic(Paragraph.java:586)
    at org.apache.zeppelin.socket.NotebookServer.runParagraph(NotebookServer.java:1337)
    at org.apache.zeppelin.socket.NotebookServer.onMessage(NotebookServer.java:226)
    at org.apache.zeppelin.socket.NotebookSocket.onWebSocketText(NotebookSocket.java:59)

After this PR
2016-11-28_1514

Questions:

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

@zjffdu zjffdu closed this Nov 28, 2016
@zjffdu zjffdu reopened this Nov 28, 2016
@zjffdu
Copy link
Contributor Author

zjffdu commented Nov 28, 2016

@Leemoonsoo Please help review.

@Leemoonsoo
Copy link
Member

Tested and LGTM. Thanks @zjffdu for the fix!
Merge to master if there're no further comments.

@asfgit asfgit closed this in 2cccfb9 Dec 1, 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.

2 participants