Repository: zeppelin
Updated Branches:
  refs/heads/branch-0.8 80efe9a4c -> 6d2b65d83


ZEPPELIN-3221. Create LocalConfigStorage to keep behavior consistent with 
previous version

### What is this PR for?
Due to ZEPPELIN-2742, config will be stored on hdfs if user add HADOOP_CONF_DIR 
in zeppelin-env.sh, this is not consistent with the previous behavior (0.7)
This PR just add LocalConfigStorage which would be the default storage for 
config which is the same behavior of 0.7

### What type of PR is it?
[Bug Fix | Improvement ]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3221

### 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 #2791 from zjffdu/ZEPPELIN-3221 and squashes the following commits:

b442808 [Jeff Zhang] ZEPPELIN-3221. Create LocalConfigStorage to keep behavior 
consistent with previous version

(cherry picked from commit d1293c6bc476378c57db47d48dd8c5355370bb8a)
Signed-off-by: Jeff Zhang <zjf...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/6d2b65d8
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/6d2b65d8
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/6d2b65d8

Branch: refs/heads/branch-0.8
Commit: 6d2b65d8342e26fa103bea1005b2c64afa30d2ed
Parents: 80efe9a
Author: Jeff Zhang <zjf...@apache.org>
Authored: Mon Feb 12 14:28:45 2018 +0800
Committer: Jeff Zhang <zjf...@apache.org>
Committed: Tue Feb 13 09:04:58 2018 +0800

----------------------------------------------------------------------
 .../zeppelin/spark/OldSparkInterpreter.java     |   3 +-
 .../zeppelin/conf/ZeppelinConfiguration.java    |   4 +-
 .../apache/zeppelin/storage/ConfigStorage.java  |  27 +++++
 .../storage/FileSystemConfigStorage.java        |  22 +---
 .../zeppelin/storage/LocalConfigStorage.java    | 110 +++++++++++++++++++
 .../notebook/repo/NotebookRepoSyncTest.java     |   5 +
 6 files changed, 150 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6d2b65d8/spark/interpreter/src/main/java/org/apache/zeppelin/spark/OldSparkInterpreter.java
----------------------------------------------------------------------
diff --git 
a/spark/interpreter/src/main/java/org/apache/zeppelin/spark/OldSparkInterpreter.java
 
b/spark/interpreter/src/main/java/org/apache/zeppelin/spark/OldSparkInterpreter.java
index da332fe..ff3a2ca 100644
--- 
a/spark/interpreter/src/main/java/org/apache/zeppelin/spark/OldSparkInterpreter.java
+++ 
b/spark/interpreter/src/main/java/org/apache/zeppelin/spark/OldSparkInterpreter.java
@@ -239,7 +239,8 @@ public class OldSparkInterpreter extends 
AbstractSparkInterpreter {
       }
 
       @Override
