This is an automated email from the ASF dual-hosted git repository. zjffdu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push: new 720b265 [ZEPPELIN-4219] User can start interpreter when interpreter dependencies jars are not downloaded 720b265 is described below commit 720b26561fb9fe6dab05fdb2c154cbfdb72a933a Author: Jeff Zhang <zjf...@apache.org> AuthorDate: Fri Jul 5 15:33:08 2019 +0800 [ZEPPELIN-4219] User can start interpreter when interpreter dependencies jars are not downloaded ### What is this PR for? Now interpreter can start even when it is downloading dependencies. It will cause ClassNotFound exception. The root cause is that the status of InterpreterSetting is incorrectly set to `READY`. This PR fix this issue. ### What type of PR is it? [Bug Fix ] ### Todos * [ ] - Task ### What is the Jira issue? * https://jira.apache.org/jira/browse/ZEPPELIN-4219 ### How should this be tested? * CI pass ### 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: Jeff Zhang <zjf...@apache.org> Closes #3398 from zjffdu/ZEPPELIN-4219 and squashes the following commits: d8d8309c6 [Jeff Zhang] [ZEPPELIN-4219] User can start interpreter when interpreter dependencies jars are not downloaded --- .../apache/zeppelin/dep/DependencyResolver.java | 6 +-- .../zeppelin/interpreter/InterpreterSetting.java | 6 ++- .../interpreter/InterpreterSettingManager.java | 52 ++++++++++------------ 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/dep/DependencyResolver.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/dep/DependencyResolver.java index 495c69b..0acfca9 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/dep/DependencyResolver.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/dep/DependencyResolver.java @@ -96,7 +96,7 @@ public class DependencyResolver extends AbstractDependencyResolver { File destFile = new File(destPath, srcFile.getName()); if (!destFile.exists() || !FileUtils.contentEquals(srcFile, destFile)) { FileUtils.copyFile(srcFile, destFile); - logger.debug("copy {} to {}", srcFile.getAbsolutePath(), destPath); + logger.info("copy {} to {}", srcFile.getAbsolutePath(), destPath); } } } @@ -114,7 +114,7 @@ public class DependencyResolver extends AbstractDependencyResolver { if (!destFile.exists() || !FileUtils.contentEquals(srcFile, destFile)) { FileUtils.copyFile(srcFile, destFile); - logger.debug("copy {} to {}", srcFile.getAbsolutePath(), destPath); + logger.info("copy {} to {}", srcFile.getAbsolutePath(), destPath); } } @@ -142,7 +142,7 @@ public class DependencyResolver extends AbstractDependencyResolver { List<File> files = new LinkedList<>(); for (ArtifactResult artifactResult : listOfArtifact) { files.add(artifactResult.getArtifact().getFile()); - logger.debug("load {}", artifactResult.getArtifact().getFile().getAbsolutePath()); + logger.info("load {}", artifactResult.getArtifact().getFile().getAbsolutePath()); } return files; 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 82ff9c2..4bac54c 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 @@ -103,7 +103,7 @@ public class InterpreterSetting { */ private Object properties = new Properties(); - private Status status; + private Status status = Status.READY; private String errorReason; @SerializedName("interpreterGroup") @@ -265,7 +265,6 @@ public class InterpreterSetting { } void postProcessing() { - this.status = Status.READY; this.id = this.name; if (this.lifecycleManager == null) { this.lifecycleManager = new NullLifecycleManager(conf); @@ -657,6 +656,7 @@ public class InterpreterSetting { } public void setStatus(Status status) { + LOGGER.info(String.format("Set interpreter %s status to %s", name, status.name())); this.status = status; } @@ -867,6 +867,7 @@ public class InterpreterSetting { // load dependencies List<Dependency> deps = getDependencies(); if (deps != null) { + LOGGER.info("Start to download dependencies for interpreter: " + name); for (Dependency d : deps) { File destDir = new File( conf.getRelativeDir(ZeppelinConfiguration.ConfVars.ZEPPELIN_DEP_LOCALREPO)); @@ -879,6 +880,7 @@ public class InterpreterSetting { .load(d.getGroupArtifactVersion(), new File(destDir, id)); } } + LOGGER.info("Finish downloading dependencies for interpreter: " + name); } setStatus(Status.READY); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java index 7340ab0..5104417 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java @@ -639,35 +639,30 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven * changed */ private void copyDependenciesFromLocalPath(final InterpreterSetting setting) { - final Thread t = new Thread() { - public void run() { - try { - List<Dependency> deps = setting.getDependencies(); - if (deps != null) { - for (Dependency d : deps) { - File destDir = new File( - conf.getRelativeDir(ConfVars.ZEPPELIN_DEP_LOCALREPO)); - - int numSplits = d.getGroupArtifactVersion().split(":").length; - if (!(numSplits >= 3 && numSplits <= 6)) { - dependencyResolver.copyLocalDependency(d.getGroupArtifactVersion(), - new File(destDir, setting.getId())); - } - } + try { + List<Dependency> deps = setting.getDependencies(); + if (deps != null) { + LOGGER.info("Start to copy dependencies for interpreter: " + setting.getName()); + for (Dependency d : deps) { + File destDir = new File( + conf.getRelativeDir(ConfVars.ZEPPELIN_DEP_LOCALREPO)); + + int numSplits = d.getGroupArtifactVersion().split(":").length; + if (!(numSplits >= 3 && numSplits <= 6)) { + dependencyResolver.copyLocalDependency(d.getGroupArtifactVersion(), + new File(destDir, setting.getId())); } - } catch (Exception e) { - LOGGER.error(String.format("Error while copying deps 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 { - } + LOGGER.info("Finish copy dependencies for interpreter: " + setting.getName()); } - }; - t.start(); + } catch (Exception e) { + LOGGER.error(String.format("Error while copying deps 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); + } } /** @@ -777,8 +772,8 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven dependencyResolver.delRepo(id); saveToFile(); } - - /** Change interpreter properties and restart */ + + /** restart in interpreter setting page */ private InterpreterSetting inlineSetPropertyAndRestart( String id, InterpreterOption option, @@ -838,6 +833,7 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven } } + @VisibleForTesting public void restart(String id) throws InterpreterException { InterpreterSetting setting = interpreterSettings.get(id); copyDependenciesFromLocalPath(setting);