[ZEPPELIN-1143] Interpreter dependencies are not downloaded on zeppelin start #1155

prabhjyotsingh · 2016-07-09T05:59:21Z

What is this PR for?

While saving interpreter setting, if zeppelin server crashed/killed/restarted by user or there was some internet/download related (intermittent issue) because of which, or any other issue because of which dependencies were not downloaded.
In any such condition, when zeppelin is started/restarted, server should try and download dependencies.

What type of PR is it?

[Improvement]

What is the Jira issue?

ZEPPELIN-1143

SCREENSHOT

How should this be tested?

Put a dependency (say "org.apache.commons:commons-csv:1.1") in any of the interpreter.
from command line delete local-repo directory
restart zeppelin server

expectation is local-repo should be recreated with all the dependencies that were mentioned in any of the interpreters.

Questions:

Does the licenses files need update? n/a
Is there breaking changes for older versions? n/a
Does this needs documentation? n/a

… start

jongyoul · 2016-07-10T15:18:52Z

@prabhjyotsingh How to deal with the case of getting error while downloading deps? Just edit and save it again?

prabhjyotsingh · 2016-07-11T05:38:05Z

Yes, so far that is the only way we can download the dependency again.

ghost · 2016-07-11T12:32:23Z

LGTM

felixcheung · 2016-07-11T22:51:04Z

zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java

+            logger.error("Error while downloading repos for interpreter group :" +
+                intpSetting.getGroup(), e);
+          } catch (IOException e) {
+            logger.error("Error while downloading repos for interpreter group :" +


you could combine the two catch clauses? or I would suggest having a different text log message for the exception

prabhjyotsingh · 2016-07-14T04:40:02Z

Merging this if no more discussion.

jongyoul · 2016-07-14T08:34:53Z

LGTM

karuppayya · 2016-07-14T12:59:29Z

A similar fix was made with https://github.com/apache/zeppelin/pull/755/files
Should we attempt to try to download dependencies when the interpreter process starts(as mentioned in the conversation).?

karuppayya · 2016-07-14T13:02:33Z

zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java

+          }
+        }
+      };
+      t.start();


Should this be synchronous?
If there are number of deps, the download might take a longer time. But the interpreter will be ready to take the paras.
If the code executed depends on these deps, it will fail I guess(until the time it is downloaded). And pass again after it is downloaded. Which will be kind of unexpected behaviour.

jongyoul · 2016-07-14T13:09:48Z

I also think @karup1990's way is proper.

karuppayya · 2016-07-14T17:15:40Z

zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java

          setting.getGroup(), setting.getInterpreterInfos(), setting.getProperties(),
          setting.getDependencies(), setting.getOption());

