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

Jump to bottom
Closed
ghost wants to merge 4 commits into from
Closed

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

ghost wants to merge 4 commits into from

Conversation

@ghost
Copy link

@ghost ghost commented Sep 28, 2016

What is this PR for?

For first time, when we add dependencies for JDBC interpreter, dependencies will start getting downloaded in background. During that time, if user runs a paragraph of JDBC interpreter now user getting error , But instead paragraph execution should be put on 'PENDING' state and wait for dependencies to get downloaded and then run the paragraph

What type of PR is it?

Bug Fix

Todos

What is the Jira issue?

[ZEPPELIN-1488] https://issues.apache.org/jira/browse/ZEPPELIN-1488

How should this be tested?

prerequisites:

  1. Any DB setup.(For my testing, i considered hive)
    Steps:
  2. Delete the local-repo folder under zeppelin project, if it exists
  3. Go to interpreter settings page, provide hive connection details under JDBC interprepreter
  4. For hive interpreter to run, it needs some dependencies to be added in interpreter settings page
    For hive below dependencies needs to added
    1. org.apache.hive:hive-jdbc:0.14.0
    2. org.apache.hadoop:hadoop-common:2.6.0
  5. Once the settings for JDBC interpreter is saved, dependencies will start getting downloaded in background.
  6. Run any paragraph with JDBC as interpreter, paragraph should not throw error, status of the paragraph should change to 'pending' while the dependencies are getting downloaded in background.
  7. Once the downloading of dependencies is done, the paragraph which were in pending will start executing in order depending on the execution mode of the interpreter (i.e Shared, Scoped, Isolated)

Screenshots (if appropriate)

Questions:

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

@ghost
Copy link
Author

ghost commented Sep 28, 2016

CI build is green.Please review

@ghost
Copy link
Author

ghost commented Oct 3, 2016

Ping

@jongyoul
Copy link
Member

jongyoul commented Oct 4, 2016

Can you answer my question?

@ghost
Copy link
Author

ghost commented Oct 4, 2016

i didn't get the question. Can you please share it again ?

@jongyoul
Copy link
Member

jongyoul commented Oct 4, 2016

I left in-line comment on your PR. Don't you see it?

jongyoul
jongyoul reviewed Oct 4, 2016
View reviewed changes
zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java
}

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

@jongyoul jongyoul Oct 4, 2016

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?

Copy link
Author

@ghost ghost Oct 4, 2016

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

Copy link
Member

@jongyoul jongyoul Oct 4, 2016

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.

Copy link
Author

@ghost ghost Oct 10, 2016

Choose a reason for hiding this comment

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

Please do check now

Copy link
Author

@ghost ghost Oct 17, 2016

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 ?

@jongyoul
Copy link
Member

jongyoul commented Oct 4, 2016

I've left my review in four days ago. But I misunderstood new Github's review system. I left my comment again as a singe comment. Can you see it?

@ghost
Copy link
Author

ghost commented Oct 4, 2016

Oh. Its' ok. now i see your comment.

@ghost
Copy link
Author

ghost commented Oct 14, 2016

ping

