Repository: zeppelin Updated Branches: refs/heads/master 1077921b7 -> 21a084b17
[ZEPPELIN-1143] Interpreter dependencies are not downloaded on zeppelin 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  ### 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 <[email protected]> Closes #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 Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/21a084b1 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/21a084b1 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/21a084b1 Branch: refs/heads/master Commit: 21a084b170b29dee0f2d57aae22b00c785ef0c8e Parents: 1077921 Author: Prabhjyot Singh <[email protected]> Authored: Fri Jul 22 12:58:56 2016 +0530 Committer: Prabhjyot Singh <[email protected]> Committed: Mon Jul 25 10:37:48 2016 +0530 ---------------------------------------------------------------------- .../zeppelin/rest/InterpreterRestApi.java | 2 +- .../app/interpreter/interpreter.controller.js | 29 ++++++++- .../src/app/interpreter/interpreter.html | 20 ++++++ .../interpreter/InterpreterFactory.java | 67 +++++++++++++------- .../interpreter/InterpreterSetting.java | 28 ++++++++ 5 files changed, 121 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/21a084b1/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java index 8f5a441..f77aac0 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java @@ -120,7 +120,7 @@ public class InterpreterRestApi { logger.error("Exception in InterpreterRestApi while updateSetting ", e); return new JsonResponse<>(Status.NOT_FOUND, e.getMessage(), ExceptionUtils.getStackTrace(e)) .build(); - } catch (IOException | RepositoryException e) { + } catch (IOException e) { logger.error("Exception in InterpreterRestApi while updateSetting ", e); return new JsonResponse<>(Status.INTERNAL_SERVER_ERROR, e.getMessage(), ExceptionUtils.getStackTrace(e)).build(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/21a084b1/zeppelin-web/src/app/interpreter/interpreter.controller.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/interpreter/interpreter.controller.js b/zeppelin-web/src/app/interpreter/interpreter.controller.js index 937e31a..e152d57 100644 --- a/zeppelin-web/src/app/interpreter/interpreter.controller.js +++ b/zeppelin-web/src/app/interpreter/interpreter.controller.js @@ -14,7 +14,7 @@ 'use strict'; angular.module('zeppelinWebApp').controller('InterpreterCtrl', - function($scope, $http, baseUrlSrv, ngToast) { + function($scope, $http, baseUrlSrv, ngToast, $timeout, $route) { var interpreterSettingsTmp = []; $scope.interpreterSettings = []; $scope.availableInterpreters = {}; @@ -25,6 +25,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(); }).error(function(data, status, headers, config) { if (status === 401) { ngToast.danger({ @@ -40,6 +41,23 @@ angular.module('zeppelinWebApp').controller('InterpreterCtrl', }); }; + var checkDownloadingDependencies = function() { + var isDownloading = false; + for (var setting = 0; setting < $scope.interpreterSettings.length; setting++) { + if ($scope.interpreterSettings[setting].status === 'DOWNLOADING_DEPENDENCIES') { + isDownloading = true; + break; + } + } + if (isDownloading) { + $timeout(function() { + if ($route.current.$$route.originalPath === '/interpreter') { + getInterpreterSettings(); + } + }, 2000); + } + }; + var getAvailableInterpreters = function() { $http.get(baseUrlSrv.getRestApiBase() + '/interpreter').success(function(data, status, headers, config) { $scope.availableInterpreters = data.body; @@ -149,6 +167,7 @@ angular.module('zeppelinWebApp').controller('InterpreterCtrl', $scope.interpreterSettings[index] = data.body; removeTMPSettings(index); thisConfirm.close(); + checkDownloadingDependencies(); }) .error(function(data, status, headers, config) { console.log('Error %o %o', status, data.message); @@ -270,6 +289,7 @@ angular.module('zeppelinWebApp').controller('InterpreterCtrl', $scope.resetNewInterpreterSetting(); getInterpreterSettings(); $scope.showAddNewSetting = false; + checkDownloadingDependencies(); }).error(function(data, status, headers, config) { console.log('Error %o %o', status, data.message); ngToast.danger({content: data.message, verticalPosition: 'bottom'}); @@ -456,6 +476,13 @@ angular.module('zeppelinWebApp').controller('InterpreterCtrl', } }; + $scope.showErrorMessage = function(setting) { + BootstrapDialog.show({ + title: 'Error downloading dependencies', + message: setting.errorReason + }); + }; + var init = function() { $scope.resetNewInterpreterSetting(); $scope.resetNewRepositorySetting(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/21a084b1/zeppelin-web/src/app/interpreter/interpreter.html ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/interpreter/interpreter.html b/zeppelin-web/src/app/interpreter/interpreter.html index 088359a..3c25246 100644 --- a/zeppelin-web/src/app/interpreter/interpreter.html +++ b/zeppelin-web/src/app/interpreter/interpreter.html @@ -99,6 +99,26 @@ limitations under the License. <span ng-show="$parent.$first && $first">(default)</span> </span> </small> + + <small ng-switch="setting.status"> + <small ng-switch-when="READY"> + <i style="color: green; margin-right: 3px;" class="fa fa-circle" + tooltip="Ready"> + </i> + </small> + <small ng-switch-when="ERROR"> + <i style="color: red; cursor: pointer" class="fa fa-circle" + ng-click="showErrorMessage(setting)" + tooltip="Error downloading dependencies"> + </i> + </small> + <small ng-switch-default> + <i style="color: blue" class="fa fa-spinner spinAnimation" + tooltip="Dependencies are downloading"> + </i> + </small> + </small> + </h3> <span style="float:right"> <button class="btn btn-default btn-xs" http://git-wip-us.apache.org/repos/asf/zeppelin/blob/21a084b1/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java index c02c882..7f8699d 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java @@ -370,6 +370,7 @@ public class InterpreterFactory implements InterpreterGroupFactory { setting.setPath(interpreterSettingsRef.get(setting.getGroup()).getPath()); setting.setInterpreterGroupFactory(this); + loadInterpreterDependencies(setting); interpreterSettings.put(k, setting); } @@ -384,27 +385,50 @@ public class InterpreterFactory implements InterpreterGroupFactory { } } - private void loadInterpreterDependencies(InterpreterSetting intSetting) - throws IOException, RepositoryException { - // dependencies to prevent library conflict - File localRepoDir = new File(conf.getInterpreterLocalRepoPath() + "/" + intSetting.getId()); - if (localRepoDir.exists()) { - FileUtils.cleanDirectory(localRepoDir); - } + private void loadInterpreterDependencies(final InterpreterSetting setting) { - // load dependencies - List<Dependency> deps = intSetting.getDependencies(); - if (deps != null) { - for (Dependency d : deps) { - File destDir = new File(conf.getRelativeDir(ConfVars.ZEPPELIN_DEP_LOCALREPO)); + setting.setStatus(InterpreterSetting.Status.DOWNLOADING_DEPENDENCIES); + interpreterSettings.put(setting.getId(), setting); + synchronized (interpreterSettings) { + final Thread t = new Thread() { + public void run() { + try { + // dependencies to prevent library conflict + File localRepoDir = new File(conf.getInterpreterLocalRepoPath() + "/" + + setting.getId()); + if (localRepoDir.exists()) { + FileUtils.cleanDirectory(localRepoDir); + } - if (d.getExclusions() != null) { - depResolver.load(d.getGroupArtifactVersion(), d.getExclusions(), - new File(destDir, intSetting.getId())); - } else { - depResolver.load(d.getGroupArtifactVersion(), new File(destDir, intSetting.getId())); + // load dependencies + List<Dependency> deps = setting.getDependencies(); + if (deps != null) { + for (Dependency d : deps) { + File destDir = new File(conf.getRelativeDir(ConfVars.ZEPPELIN_DEP_LOCALREPO)); + + if (d.getExclusions() != null) { + depResolver.load(d.getGroupArtifactVersion(), d.getExclusions(), + new File(destDir, setting.getId())); + } else { + depResolver.load(d.getGroupArtifactVersion(), new File(destDir, setting.getId())); + } + } + } + + setting.setStatus(InterpreterSetting.Status.READY); + } catch (Exception e) { + logger.error(String.format("Error while downloading repos for interpreter group : %s," + + " go to interpreter setting page click on edit and save it again to make " + + "this interpreter work properly.", + setting.getGroup()), e); + setting.setErrorReason(e.getLocalizedMessage()); + setting.setStatus(InterpreterSetting.Status.ERROR); + } finally { + interpreterSettings.put(setting.getId(), setting); + } } - } + }; + t.start(); } } @@ -500,12 +524,9 @@ public class InterpreterFactory implements InterpreterGroupFactory { * @param group InterpreterSetting reference name * @param properties * @return - * @throws InterpreterException - * @throws IOException */ public InterpreterSetting add(String group, ArrayList<InterpreterInfo> interpreterInfos, - List<Dependency> dependencies, InterpreterOption option, Properties properties, String path) - throws InterpreterException, IOException, RepositoryException { + List<Dependency> dependencies, InterpreterOption option, Properties properties, String path) { Preconditions.checkNotNull(group, "name should not be null"); Preconditions.checkNotNull(interpreterInfos, "interpreterInfos should not be null"); Preconditions.checkNotNull(dependencies, "dependencies should not be null"); @@ -806,7 +827,7 @@ public class InterpreterFactory implements InterpreterGroupFactory { * @throws IOException */ public void setPropertyAndRestart(String id, InterpreterOption option, Properties properties, - List<Dependency> dependencies) throws IOException, RepositoryException { + List<Dependency> dependencies) throws IOException { synchronized (interpreterSettings) { InterpreterSetting intpsetting = interpreterSettings.get(id); if (intpsetting != null) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/21a084b1/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java index 5f340e9..0288eb4 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java @@ -40,6 +40,8 @@ public class InterpreterSetting { private String name; private String group; // always be null in case of InterpreterSettingRef private Properties properties; + private Status status; + private String errorReason; @SerializedName("interpreterGroup") private List<InterpreterInfo> interpreterInfos; private final transient Map<String, InterpreterGroup> interpreterGroupRef = new HashMap<>(); @@ -64,6 +66,7 @@ public class InterpreterSetting { this.dependencies = dependencies; this.option = option; this.path = path; + this.status = Status.READY; } public InterpreterSetting(String name, String group, List<InterpreterInfo> interpreterInfos, @@ -210,4 +213,29 @@ public class InterpreterSetting { void setName(String name) { this.name = name; } + + /*** + * Interpreter status + */ + public enum Status { + DOWNLOADING_DEPENDENCIES, + ERROR, + READY + } + + public Status getStatus() { + return status; + } + + public void setStatus(Status status) { + this.status = status; + } + + public String getErrorReason() { + return errorReason; + } + + public void setErrorReason(String errorReason) { + this.errorReason = errorReason; + } }