+      try {
+        synchronized (interpreterSettings) {
+          loadInterpreterDependencies(intpSetting);


The issue that i faced when loading interpreters synchronously:
The dependencies can take some time(time to download from mvn , erroneous scenarios etc). This will block loading other interpreters(since it is load method).

But the Notebook websocket server is running. The notebooks web UI will render properly. But when a para is run, the UI will seems to not respond(the para status will not change), because the interpreter might not have got loaded or in the process of being loaded.

It is good to show some UI feedback in these cases.
@prabhjyotsingh

# Conflicts: # zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java

prabhjyotsingh · 2016-07-21T14:00:35Z

@karup1990 have tried to implement the behaviour that you were asking about, can you take a look. Have added a GIF in PR description.

karuppayya · 2016-07-21T14:11:48Z

Thanks for addressing the feedbacks @prabhjyotsingh ..
I cannot see the GIF In the description of the PR .

karuppayya · 2016-07-21T14:13:56Z

zeppelin-web/src/app/interpreter/interpreter.controller.js

+      if (isDownloading) {
+        $timeout(function() {
+          getInterpreterSettings();
+        }, 2000);


This timeout period will vary based on the number of dependencies.
Should we have Zeppelin broadcast a message when it is done with the downloading?

Yes I thought about it, but I'll take care of this in another PR, where instead of doing a get call, migrate this web-sockets.

prabhjyotsingh · 2016-07-21T14:20:37Z

Have uploaded the GIF again, I think it would have failed last time.

karuppayya · 2016-07-22T04:52:33Z

zeppelin-web/src/app/interpreter/interpreter.controller.js

@@ -26,6 +26,7 @@ angular.module('zeppelinWebApp').controller('InterpreterCtrl',
    var getInterpreterSettings = function() {
      $http.get(baseUrlSrv.getRestApiBase() + '/interpreter/setting').success(function(data, status, headers, config) {
        $scope.interpreterSettings = data.body;
+        checkDownloadingDependencies();


This loop getInterpreterSettings<> checkDownloadingDependencies(this in turn iterates through setting) will be executed until the dependencies are downloaded.
Should be fine?
Otherwise, lgtm

Have added another condition that will check for route url, and if it is not /interpreter stop requesting for getInterpreterSettings().

prabhjyotsingh · 2016-07-23T04:31:23Z

Will merge this if no more discussion.

@felixcheung

…in start ### What is this PR for? While saving interpreter setting, if zeppelin server crashed/killed/restarted by user or there was some internet/download related (intermittent issue) because of which, or any other issue because of which dependencies were not downloaded. In any such condition, when zeppelin is started/restarted, server should try and download dependencies. ### What type of PR is it? [Improvement] ### What is the Jira issue? * [ZEPPELIN-1143](https://issues.apache.org/jira/browse/ZEPPELIN-1143) ### SCREENSHOT ![zeppelin-1143](https://cloud.githubusercontent.com/assets/674497/17025872/7d94d3e4-4f7b-11e6-9182-3e84f0badbab.gif) ### How should this be tested? - Put a dependency (say "org.apache.commons:commons-csv:1.1") in any of the interpreter. - from command line delete `local-repo` directory - restart zeppelin server expectation is `local-repo` should be recreated with all the dependencies that were mentioned in any of the interpreters. ### Questions: * Does the licenses files need update? n/a * Is there breaking changes for older versions? n/a * Does this needs documentation? n/a Author: Prabhjyot Singh <prabhjyotsingh@gmail.com> Closes apache#1155 from prabhjyotsingh/ZEPPELIN-1143 and squashes the following commits: 36cc81d [Prabhjyot Singh] on change of route, stop requesting for getInterpreterSettings() 5aefc5c [Prabhjyot Singh] show update of interpreter dependencies in UI 4f2484a [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-1143 c12c43d [Prabhjyot Singh] address @karup1990 feedback 6ad14a7 [Prabhjyot Singh] Address @felixcheung feedback fb869e8 [Prabhjyot Singh] ZEPPELIN-1143 Interpreter dependencies are not downloaded on zeppelin start

@felixcheung

…in start While saving interpreter setting, if zeppelin server crashed/killed/restarted by user or there was some internet/download related (intermittent issue) because of which, or any other issue because of which dependencies were not downloaded. In any such condition, when zeppelin is started/restarted, server should try and download dependencies. [Improvement] * [ZEPPELIN-1143](https://issues.apache.org/jira/browse/ZEPPELIN-1143) ![zeppelin-1143](https://cloud.githubusercontent.com/assets/674497/17025872/7d94d3e4-4f7b-11e6-9182-3e84f0badbab.gif) - Put a dependency (say "org.apache.commons:commons-csv:1.1") in any of the interpreter. - from command line delete `local-repo` directory - restart zeppelin server expectation is `local-repo` should be recreated with all the dependencies that were mentioned in any of the interpreters. * Does the licenses files need update? n/a * Is there breaking changes for older versions? n/a * Does this needs documentation? n/a Author: Prabhjyot Singh <prabhjyotsingh@gmail.com> Closes apache#1155 from prabhjyotsingh/ZEPPELIN-1143 and squashes the following commits: 36cc81d [Prabhjyot Singh] on change of route, stop requesting for getInterpreterSettings() 5aefc5c [Prabhjyot Singh] show update of interpreter dependencies in UI 4f2484a [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-1143 c12c43d [Prabhjyot Singh] address @karup1990 feedback 6ad14a7 [Prabhjyot Singh] Address @felixcheung feedback fb869e8 [Prabhjyot Singh] ZEPPELIN-1143 Interpreter dependencies are not downloaded on zeppelin start (cherry picked from commit 21a084b)

ZEPPELIN-1143 Interpreter dependencies are not downloaded on zeppelin…

fb869e8

… start

prabhjyotsingh changed the title ~~[ZEPPELIN-1143] Interpreter dependencies are not downloaded on zeppelin…~~ [ZEPPELIN-1143] Interpreter dependencies are not downloaded on zeppelin start Jul 9, 2016

felixcheung reviewed Jul 11, 2016
View reviewed changes

Address @felixcheung feedback

6ad14a7

karuppayya reviewed Jul 14, 2016
View reviewed changes

address @karup1990 feedback

c12c43d

karuppayya reviewed Jul 14, 2016
View reviewed changes

prabhjyotsingh added 2 commits July 21, 2016 10:36

Merge remote-tracking branch 'origin/master' into ZEPPELIN-1143

4f2484a

# Conflicts: # zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java

show update of interpreter dependencies in UI

5aefc5c

prabhjyotsingh force-pushed the ZEPPELIN-1143 branch from e7f872d to 5aefc5c Compare July 21, 2016 13:53

karuppayya reviewed Jul 21, 2016
View reviewed changes

karuppayya reviewed Jul 22, 2016
View reviewed changes

on change of route, stop requesting for getInterpreterSettings()

36cc81d

asfgit closed this in 21a084b Jul 25, 2016

prabhjyotsingh deleted the ZEPPELIN-1143 branch February 25, 2018 03:45

sameer79 mentioned this pull request Jun 4, 2018

ZEPPELIN-3472 No interpreter status is shown after restarting the individual interpreter. #2974

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ZEPPELIN-1143] Interpreter dependencies are not downloaded on zeppelin start #1155

[ZEPPELIN-1143] Interpreter dependencies are not downloaded on zeppelin start #1155

prabhjyotsingh commented Jul 9, 2016 •

edited

Loading

jongyoul commented Jul 10, 2016 •

edited

Loading

prabhjyotsingh commented Jul 11, 2016

ghost commented Jul 11, 2016 •

edited by ghost

Loading

felixcheung Jul 11, 2016

prabhjyotsingh commented Jul 14, 2016

jongyoul commented Jul 14, 2016

karuppayya commented Jul 14, 2016

karuppayya Jul 14, 2016

jongyoul commented Jul 14, 2016

karuppayya Jul 14, 2016

prabhjyotsingh commented Jul 21, 2016

karuppayya commented Jul 21, 2016

karuppayya Jul 21, 2016

prabhjyotsingh Jul 21, 2016

prabhjyotsingh commented Jul 21, 2016

karuppayya Jul 22, 2016

prabhjyotsingh Jul 22, 2016

prabhjyotsingh commented Jul 23, 2016

[ZEPPELIN-1143] Interpreter dependencies are not downloaded on zeppelin start #1155

[ZEPPELIN-1143] Interpreter dependencies are not downloaded on zeppelin start #1155

Conversation

prabhjyotsingh commented Jul 9, 2016 • edited Loading

What is this PR for?

What type of PR is it?

What is the Jira issue?

SCREENSHOT

How should this be tested?

Questions:

jongyoul commented Jul 10, 2016 • edited Loading

prabhjyotsingh commented Jul 11, 2016

ghost commented Jul 11, 2016 • edited by ghost Loading

felixcheung Jul 11, 2016

Choose a reason for hiding this comment

prabhjyotsingh commented Jul 14, 2016

jongyoul commented Jul 14, 2016

karuppayya commented Jul 14, 2016

karuppayya Jul 14, 2016

Choose a reason for hiding this comment

jongyoul commented Jul 14, 2016

karuppayya Jul 14, 2016

Choose a reason for hiding this comment

prabhjyotsingh commented Jul 21, 2016

karuppayya commented Jul 21, 2016

karuppayya Jul 21, 2016

Choose a reason for hiding this comment

prabhjyotsingh Jul 21, 2016

Choose a reason for hiding this comment

prabhjyotsingh commented Jul 21, 2016

karuppayya Jul 22, 2016

Choose a reason for hiding this comment

prabhjyotsingh Jul 22, 2016

Choose a reason for hiding this comment

prabhjyotsingh commented Jul 23, 2016

prabhjyotsingh commented Jul 9, 2016 •

edited

Loading

jongyoul commented Jul 10, 2016 •

edited

Loading

ghost commented Jul 11, 2016 •

edited by ghost

Loading