Skip to content

[ZEPPELIN-1488] JDBC Interpreter throws error while the interpreter is downloading dependencies #1467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ghost wants to merge 4 commits into from
Closed

[ZEPPELIN-1488] JDBC Interpreter throws error while the interpreter is downloading dependencies #1467

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4f7b938
code fix for ZEPPELIN-1488
raja-imaginea Sep 27, 2016
c412dd6
review fix
raja-imaginea Oct 10, 2016
ee98a84
checkstyle fix
raja-imaginea Oct 14, 2016
b069328
removed unnecessary method call
raja-imaginea Oct 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,13 @@ protected Object jobRun() throws Throwable {
logger.error("Can not find interpreter name " + repl);
throw new RuntimeException("Can not find interpreter for " + getRequiredReplName());
}

InterpreterSetting intp = getInterpreterSettingById(repl.getInterpreterGroup().getId());
while (intp.getStatus().equals(

@jongyoul jongyoul Oct 4, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh.. my... Github's new review system is weird. Sorry for confusing you. My question is at this point. this loop execute too often. If an interpreter is under the downloading progress, it will take a long time, usually. I think we need to change this loop to run slowly. How do you think?

@ghost ghost Oct 4, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Even i was thinking about the same while making this changes. Do you think we should have a thread that should execute this function for every 'n' milliseconds. Should 1000 milliseconds be good enough ?

Any thoughts would also be great

@jongyoul jongyoul Oct 4, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, I think it's enough to sleep for a while.

@ghost ghost Oct 10, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Please do check now

@ghost ghost Oct 17, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@jongyoul : Will changes be good enough now ?

org.apache.zeppelin.interpreter.InterpreterSetting.Status.DOWNLOADING_DEPENDENCIES)) {
Thread.sleep(200);

@jongyoul jongyoul Oct 17, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's not perfect but realistic solution for it.

intp = getInterpreterSettingById(repl.getInterpreterGroup().getId());

@jongyoul jongyoul Oct 17, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you get intp every loop? AFAIK, it has same reference, doesn't it?

@ghost ghost Oct 18, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for pointing out. Removed the unnecessary method call now

}
if (this.noteHasUser() && this.noteHasInterpreters()) {
InterpreterSetting intp = getInterpreterSettingById(repl.getInterpreterGroup().getId());
if (intp != null &&
interpreterHasUser(intp) &&
isUserAuthorizedToAccessInterpreter(intp.getOption()) == false) {
Expand Down