Repository: zeppelin Updated Branches: refs/heads/master c6afe8c63 -> d1293c6bc
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 Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/d1293c6b Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/d1293c6b Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/d1293c6b Branch: refs/heads/master Commit: d1293c6bc476378c57db47d48dd8c5355370bb8a Parents: c6afe8c 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:45 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/d1293c6b/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/d1293c6b/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 a107320..6bce468 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/d1293c6b/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/d1293c6b/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/d1293c6b/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/d1293c6b/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");
