[zeppelin] branch branch-0.10 updated: [ZEPPELIN-5526] interpreter setting still in error status when remove the invalid dependency

[zj...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch branch-0.10
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/branch-0.10 by this push:
     new 5da5557  [ZEPPELIN-5526] interpreter setting still in error status when remove the invalid dependency
5da5557 is described below

commit 5da55570643ee9ff5fedd2b2d5aef282f340c68c
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Thu Sep 16 23:32:37 2021 +0800

    [ZEPPELIN-5526] interpreter setting still in error status when remove the invalid dependency
    
    ### What is this PR for?
    
    The root cause is that the status and errorReason is not reset after invalid dependency is removed. This PR just reset them and also add unit test for that.
    
    ### What type of PR is it?
    [Bug Fix ]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-5526
    
    ### 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 <zj...@apache.org>
    
    Closes #4227 from zjffdu/ZEPPELIN-5526 and squashes the following commits:
    
    057b455840 [Jeff Zhang] [ZEPPELIN-5526] interpreter setting still in error status when remove the invalid dependency
    
    (cherry picked from commit 6a30c0c745f53a6eb0462d17c32459567382a91a)
    Signed-off-by: Jeff Zhang <zj...@apache.org>
---
 .../zeppelin/interpreter/InterpreterSetting.java   |  5 +++
 .../interpreter/InterpreterSettingTest.java        | 39 ++++++++++++++++++++++
 2 files changed, 44 insertions(+)

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 ca2f9d7..3b0728e 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
@@ -667,6 +667,11 @@ public class InterpreterSetting {
     this.dependencies = dependencies;
     if (!this.dependencies.isEmpty()) {
       loadInterpreterDependencies();
+    } else {
+      // Interpreter setting may be in ERROR due to download fail,
+      // reset status to be READY when dependency is cleaned.
+      setStatus(Status.READY);
+      setErrorReason(null);
     }
   }
 
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java
index 73b1886..5d509bf 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java
@@ -17,11 +17,16 @@
 
 package org.apache.zeppelin.interpreter;
 
+import com.google.common.collect.Lists;
+import org.apache.zeppelin.dep.Dependency;
+import org.apache.zeppelin.dep.DependencyResolver;
 import org.apache.zeppelin.notebook.Note;
 import org.apache.zeppelin.notebook.NoteInfo;
 import org.apache.zeppelin.notebook.Notebook;
 import org.junit.Before;
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -38,6 +43,8 @@ import static org.mockito.Mockito.when;
 
 public class InterpreterSettingTest {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(InterpreterSettingTest.class);
+
   private InterpreterSettingManager interpreterSettingManager;
   private Notebook notebook;
   private Note note1;
@@ -553,4 +560,36 @@ public class InterpreterSettingTest {
           .create();
       assertTrue(interpreterSetting.isUserAuthorized(userAndRoles));
   }
+
+  @Test
+  public void testLoadDependency() throws InterruptedException {
+    InterpreterOption interpreterOption = new InterpreterOption();
+    interpreterOption.setUserPermission(true);
+    InterpreterSetting interpreterSetting = new InterpreterSetting.Builder()
+            .setId("id")
+            .setName("id")
+            .setGroup("group")
+            .setOption(interpreterOption)
+            .setIntepreterSettingManager(interpreterSettingManager)
+            .setDependencyResolver(new DependencyResolver("/tmp"))
+            .create();
+
+    // set invalid dependency
+    interpreterSetting.setDependencies(Lists.newArrayList(new Dependency("a:b:0.1")));
+    long start = System.currentTimeMillis();
+    long threshold = 60 * 1000;
+    while(interpreterSetting.getStatus() != InterpreterSetting.Status.ERROR &&
+            (System.currentTimeMillis() - start) < threshold) {
+      Thread.sleep(1000);
+      LOGGER.warn("Downloading dependency");
+    }
+    assertTrue(interpreterSetting.getErrorReason(),
+            interpreterSetting.getErrorReason().contains("Cannot fetch dependencies"));
+
+    // clean dependency
+    interpreterSetting.setDependencies(new ArrayList<>());
+    assertEquals(InterpreterSetting.Status.READY, interpreterSetting.getStatus());
+    assertNull(interpreterSetting.getErrorReason());
+
+  }
 }