-      public void onExecutorMetricsUpdate(SparkListenerExecutorMetricsUpdate 
executorMetricsUpdate) {
+      public void onExecutorMetricsUpdate(
+          SparkListenerExecutorMetricsUpdate executorMetricsUpdate) {
 
       }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6d2b65d8/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
index 17e3e5b..59a1f94 100644
--- 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
+++ 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
@@ -518,7 +518,7 @@ public class ZeppelinConfiguration extends XMLConfiguration 
{
     if (StringUtils.isBlank(fsConfigDir)) {
       LOG.warn(ConfVars.ZEPPELIN_CONFIG_FS_DIR.varName + " is not specified, 
fall back to local " +
           "conf directory " + ConfVars.ZEPPELIN_CONF_DIR.varName);
-      return "file://" + getConfDir();
+      return getConfDir();
     }
     return fsConfigDir;
   }
@@ -709,7 +709,7 @@ public class ZeppelinConfiguration extends XMLConfiguration 
{
     ZEPPELIN_CONF_DIR("zeppelin.conf.dir", "conf"),
     ZEPPELIN_CONFIG_FS_DIR("zeppelin.config.fs.dir", ""),
     ZEPPELIN_CONFIG_STORAGE_CLASS("zeppelin.config.storage.class",
-        "org.apache.zeppelin.storage.FileSystemConfigStorage"),
+        "org.apache.zeppelin.storage.LocalConfigStorage"),
     ZEPPELIN_DEP_LOCALREPO("zeppelin.dep.localrepo", "local-repo"),
     ZEPPELIN_HELIUM_REGISTRY("zeppelin.helium.registry", "helium," + 
HELIUM_PACKAGE_DEFAULT_URL),
     ZEPPELIN_HELIUM_NODE_INSTALLER_URL("zeppelin.helium.node.installer.url",

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6d2b65d8/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java
index 3dc935f..b3175e5 100644
--- 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java
+++ 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/ConfigStorage.java
@@ -18,9 +18,13 @@
 
 package org.apache.zeppelin.storage;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonParser;
 import org.apache.zeppelin.conf.ZeppelinConfiguration;
 import org.apache.zeppelin.helium.HeliumConf;
 import org.apache.zeppelin.interpreter.InterpreterInfoSaving;
+import org.apache.zeppelin.interpreter.InterpreterSetting;
 import org.apache.zeppelin.notebook.NotebookAuthorizationInfoSaving;
 import org.apache.zeppelin.user.Credentials;
 import org.apache.zeppelin.user.CredentialsInfoSaving;
@@ -75,4 +79,27 @@ public abstract class ConfigStorage {
   public abstract String loadCredentials() throws IOException;
 
   public abstract void saveCredentials(String credentials) throws IOException;
+
+  protected InterpreterInfoSaving buildInterpreterInfoSaving(String json) {
+    //TODO(zjffdu) This kind of post processing is ugly.
+    JsonParser jsonParser = new JsonParser();
+    JsonObject jsonObject = jsonParser.parse(json).getAsJsonObject();
+    InterpreterInfoSaving infoSaving = InterpreterInfoSaving.fromJson(json);
+    for (InterpreterSetting interpreterSetting : 
infoSaving.interpreterSettings.values()) {
+      // Always use separate interpreter process
+      // While we decided to turn this feature on always (without providing
+      // enable/disable option on GUI).
+      // previously created setting should turn this feature on here.
+      interpreterSetting.getOption();
+      interpreterSetting.convertPermissionsFromUsersToOwners(
+          jsonObject.getAsJsonObject("interpreterSettings")
+              .getAsJsonObject(interpreterSetting.getId()));
+    }
+    return infoSaving;
+  }
+
+  @VisibleForTesting
+  public static void reset() {
+    instance = null;
+  }
 }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6d2b65d8/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
index 4df8163..20c19b6 100644
--- 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
+++ 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
@@ -74,21 +74,7 @@ public class FileSystemConfigStorage extends ConfigStorage {
     }
     LOGGER.info("Load Interpreter Setting from file: " + 
interpreterSettingPath);
     String json = fs.readFile(interpreterSettingPath);
-    //TODO(zjffdu) This kind of post processing is ugly.
-    JsonParser jsonParser = new JsonParser();
-    JsonObject jsonObject = jsonParser.parse(json).getAsJsonObject();
-    InterpreterInfoSaving infoSaving = InterpreterInfoSaving.fromJson(json);
-    for (InterpreterSetting interpreterSetting : 
infoSaving.interpreterSettings.values()) {
-      // Always use separate interpreter process
-      // While we decided to turn this feature on always (without providing
-      // enable/disable option on GUI).
-      // previously created setting should turn this feature on here.
-      interpreterSetting.getOption();
-      interpreterSetting.convertPermissionsFromUsersToOwners(
-          jsonObject.getAsJsonObject("interpreterSettings")
-              .getAsJsonObject(interpreterSetting.getId()));
-    }
-    return infoSaving;
+    return buildInterpreterInfoSaving(json);
   }
 
   public void save(NotebookAuthorizationInfoSaving authorizationInfoSaving) 
throws IOException {
@@ -99,7 +85,7 @@ public class FileSystemConfigStorage extends ConfigStorage {
   @Override
   public NotebookAuthorizationInfoSaving loadNotebookAuthorization() throws 
IOException {
     if (!fs.exists(authorizationPath)) {
-      LOGGER.warn("Interpreter Setting file {} is not existed", 
authorizationPath);
+      LOGGER.warn("Notebook Authorization file {} is not existed", 
authorizationPath);
       return null;
     }
     LOGGER.info("Load notebook authorization from file: " + authorizationPath);
@@ -110,10 +96,10 @@ public class FileSystemConfigStorage extends ConfigStorage 
{
   @Override
   public String loadCredentials() throws IOException {
     if (!fs.exists(credentialPath)) {
-      LOGGER.warn("Credential file {} is not existed", authorizationPath);
+      LOGGER.warn("Credential file {} is not existed", credentialPath);
       return null;
     }
-    LOGGER.info("Load Credential from file: " + authorizationPath);
+    LOGGER.info("Load Credential from file: " + credentialPath);
     return this.fs.readFile(credentialPath);
   }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6d2b65d8/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
new file mode 100644
index 0000000..c1edbb5
--- /dev/null
+++ 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.storage;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.zeppelin.conf.ZeppelinConfiguration;
+import org.apache.zeppelin.interpreter.InterpreterInfoSaving;
+import org.apache.zeppelin.notebook.NotebookAuthorizationInfoSaving;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+
+/**
+ * Storing config in local file system
+ */
+public class LocalConfigStorage extends ConfigStorage {
+
+  private static Logger LOGGER = 
LoggerFactory.getLogger(LocalConfigStorage.class);
+
+  private File interpreterSettingPath;
+  private File authorizationPath;
+  private File credentialPath;
+
+  public LocalConfigStorage(ZeppelinConfiguration zConf) {
+    super(zConf);
+    this.interpreterSettingPath = new File(zConf.getInterpreterSettingPath());
+    this.authorizationPath = new File(zConf.getNotebookAuthorizationPath());
+    this.credentialPath = new File(zConf.getCredentialsPath());
+  }
+
+  @Override
+  public void save(InterpreterInfoSaving settingInfos) throws IOException {
+    writeToFile(settingInfos.toJson(), interpreterSettingPath);
+  }
+
+  @Override
+  public InterpreterInfoSaving loadInterpreterSettings() throws IOException {
+    if (!interpreterSettingPath.exists()) {
+      LOGGER.warn("Interpreter Setting file {} is not existed", 
interpreterSettingPath);
+      return null;
+    }
+    LOGGER.info("Load Interpreter Setting from file: " + 
interpreterSettingPath);
+    String json = readFromFile(interpreterSettingPath);
+    return buildInterpreterInfoSaving(json);
+  }
+
+  @Override
+  public void save(NotebookAuthorizationInfoSaving authorizationInfoSaving) 
throws IOException {
+    LOGGER.info("Save notebook authorization to file: " + authorizationPath);
+    writeToFile(authorizationInfoSaving.toJson(), authorizationPath);
+  }
+
+  @Override
+  public NotebookAuthorizationInfoSaving loadNotebookAuthorization() throws 
IOException {
+    if (!authorizationPath.exists()) {
+      LOGGER.warn("NotebookAuthorization file {} is not existed", 
authorizationPath);
+      return null;
+    }
+    LOGGER.info("Load notebook authorization from file: " + authorizationPath);
+    String json = readFromFile(authorizationPath);
+    return NotebookAuthorizationInfoSaving.fromJson(json);
+  }
+
+  @Override
+  public String loadCredentials() throws IOException {
+    if (!credentialPath.exists()) {
+      LOGGER.warn("Credential file {} is not existed", credentialPath);
+      return null;
+    }
+    LOGGER.info("Load Credential from file: " + credentialPath);
+    return readFromFile(credentialPath);
+  }
+
+  @Override
+  public void saveCredentials(String credentials) throws IOException {
+    LOGGER.info("Save Credentials to file: " + credentialPath);
+    writeToFile(credentials, credentialPath);
+  }
+
+  private String readFromFile(File file) throws IOException {
+    return IOUtils.toString(new FileInputStream(file));
+  }
+
+  private void writeToFile(String content, File file) throws IOException {
+    FileOutputStream out = new FileOutputStream(file);
+    IOUtils.write(content, out);
+    out.close();
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6d2b65d8/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java
 
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java
index 2236654..8904239 100644
--- 
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java
+++ 
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java
@@ -44,6 +44,7 @@ import org.apache.zeppelin.scheduler.Job;
 import org.apache.zeppelin.scheduler.Job.Status;
 import org.apache.zeppelin.scheduler.SchedulerFactory;
 import org.apache.zeppelin.search.SearchService;
+import org.apache.zeppelin.storage.ConfigStorage;
 import org.apache.zeppelin.user.AuthenticationInfo;
 import org.apache.zeppelin.user.Credentials;
 import org.junit.After;
@@ -89,10 +90,14 @@ public class NotebookRepoSyncTest implements 
JobListenerFactory {
     System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(), 
mainNotebookDir.getAbsolutePath());
     System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE.getVarName(), 
"org.apache.zeppelin.notebook.repo.VFSNotebookRepo,org.apache.zeppelin.notebook.repo.mock.VFSNotebookRepoMock");
     System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_ONE_WAY_SYNC.getVarName(), 
"false");
+    System.setProperty(ConfVars.ZEPPELIN_CONFIG_FS_DIR.getVarName(), 
mainZepDir.getAbsolutePath() + "/conf");
+
     LOG.info("main Note dir : " + mainNotePath);
     LOG.info("secondary note dir : " + secNotePath);
     conf = ZeppelinConfiguration.create();
 
+    ConfigStorage.reset();
+
     this.schedulerFactory = SchedulerFactory.singleton();
 
     depResolver = new DependencyResolver(mainZepDir.getAbsolutePath() + 
"/local-repo");

Reply via email to