jongyoul
jongyoul reviewed Oct 17, 2016
View reviewed changes
zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java
InterpreterSetting intp = getInterpreterSettingById(repl.getInterpreterGroup().getId());
while (intp.getStatus().equals(
org.apache.zeppelin.interpreter.InterpreterSetting.Status.DOWNLOADING_DEPENDENCIES)) {
Thread.sleep(200);
Copy link
Member

@jongyoul jongyoul Oct 17, 2016

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.

jongyoul
jongyoul reviewed Oct 17, 2016
View reviewed changes
zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java Outdated
while (intp.getStatus().equals(
org.apache.zeppelin.interpreter.InterpreterSetting.Status.DOWNLOADING_DEPENDENCIES)) {
Thread.sleep(200);
intp = getInterpreterSettingById(repl.getInterpreterGroup().getId());
Copy link
Member

@jongyoul jongyoul Oct 17, 2016

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?

Copy link
Author

@ghost ghost Oct 18, 2016

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

@jongyoul
Copy link
Member

jongyoul commented Oct 20, 2016

LGTM. Merging there's no more discussion.

@asfgit asfgit closed this in 5cc7cc5 Oct 22, 2016
darionyaphet pushed a commit to darionyaphet/zeppelin that referenced this pull request Oct 27, 2016
@raja-imaginea @darionyaphet
[ZEPPELIN-1488] JDBC Interpreter throws error while the interpreter i…
f4265b9 
…s downloading dependencies

### What is this PR for?
For first time, when we add dependencies for JDBC interpreter, dependencies will start getting downloaded in background. During that time, if user runs a paragraph of JDBC interpreter now user getting error , But instead paragraph execution should be put on 'PENDING' state and wait for dependencies to get downloaded and then run the paragraph

### What type of PR is it?
Bug Fix

### Todos

### What is the Jira issue?
[ZEPPELIN-1488] https://issues.apache.org/jira/browse/ZEPPELIN-1488

### How should this be tested?
prerequisites:
1. Any DB setup.(For my testing, i considered hive)
Steps:
1. Delete the local-repo folder under zeppelin project, if it exists
2. Go to interpreter settings page, provide hive connection details under JDBC interprepreter
3. For hive interpreter to run, it needs some dependencies to be added in interpreter settings page
    For hive below dependencies needs to added
    1. org.apache.hive:hive-jdbc:0.14.0
    2. org.apache.hadoop:hadoop-common:2.6.0
4. Once the settings for JDBC interpreter is saved, dependencies will start getting downloaded in background.
5. Run any paragraph with JDBC as interpreter, paragraph should not throw error, status of the paragraph should change to 'pending' while the dependencies are getting downloaded in background.
6. Once the downloading of dependencies is done, the paragraph which were in pending will start executing in order depending on the execution mode of the interpreter (i.e Shared, Scoped, Isolated)

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: rajarajan-g <[email protected]>

Closes apache#1467 from rajarajan-g/ZEPPELIN-1488 and squashes the following commits:

b069328 [rajarajan-g] removed unnecessary method call
ee98a84 [rajarajan-g] checkstyle fix
c412dd6 [rajarajan-g] review fix
4f7b938 [rajarajan-g] code fix for ZEPPELIN-1488
pedrozatta pushed a commit to pedrozatta/zeppelin that referenced this pull request Oct 27, 2016
@raja-imaginea
[ZEPPELIN-1488] JDBC Interpreter throws error while the interpreter i…
b829ef9 
…s downloading dependencies

### What is this PR for?
For first time, when we add dependencies for JDBC interpreter, dependencies will start getting downloaded in background. During that time, if user runs a paragraph of JDBC interpreter now user getting error , But instead paragraph execution should be put on 'PENDING' state and wait for dependencies to get downloaded and then run the paragraph

### What type of PR is it?
Bug Fix

### Todos

### What is the Jira issue?
[ZEPPELIN-1488] https://issues.apache.org/jira/browse/ZEPPELIN-1488

### How should this be tested?
prerequisites:
1. Any DB setup.(For my testing, i considered hive)
Steps:
1. Delete the local-repo folder under zeppelin project, if it exists
2. Go to interpreter settings page, provide hive connection details under JDBC interprepreter
3. For hive interpreter to run, it needs some dependencies to be added in interpreter settings page
    For hive below dependencies needs to added
    1. org.apache.hive:hive-jdbc:0.14.0
    2. org.apache.hadoop:hadoop-common:2.6.0
4. Once the settings for JDBC interpreter is saved, dependencies will start getting downloaded in background.
5. Run any paragraph with JDBC as interpreter, paragraph should not throw error, status of the paragraph should change to 'pending' while the dependencies are getting downloaded in background.
6. Once the downloading of dependencies is done, the paragraph which were in pending will start executing in order depending on the execution mode of the interpreter (i.e Shared, Scoped, Isolated)

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: rajarajan-g <[email protected]>

Closes apache#1467 from rajarajan-g/ZEPPELIN-1488 and squashes the following commits:

b069328 [rajarajan-g] removed unnecessary method call
ee98a84 [rajarajan-g] checkstyle fix
c412dd6 [rajarajan-g] review fix
4f7b938 [rajarajan-g] code fix for ZEPPELIN-1488
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jongyoul @raja-imaginea