Repository: zeppelin Updated Branches: refs/heads/master f7c7efdb8 -> 28adacb9c
[ZEPPELIN-1461] Doesn't display "description" value in interpreter creation page ### What is this PR for? #1522 tried to fix same issue by making as least change as possible, but fixing it in backend side looks like more proper approach as https://github.com/apache/zeppelin/pull/1522#issuecomment-255109922. This PR fixes ZEPPELIN-1461 by changing `properties` field of `InterpreterSetting` class from `Properties` -> `Object`. ### What type of PR is it? Bug Fix ### What is the Jira issue? [ZEPPELIN-1461](https://issues.apache.org/jira/browse/ZEPPELIN-1461) ### Screenshots (if appropriate) Before  After  ### Questions: - Does the licenses files need update? no - Is there breaking changes for older versions? no - Does this needs documentation? no Author: Mina Lee <[email protected]> Closes #1559 from minahlee/ZEPPELIN-1461 and squashes the following commits: 4a278f0 [Mina Lee] Add test checking InterpreterProperty class 14a6300 [Mina Lee] Add selenium test for display description on interpreter create 4eba177 [Mina Lee] Fix order of properties in ui and java code style 1a2a41d [Mina Lee] Show description when create new interpreter Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/28adacb9 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/28adacb9 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/28adacb9 Branch: refs/heads/master Commit: 28adacb9c878f2d798f872048009bcdf3a41879c Parents: f7c7efd Author: Mina Lee <[email protected]> Authored: Mon Oct 24 19:54:33 2016 +0900 Committer: Jongyoul Lee <[email protected]> Committed: Wed Nov 2 13:27:03 2016 +0900 ---------------------------------------------------------------------- .../zeppelin/integration/InterpreterIT.java | 82 ++++++++++++++++ .../zeppelin/rest/AbstractTestRestApi.java | 47 +++++----- .../interpreter-create/interpreter-create.html | 6 +- .../app/interpreter/interpreter.controller.js | 3 +- .../interpreter/InterpreterFactory.java | 98 ++++++++++---------- .../interpreter/InterpreterSetting.java | 27 ++++-- .../interpreter/InterpreterFactoryTest.java | 48 ++++++++-- 7 files changed, 216 insertions(+), 95 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/28adacb9/zeppelin-server/src/test/java/org/apache/zeppelin/integration/InterpreterIT.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/InterpreterIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/InterpreterIT.java new file mode 100644 index 0000000..9587cd6 --- /dev/null +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/InterpreterIT.java @@ -0,0 +1,82 @@ +/* + * 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.integration; + +import org.apache.zeppelin.AbstractZeppelinIT; +import org.apache.zeppelin.WebDriverManager; +import org.hamcrest.CoreMatchers; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ErrorCollector; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.support.ui.Select; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class InterpreterIT extends AbstractZeppelinIT { + private static final Logger LOG = LoggerFactory.getLogger(InterpreterIT.class); + + @Rule + public ErrorCollector collector = new ErrorCollector(); + + @Before + public void startUp() { + if (!endToEndTestEnabled()) { + return; + } + driver = WebDriverManager.getWebDriver(); + } + + @After + public void tearDown() { + if (!endToEndTestEnabled()) { + return; + } + driver.quit(); + } + + @Test + public void testShowDescriptionOnInterpreterCreate() throws Exception { + if (!endToEndTestEnabled()) { + return; + } + try { + // navigate to interpreter page + WebElement settingButton = driver.findElement(By.xpath("//button[@class='nav-btn dropdown-toggle ng-scope']")); + settingButton.click(); + WebElement interpreterLink = driver.findElement(By.xpath("//a[@href='#/interpreter']")); + interpreterLink.click(); + + WebElement createButton = driver.findElement(By.xpath("//button[contains(., 'Create')]")); + createButton.click(); + + Select select = new Select(driver.findElement(By.xpath("//select[@ng-change='newInterpreterGroupChange()']"))); + select.selectByVisibleText("spark"); + + collector.checkThat("description of interpreter property is displayed", + driver.findElement(By.xpath("//tr/td[contains(text(), 'spark.app.name')]/following-sibling::td[2]")).getText(), + CoreMatchers.equalTo("The name of spark application.")); + + } catch (Exception e) { + handleException("Exception in InterpreterIT while testShowDescriptionOnInterpreterCreate ", e); + } + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/zeppelin/blob/28adacb9/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java index 6d0d0a9..823b1dd 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java @@ -23,6 +23,7 @@ import java.lang.ref.WeakReference; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.List; +import java.util.Properties; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -140,47 +141,41 @@ public abstract class AbstractTestRestApi { LOG.info("Test Zeppelin stared."); + // assume first one is spark + InterpreterSetting sparkIntpSetting = null; + for(InterpreterSetting intpSetting : ZeppelinServer.notebook.getInterpreterFactory().get()) { + if (intpSetting.getName().equals("spark")) { + sparkIntpSetting = intpSetting; + } + } + + Properties sparkProperties = (Properties) sparkIntpSetting.getProperties(); // ci environment runs spark cluster for testing // so configure zeppelin use spark cluster if ("true".equals(System.getenv("CI"))) { - // assume first one is spark - InterpreterSetting sparkIntpSetting = null; - for(InterpreterSetting intpSetting : ZeppelinServer.notebook.getInterpreterFactory().get()) { - if (intpSetting.getName().equals("spark")) { - sparkIntpSetting = intpSetting; - } - } - // set spark master and other properties - sparkIntpSetting.getProperties().setProperty("master", "local[2]"); - sparkIntpSetting.getProperties().setProperty("spark.cores.max", "2"); - sparkIntpSetting.getProperties().setProperty("zeppelin.spark.useHiveContext", "false"); + sparkProperties.setProperty("master", "local[2]"); + sparkProperties.setProperty("spark.cores.max", "2"); + sparkProperties.setProperty("zeppelin.spark.useHiveContext", "false"); // set spark home for pyspark - sparkIntpSetting.getProperties().setProperty("spark.home", getSparkHome()); + sparkProperties.setProperty("spark.home", getSparkHome()); + + sparkIntpSetting.setProperties(sparkProperties); pySpark = true; sparkR = true; ZeppelinServer.notebook.getInterpreterFactory().restart(sparkIntpSetting.getId()); } else { - // assume first one is spark - InterpreterSetting sparkIntpSetting = null; - for(InterpreterSetting intpSetting : ZeppelinServer.notebook.getInterpreterFactory().get()) { - if (intpSetting.getName().equals("spark")) { - sparkIntpSetting = intpSetting; - } - } - String sparkHome = getSparkHome(); if (sparkHome != null) { if (System.getenv("SPARK_MASTER") != null) { - sparkIntpSetting.getProperties().setProperty("master", System.getenv("SPARK_MASTER")); + sparkProperties.setProperty("master", System.getenv("SPARK_MASTER")); } else { - sparkIntpSetting.getProperties() - .setProperty("master", "local[2]"); + sparkProperties.setProperty("master", "local[2]"); } - sparkIntpSetting.getProperties().setProperty("spark.cores.max", "2"); + sparkProperties.setProperty("spark.cores.max", "2"); // set spark home for pyspark - sparkIntpSetting.getProperties().setProperty("spark.home", sparkHome); - sparkIntpSetting.getProperties().setProperty("zeppelin.spark.useHiveContext", "false"); + sparkProperties.setProperty("spark.home", sparkHome); + sparkProperties.setProperty("zeppelin.spark.useHiveContext", "false"); pySpark = true; sparkR = true; } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/28adacb9/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html b/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html index 9013aab..2407e82 100644 --- a/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html +++ b/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html @@ -263,10 +263,10 @@ limitations under the License. <th>description</th> <th>action</th> </tr> - <tr ng-repeat="(key, value) in newInterpreterSetting.properties"> + <tr ng-repeat="key in newInterpreterSetting.properties | sortByKey"> <td>{{key}}</td> - <td><textarea msd-elastic ng-model="value.value"></textarea></td> - <td>{{value.description}}</td> + <td><textarea msd-elastic ng-model="newInterpreterSetting.properties[key].value"></textarea></td> + <td>{{newInterpreterSetting.properties[key].description}}</td> <td> <button class="btn btn-default btn-sm fa fa-remove" ng-click="removeInterpreterProperty(key)"> </button> http://git-wip-us.apache.org/repos/asf/zeppelin/blob/28adacb9/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 0669ff8..e4bebeb 100644 --- a/zeppelin-web/src/app/interpreter/interpreter.controller.js +++ b/zeppelin-web/src/app/interpreter/interpreter.controller.js @@ -399,12 +399,11 @@ var intpInfo = el[i]; for (var key in intpInfo) { properties[key] = { - value: intpInfo[key], + value: intpInfo[key].defaultValue, description: intpInfo[key].description }; } } - $scope.newInterpreterSetting.properties = properties; }; http://git-wip-us.apache.org/repos/asf/zeppelin/blob/28adacb9/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 7711b9a..85a9254 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 @@ -53,6 +53,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.gson.Gson; import com.google.gson.GsonBuilder; +import com.google.gson.internal.StringMap; import com.google.gson.reflect.TypeToken; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.ArrayUtils; @@ -237,8 +238,7 @@ public class InterpreterFactory implements InterpreterGroupFactory { interpreterInfo = new InterpreterInfo(r.getClassName(), r.getName(), r.isDefaultInterpreter(), r.getEditor()); - add(r.getGroup(), interpreterInfo, convertInterpreterProperties(r.getProperties()), - r.getPath()); + add(r.getGroup(), interpreterInfo, r.getProperties(), r.getPath()); } for (String settingId : interpreterSettingsRef.keySet()) { @@ -286,7 +286,8 @@ public class InterpreterFactory implements InterpreterGroupFactory { private InterpreterSetting createFromInterpreterSettingRef(InterpreterSetting o) { InterpreterSetting setting = - new InterpreterSetting(o.getName(), o.getName(), o.getInterpreterInfos(), o.getProperties(), + new InterpreterSetting(o.getName(), o.getName(), o.getInterpreterInfos(), + convertInterpreterProperties((Map<String, InterpreterProperty>) o.getProperties()), o.getDependencies(), o.getOption(), o.getPath()); setting.setInterpreterGroupFactory(this); return setting; @@ -349,16 +350,9 @@ public class InterpreterFactory implements InterpreterGroupFactory { InterpreterInfo interpreterInfo = new InterpreterInfo(registeredInterpreter.getClassName(), registeredInterpreter.getName(), registeredInterpreter.isDefaultInterpreter(), registeredInterpreter.getEditor()); - Properties properties = new Properties(); - Map<String, InterpreterProperty> p = registeredInterpreter.getProperties(); - - if (null != p) { - for (String key : p.keySet()) { - properties.setProperty(key, p.get(key).getValue()); - } - } - add(registeredInterpreter.getGroup(), interpreterInfo, properties, absolutePath); + add(registeredInterpreter.getGroup(), interpreterInfo, registeredInterpreter.getProperties(), + absolutePath); } } @@ -389,6 +383,14 @@ public class InterpreterFactory implements InterpreterGroupFactory { InterpreterSetting setting = infoSaving.interpreterSettings.get(k); List<InterpreterInfo> infos = setting.getInterpreterInfos(); + // Convert json StringMap to Properties + StringMap<String> p = (StringMap<String>) setting.getProperties(); + Properties properties = new Properties(); + for (String key : p.keySet()) { + properties.put(key, p.get(key)); + } + setting.setProperties(properties); + // Always use separate interpreter process // While we decided to turn this feature on always (without providing // enable/disable option on GUI). @@ -615,11 +617,12 @@ public class InterpreterFactory implements InterpreterGroupFactory { } private InterpreterSetting add(String group, InterpreterInfo interpreterInfo, - Properties properties, String path) + Map<String, InterpreterProperty> interpreterProperties, String path) throws InterpreterException, IOException, RepositoryException { ArrayList<InterpreterInfo> infos = new ArrayList<>(); infos.add(interpreterInfo); - return add(group, infos, new ArrayList<Dependency>(), defaultOption, properties, path); + return add(group, infos, new ArrayList<Dependency>(), defaultOption, + interpreterProperties, path); } /** @@ -627,12 +630,13 @@ public class InterpreterFactory implements InterpreterGroupFactory { * @return */ public InterpreterSetting add(String group, ArrayList<InterpreterInfo> interpreterInfos, - List<Dependency> dependencies, InterpreterOption option, Properties properties, String path) { + List<Dependency> dependencies, InterpreterOption option, + Map<String, InterpreterProperty> interpreterProperties, 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"); Preconditions.checkNotNull(option, "option should not be null"); - Preconditions.checkNotNull(properties, "properties should not be null"); + Preconditions.checkNotNull(interpreterProperties, "properties should not be null"); InterpreterSetting interpreterSetting; @@ -663,17 +667,18 @@ public class InterpreterFactory implements InterpreterGroupFactory { } // Append properties - Properties interpreterProperties = interpreterSetting.getProperties(); - for (String key : properties.stringPropertyNames()) { - if (!interpreterProperties.containsKey(key)) { - interpreterProperties.setProperty(key, properties.getProperty(key)); + Map<String, InterpreterProperty> properties = + (Map<String, InterpreterProperty>) interpreterSetting.getProperties(); + for (String key : interpreterProperties.keySet()) { + if (!properties.containsKey(key)) { + properties.put(key, interpreterProperties.get(key)); } } } else { interpreterSetting = - new InterpreterSetting(group, null, interpreterInfos, properties, dependencies, option, - path); + new InterpreterSetting(group, null, interpreterInfos, interpreterProperties, + dependencies, option, path); interpreterSettingsRef.put(group, interpreterSetting); } } @@ -734,7 +739,7 @@ public class InterpreterFactory implements InterpreterGroupFactory { String noteId, String key) { InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup(user, noteId); InterpreterOption option = interpreterSetting.getOption(); - Properties properties = interpreterSetting.getProperties(); + Properties properties = (Properties) interpreterSetting.getProperties(); if (option.isExistingProcess) { properties.put(Constants.ZEPPELIN_INTERPRETER_HOST, option.getHost()); properties.put(Constants.ZEPPELIN_INTERPRETER_PORT, option.getPort()); @@ -932,16 +937,16 @@ public class InterpreterFactory implements InterpreterGroupFactory { public void setPropertyAndRestart(String id, InterpreterOption option, Properties properties, List<Dependency> dependencies) throws IOException { synchronized (interpreterSettings) { - InterpreterSetting intpsetting = interpreterSettings.get(id); - if (intpsetting != null) { + InterpreterSetting intpSetting = interpreterSettings.get(id); + if (intpSetting != null) { try { - stopJobAllInterpreter(intpsetting); + stopJobAllInterpreter(intpSetting); - intpsetting.closeAndRmoveAllInterpreterGroups(); - intpsetting.setOption(option); - intpsetting.setProperties(properties); - intpsetting.setDependencies(dependencies); - loadInterpreterDependencies(intpsetting); + intpSetting.closeAndRmoveAllInterpreterGroups(); + intpSetting.setOption(option); + intpSetting.setProperties(properties); + intpSetting.setDependencies(dependencies); + loadInterpreterDependencies(intpSetting); saveToFile(); } catch (Exception e) { @@ -960,12 +965,11 @@ public class InterpreterFactory implements InterpreterGroupFactory { } public void restart(String settingId, String noteId) { - InterpreterSetting intpsetting = interpreterSettings.get(settingId); - Preconditions.checkNotNull(intpsetting); + InterpreterSetting intpSetting = interpreterSettings.get(settingId); + Preconditions.checkNotNull(intpSetting); - if (noteIdIsExist(noteId) && - intpsetting.getOption().isProcess()) { - intpsetting.closeAndRemoveInterpreterGroup(noteId); + if (noteIdIsExist(noteId) && intpSetting.getOption().isProcess()) { + intpSetting.closeAndRemoveInterpreterGroup(noteId); return; } restart(settingId); @@ -973,15 +977,15 @@ public class InterpreterFactory implements InterpreterGroupFactory { public void restart(String id) { synchronized (interpreterSettings) { - InterpreterSetting intpsetting = interpreterSettings.get(id); + InterpreterSetting intpSetting = interpreterSettings.get(id); // Check if dependency in specified path is changed // If it did, overwrite old dependency jar with new one - if (intpsetting != null) { - copyDependenciesFromLocalPath(intpsetting); + if (intpSetting != null) { + copyDependenciesFromLocalPath(intpSetting); - stopJobAllInterpreter(intpsetting); + stopJobAllInterpreter(intpSetting); - intpsetting.closeAndRmoveAllInterpreterGroups(); + intpSetting.closeAndRmoveAllInterpreterGroups(); } else { throw new InterpreterException("Interpreter setting id " + id + " not found"); @@ -989,9 +993,9 @@ public class InterpreterFactory implements InterpreterGroupFactory { } } - private void stopJobAllInterpreter(InterpreterSetting intpsetting) { - if (intpsetting != null) { - for (InterpreterGroup intpGroup : intpsetting.getAllInterpreterGroups()) { + private void stopJobAllInterpreter(InterpreterSetting intpSetting) { + if (intpSetting != null) { + for (InterpreterGroup intpGroup : intpSetting.getAllInterpreterGroups()) { for (List<Interpreter> interpreters : intpGroup.values()) { for (Interpreter intp : interpreters) { for (Job job : intp.getScheduler().getJobsRunning()) { @@ -1013,11 +1017,11 @@ public class InterpreterFactory implements InterpreterGroupFactory { public void close() { List<Thread> closeThreads = new LinkedList<>(); synchronized (interpreterSettings) { - Collection<InterpreterSetting> intpsettings = interpreterSettings.values(); - for (final InterpreterSetting intpsetting : intpsettings) { + Collection<InterpreterSetting> intpSettings = interpreterSettings.values(); + for (final InterpreterSetting intpSetting : intpSettings) { Thread t = new Thread() { public void run() { - intpsetting.closeAndRmoveAllInterpreterGroups(); + intpSetting.closeAndRmoveAllInterpreterGroups(); } }; t.start(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/28adacb9/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 4611559..47d0ef9 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 @@ -42,8 +42,19 @@ public class InterpreterSetting { private static final String SHARED_PROCESS = "shared_process"; private String id; private String name; - private String group; // always be null in case of InterpreterSettingRef - private Properties properties; + // always be null in case of InterpreterSettingRef + private String group; + /** + * properties can be either Properties or Map<String, InterpreterProperty> + * properties should be: + * - Properties when Interpreter instances are saved to `conf/interpreter.json` file + * - Map<String, InterpreterProperty> when Interpreters are registered + * : this is needed after https://github.com/apache/zeppelin/pull/1145 + * which changed the way of getting default interpreter setting AKA interpreterSettingsRef + * Note(mina): In order to simplify the implementation, I chose to change properties + * from Properties to Object instead of creating new classes. + */ + private Object properties; private Status status; private String errorReason; @@ -65,7 +76,7 @@ public class InterpreterSetting { } public InterpreterSetting(String id, String name, String group, - List<InterpreterInfo> interpreterInfos, Properties properties, List<Dependency> dependencies, + List<InterpreterInfo> interpreterInfos, Object properties, List<Dependency> dependencies, InterpreterOption option, String path) { this(); this.id = id; @@ -80,7 +91,7 @@ public class InterpreterSetting { } public InterpreterSetting(String name, String group, List<InterpreterInfo> interpreterInfos, - Properties properties, List<Dependency> dependencies, InterpreterOption option, String path) { + Object properties, List<Dependency> dependencies, InterpreterOption option, String path) { this(generateId(), name, group, interpreterInfos, properties, dependencies, option, path); } @@ -174,7 +185,7 @@ public class InterpreterSetting { } } - public Properties getProperties() { + public Object getProperties() { return properties; } @@ -229,11 +240,7 @@ public class InterpreterSetting { this.option = interpreterOption; } - void updateProperties(Properties p) { - this.properties.putAll(p); - } - - void setProperties(Properties p) { + public void setProperties(Properties p) { this.properties = p; } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/28adacb9/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java index f15cdd0..9b3b3ef 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java @@ -18,13 +18,17 @@ package org.apache.zeppelin.interpreter; import java.io.*; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.HashMap; import java.util.Properties; +import com.google.gson.Gson; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.NullArgumentException; import org.apache.zeppelin.conf.ZeppelinConfiguration; @@ -33,7 +37,6 @@ import org.apache.zeppelin.dep.Dependency; import org.apache.zeppelin.dep.DependencyResolver; import org.apache.zeppelin.interpreter.mock.MockInterpreter1; import org.apache.zeppelin.interpreter.mock.MockInterpreter2; -import org.apache.zeppelin.notebook.repo.zeppelinhub.security.Authentication; import org.apache.zeppelin.user.AuthenticationInfo; import org.apache.zeppelin.interpreter.remote.RemoteInterpreter; import org.apache.zeppelin.notebook.JobListenerFactory; @@ -165,12 +168,12 @@ public class InterpreterFactoryTest { List<String> all = factory.getDefaultInterpreterSettingList(); // add setting with null option & properties expected nullArgumentException.class try { - factory.add("mock2", new ArrayList<InterpreterInfo>(), new LinkedList<Dependency>(), new InterpreterOption(false), new Properties(), ""); + factory.add("mock2", new ArrayList<InterpreterInfo>(), new LinkedList<Dependency>(), new InterpreterOption(false), Collections.EMPTY_MAP, ""); } catch(NullArgumentException e) { assertEquals("Test null option" , e.getMessage(),new NullArgumentException("option").getMessage()); } try { - factory.add("mock2", new ArrayList<InterpreterInfo>(), new LinkedList<Dependency>(), new InterpreterOption(false), new Properties(), ""); + factory.add("mock2", new ArrayList<InterpreterInfo>(), new LinkedList<Dependency>(), new InterpreterOption(false), Collections.EMPTY_MAP, ""); } catch (NullArgumentException e){ assertEquals("Test null properties" , e.getMessage(),new NullArgumentException("properties").getMessage()); } @@ -193,16 +196,47 @@ public class InterpreterFactoryTest { } @Test + public void testInterpreterSettingPropertyClass() throws IOException, RepositoryException { + // check if default interpreter reference's property type is map + Map<String, InterpreterSetting> interpreterSettingRefs = factory.getAvailableInterpreterSettings(); + InterpreterSetting intpSetting = interpreterSettingRefs.get("mock1"); + Map<String, InterpreterProperty> intpProperties = + (Map<String, InterpreterProperty>) intpSetting.getProperties(); + assertTrue(intpProperties instanceof Map); + + // check if interpreter instance is saved as Properties in conf/interpreter.json file + Properties properties = new Properties(); + properties.put("key1", "value1"); + properties.put("key2", "value2"); + + factory.createNewSetting("newMock", "mock1", new LinkedList<Dependency>(), new InterpreterOption(false), properties); + + String confFilePath = conf.getInterpreterSettingPath(); + byte[] encoded = Files.readAllBytes(Paths.get(confFilePath)); + String json = new String(encoded, "UTF-8"); + + Gson gson = new Gson(); + InterpreterInfoSaving infoSaving = gson.fromJson(json, InterpreterInfoSaving.class); + Map<String, InterpreterSetting> interpreterSettings = infoSaving.interpreterSettings; + for (String key : interpreterSettings.keySet()) { + InterpreterSetting setting = interpreterSettings.get(key); + if (setting.getName().equals("newMock")) { + assertEquals(setting.getProperties().toString(), properties.toString()); + } + } + } + + @Test public void testInterpreterAliases() throws IOException, RepositoryException { factory = new InterpreterFactory(conf, null, null, null, depResolver, false); final InterpreterInfo info1 = new InterpreterInfo("className1", "name1", true, null); final InterpreterInfo info2 = new InterpreterInfo("className2", "name1", true, null); - factory.add("group1", new ArrayList<InterpreterInfo>(){{ + factory.add("group1", new ArrayList<InterpreterInfo>() {{ add(info1); - }}, new ArrayList<Dependency>(), new InterpreterOption(true), new Properties(), "/path1"); + }}, new ArrayList<Dependency>(), new InterpreterOption(true), Collections.EMPTY_MAP, "/path1"); factory.add("group2", new ArrayList<InterpreterInfo>(){{ add(info2); - }}, new ArrayList<Dependency>(), new InterpreterOption(true), new Properties(), "/path2"); + }}, new ArrayList<Dependency>(), new InterpreterOption(true), Collections.EMPTY_MAP, "/path2"); final InterpreterSetting setting1 = factory.createNewSetting("test-group1", "group1", new ArrayList<Dependency>(), new InterpreterOption(true), new Properties()); final InterpreterSetting setting2 = factory.createNewSetting("test-group2", "group1", new ArrayList<Dependency>(), new InterpreterOption(true), new Properties()); @@ -222,7 +256,7 @@ public class InterpreterFactoryTest { final InterpreterInfo info1 = new InterpreterInfo("className1", "name1", true, null); factory.add("group1", new ArrayList<InterpreterInfo>(){{ add(info1); - }}, new ArrayList<Dependency>(), new InterpreterOption(true), new Properties(), "/path1"); + }}, new ArrayList<Dependency>(), new InterpreterOption(true), Collections.EMPTY_MAP, "/path1"); InterpreterOption perUserInterpreterOption = new InterpreterOption(true, InterpreterOption.ISOLATED, InterpreterOption.SHARED); final InterpreterSetting setting1 = factory.createNewSetting("test-group1", "group1", new ArrayList<Dependency>(), perUserInterpreterOption, new Properties());
