Skip to content

Commit 1a2a41d

Browse files
minahleeminahlee
committed
Show description when create new interpreter
1 parent 2f79852 commit 1a2a41d

File tree

5 files changed

+85
-89
lines changed

5 files changed

+85
-89
lines changed

‎zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.net.InetAddress;
2424
import java.net.UnknownHostException;
2525
import java.util.List;
26+
import java.util.Properties;
2627
import java.util.concurrent.ExecutorService;
2728
import java.util.concurrent.Executors;
2829

@@ -140,47 +141,41 @@ protected static void startUp() throws Exception {
140141
LOG.info("Test Zeppelin stared.");
141142

142143

144+
// assume first one is spark
145+
InterpreterSetting sparkIntpSetting = null;
146+
for(InterpreterSetting intpSetting : ZeppelinServer.notebook.getInterpreterFactory().get()) {
147+
if (intpSetting.getName().equals("spark")) {
148+
sparkIntpSetting = intpSetting;
149+
}
150+
}
151+
152+
Properties sparkProperties = (Properties) sparkIntpSetting.getProperties();
143153
// ci environment runs spark cluster for testing
144154
// so configure zeppelin use spark cluster
145155
if ("true".equals(System.getenv("CI"))) {
146-
// assume first one is spark
147-
InterpreterSetting sparkIntpSetting = null;
148-
for(InterpreterSetting intpSetting : ZeppelinServer.notebook.getInterpreterFactory().get()) {
149-
if (intpSetting.getName().equals("spark")) {
150-
sparkIntpSetting = intpSetting;
151-
}
152-
}
153-
154156
// set spark master and other properties
155-
sparkIntpSetting.getProperties().setProperty("master", "local[2]");
156-
sparkIntpSetting.getProperties().setProperty("spark.cores.max", "2");
157-
sparkIntpSetting.getProperties().setProperty("zeppelin.spark.useHiveContext", "false");
157+
sparkProperties.setProperty("master", "local[2]");
158+
sparkProperties.setProperty("spark.cores.max", "2");
159+
sparkProperties.setProperty("zeppelin.spark.useHiveContext", "false");
158160
// set spark home for pyspark
159-
sparkIntpSetting.getProperties().setProperty("spark.home", getSparkHome());
161+
sparkProperties.setProperty("spark.home", getSparkHome());
162+
163+
sparkIntpSetting.setProperties(sparkProperties);
160164
pySpark = true;
161165
sparkR = true;
162166
ZeppelinServer.notebook.getInterpreterFactory().restart(sparkIntpSetting.getId());
163167
} else {
164-
// assume first one is spark
165-
InterpreterSetting sparkIntpSetting = null;
166-
for(InterpreterSetting intpSetting : ZeppelinServer.notebook.getInterpreterFactory().get()) {
167-
if (intpSetting.getName().equals("spark")) {
168-
sparkIntpSetting = intpSetting;
169-
}
170-
}
171-
172168
String sparkHome = getSparkHome();
173169
if (sparkHome != null) {
174170
if (System.getenv("SPARK_MASTER") != null) {
175-
sparkIntpSetting.getProperties().setProperty("master", System.getenv("SPARK_MASTER"));
171+
sparkProperties.setProperty("master", System.getenv("SPARK_MASTER"));
176172
} else {
177-
sparkIntpSetting.getProperties()
178-
.setProperty("master", "local[2]");
173+
sparkProperties.setProperty("master", "local[2]");
179174
}
180-
sparkIntpSetting.getProperties().setProperty("spark.cores.max", "2");
175+
sparkProperties.setProperty("spark.cores.max", "2");
181176
// set spark home for pyspark
182-
sparkIntpSetting.getProperties().setProperty("spark.home", sparkHome);
183-
sparkIntpSetting.getProperties().setProperty("zeppelin.spark.useHiveContext", "false");
177+
sparkProperties.setProperty("spark.home", sparkHome);
178+
sparkProperties.setProperty("zeppelin.spark.useHiveContext", "false");
184179
pySpark = true;
185180
sparkR = true;
186181
}

‎zeppelin-web/src/app/interpreter/interpreter.controller.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -399,12 +399,11 @@
399399
var intpInfo = el[i];
400400
for (var key in intpInfo) {
401401
properties[key] = {
402-
value: intpInfo[key],
402+
value: intpInfo[key].defaultValue,
403403
description: intpInfo[key].description
404404
};
405405
}
406406
}
407-
408407
$scope.newInterpreterSetting.properties = properties;
409408
};
410409

‎zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java‎

Lines changed: 40 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,7 @@ public boolean accept(Path entry) throws IOException {
237237
interpreterInfo =
238238
new InterpreterInfo(r.getClassName(), r.getName(), r.isDefaultInterpreter(),
239239
r.getEditor());
240-
add(r.getGroup(), interpreterInfo, convertInterpreterProperties(r.getProperties()),
241-
r.getPath());
240+
add(r.getGroup(), interpreterInfo, r.getProperties(), r.getPath());
242241
}
243242

244243
for (String settingId : interpreterSettingsRef.keySet()) {
@@ -286,7 +285,8 @@ private InterpreterSetting createFromInterpreterSettingRef(String name) {
286285

287286
private InterpreterSetting createFromInterpreterSettingRef(InterpreterSetting o) {
288287
InterpreterSetting setting =
289-
new InterpreterSetting(o.getName(), o.getName(), o.getInterpreterInfos(), o.getProperties(),
288+
new InterpreterSetting(o.getName(), o.getName(), o.getInterpreterInfos(),
289+
convertInterpreterProperties((Map <String, InterpreterProperty>)o.getProperties()),
290290
o.getDependencies(), o.getOption(), o.getPath());
291291
setting.setInterpreterGroupFactory(this);
292292
return setting;
@@ -349,16 +349,9 @@ private void registerInterpreters(List<RegisteredInterpreter> registeredInterpre
349349
InterpreterInfo interpreterInfo =
350350
new InterpreterInfo(registeredInterpreter.getClassName(), registeredInterpreter.getName(),
351351
registeredInterpreter.isDefaultInterpreter(), registeredInterpreter.getEditor());
352-
Properties properties = new Properties();
353-
Map<String, InterpreterProperty> p = registeredInterpreter.getProperties();
354352

355-
if (null != p) {
356-
for (String key : p.keySet()) {
357-
properties.setProperty(key, p.get(key).getValue());
358-
}
359-
}
360-
361-
add(registeredInterpreter.getGroup(), interpreterInfo, properties, absolutePath);
353+
add(registeredInterpreter.getGroup(), interpreterInfo, registeredInterpreter.getProperties(),
354+
absolutePath);
362355
}
363356

364357
}
@@ -615,24 +608,25 @@ public InterpreterSetting createNewSetting(String name, String group,
615608
}
616609

617610
private InterpreterSetting add(String group, InterpreterInfo interpreterInfo,
618-
Properties properties, String path)
611+
Map<String, InterpreterProperty> interpreterProperties, String path)
619612
throws InterpreterException, IOException, RepositoryException {
620613
ArrayList<InterpreterInfo> infos = new ArrayList<>();
621614
infos.add(interpreterInfo);
622-
return add(group, infos, new ArrayList<Dependency>(), defaultOption, properties, path);
615+
return add(group, infos, new ArrayList<Dependency>(), defaultOption, interpreterProperties, path);
623616
}
624617

625618
/**
626619
* @param group InterpreterSetting reference name
627620
* @return
628621
*/
629622
public InterpreterSetting add(String group, ArrayList<InterpreterInfo> interpreterInfos,
630-
List<Dependency> dependencies, InterpreterOption option, Properties properties, String path) {
623+
List<Dependency> dependencies, InterpreterOption option,
624+
Map<String, InterpreterProperty> interpreterProperties, String path) {
631625
Preconditions.checkNotNull(group, "name should not be null");
632626
Preconditions.checkNotNull(interpreterInfos, "interpreterInfos should not be null");
633627
Preconditions.checkNotNull(dependencies, "dependencies should not be null");
634628
Preconditions.checkNotNull(option, "option should not be null");
635-
Preconditions.checkNotNull(properties, "properties should not be null");
629+
Preconditions.checkNotNull(interpreterProperties, "properties should not be null");
636630

637631
InterpreterSetting interpreterSetting;
638632

@@ -663,16 +657,17 @@ public InterpreterSetting add(String group, ArrayList<InterpreterInfo> interpret
663657
}
664658

665659
// Append properties
666-
Properties interpreterProperties = interpreterSetting.getProperties();
667-
for (String key : properties.stringPropertyNames()) {
668-
if (!interpreterProperties.containsKey(key)) {
669-
interpreterProperties.setProperty(key, properties.getProperty(key));
660+
Map<String, InterpreterProperty> properties =
661+
(Map<String, InterpreterProperty>) interpreterSetting.getProperties();
662+
for (String key : interpreterProperties.keySet()) {
663+
if (!properties.containsKey(key)) {
664+
properties.put(key, interpreterProperties.get(key));
670665
}
671666
}
672667

673668
} else {
674669
interpreterSetting =
675-
new InterpreterSetting(group, null, interpreterInfos, properties, dependencies, option,
670+
new InterpreterSetting(group, null, interpreterInfos, interpreterProperties, dependencies, option,
676671
path);
677672
interpreterSettingsRef.put(group, interpreterSetting);
678673
}
@@ -734,7 +729,7 @@ public void createInterpretersForNote(InterpreterSetting interpreterSetting, Str
734729
String noteId, String key) {
735730
InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup(user, noteId);
736731
InterpreterOption option = interpreterSetting.getOption();
737-
Properties properties = interpreterSetting.getProperties();
732+
Properties properties = (Properties) interpreterSetting.getProperties();
738733
if (option.isExistingProcess) {
739734
properties.put(Constants.ZEPPELIN_INTERPRETER_HOST, option.getHost());
740735
properties.put(Constants.ZEPPELIN_INTERPRETER_PORT, option.getPort());
@@ -932,16 +927,16 @@ private List<String> getNoteInterpreterSettingBinding(String noteId) {
932927
public void setPropertyAndRestart(String id, InterpreterOption option, Properties properties,
933928
List<Dependency> dependencies) throws IOException {
934929
synchronized (interpreterSettings) {
935-
InterpreterSetting intpsetting = interpreterSettings.get(id);
936-
if (intpsetting != null) {
930+
InterpreterSetting intpSetting = interpreterSettings.get(id);
931+
if (intpSetting != null) {
937932
try {
938-
stopJobAllInterpreter(intpsetting);
933+
stopJobAllInterpreter(intpSetting);
939934

940-
intpsetting.closeAndRmoveAllInterpreterGroups();
941-
intpsetting.setOption(option);
942-
intpsetting.setProperties(properties);
943-
intpsetting.setDependencies(dependencies);
944-
loadInterpreterDependencies(intpsetting);
935+
intpSetting.closeAndRmoveAllInterpreterGroups();
936+
intpSetting.setOption(option);
937+
intpSetting.setProperties(properties);
938+
intpSetting.setDependencies(dependencies);
939+
loadInterpreterDependencies(intpSetting);
945940

946941
saveToFile();
947942
} catch (Exception e) {
@@ -960,38 +955,37 @@ private boolean noteIdIsExist(String noteId) {
960955
}
961956

962957
public void restart(String settingId, String noteId) {
963-
InterpreterSetting intpsetting = interpreterSettings.get(settingId);
964-
Preconditions.checkNotNull(intpsetting);
958+
InterpreterSetting intpSetting = interpreterSettings.get(settingId);
959+
Preconditions.checkNotNull(intpSetting);
965960

966-
if (noteIdIsExist(noteId) &&
967-
intpsetting.getOption().isProcess()) {
968-
intpsetting.closeAndRemoveInterpreterGroup(noteId);
961+
if (noteIdIsExist(noteId) && intpSetting.getOption().isProcess()) {
962+
intpSetting.closeAndRemoveInterpreterGroup(noteId);
969963
return;
970964
}
971965
restart(settingId);
972966
}
973967

974968
public void restart(String id) {
975969
synchronized (interpreterSettings) {
976-
InterpreterSetting intpsetting = interpreterSettings.get(id);
970+
InterpreterSetting intpSetting = interpreterSettings.get(id);
977971
// Check if dependency in specified path is changed
978972
// If it did, overwrite old dependency jar with new one
979-
if (intpsetting != null) {
980-
copyDependenciesFromLocalPath(intpsetting);
973+
if (intpSetting != null) {
974+
copyDependenciesFromLocalPath(intpSetting);
981975

982-
stopJobAllInterpreter(intpsetting);
976+
stopJobAllInterpreter(intpSetting);
983977

984-
intpsetting.closeAndRmoveAllInterpreterGroups();
978+
intpSetting.closeAndRmoveAllInterpreterGroups();
985979

986980
} else {
987981
throw new InterpreterException("Interpreter setting id " + id + " not found");
988982
}
989983
}
990984
}
991985

992-
private void stopJobAllInterpreter(InterpreterSetting intpsetting) {
993-
if (intpsetting != null) {
994-
for (InterpreterGroup intpGroup : intpsetting.getAllInterpreterGroups()) {
986+
private void stopJobAllInterpreter(InterpreterSetting intpSetting) {
987+
if (intpSetting != null) {
988+
for (InterpreterGroup intpGroup : intpSetting.getAllInterpreterGroups()) {
995989
for (List<Interpreter> interpreters : intpGroup.values()) {
996990
for (Interpreter intp : interpreters) {
997991
for (Job job : intp.getScheduler().getJobsRunning()) {
@@ -1013,11 +1007,11 @@ private void stopJobAllInterpreter(InterpreterSetting intpsetting) {
10131007
public void close() {
10141008
List<Thread> closeThreads = new LinkedList<>();
10151009
synchronized (interpreterSettings) {
1016-
Collection<InterpreterSetting> intpsettings = interpreterSettings.values();
1017-
for (final InterpreterSetting intpsetting : intpsettings) {
1010+
Collection<InterpreterSetting> intpSettings = interpreterSettings.values();
1011+
for (final InterpreterSetting intpSetting : intpSettings) {
10181012
Thread t = new Thread() {
10191013
public void run() {
1020-
intpsetting.closeAndRmoveAllInterpreterGroups();
1014+
intpSetting.closeAndRmoveAllInterpreterGroups();
10211015
}
10221016
};
10231017
t.start();

‎zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,19 @@ public class InterpreterSetting {
4242
private static final String SHARED_PROCESS = "shared_process";
4343
private String id;
4444
private String name;
45-
private String group; // always be null in case of InterpreterSettingRef
46-
private Properties properties;
45+
// always be null in case of InterpreterSettingRef
46+
private String group;
47+
/**
48+
* properties can be either Properties or Map<String, InterpreterProperty>
49+
* properties should be:
50+
* - Properties when Interpreter instances are saved to `conf/interpreter.json` file
51+
* - Map<String, InterpreterProperty> when Interpreters are registered
52+
* : this is needed after https://github.com/apache/zeppelin/pull/1145
53+
* which changed the way of getting default interpreter setting AKA interpreterSettingsRef
54+
* Note(mina): In order to simplify the implementation, I chose to change properties
55+
* from Properties to Object instead of creating new classes.
56+
*/
57+
private Object properties;
4758
private Status status;
4859
private String errorReason;
4960

@@ -65,7 +76,7 @@ public InterpreterSetting() {
6576
}
6677

6778
public InterpreterSetting(String id, String name, String group,
68-
List<InterpreterInfo> interpreterInfos, Properties properties, List<Dependency> dependencies,
79+
List<InterpreterInfo> interpreterInfos, Object properties, List<Dependency> dependencies,
6980
InterpreterOption option, String path) {
7081
this();
7182
this.id = id;
@@ -80,7 +91,7 @@ public InterpreterSetting(String id, String name, String group,
8091
}
8192

8293
public InterpreterSetting(String name, String group, List<InterpreterInfo> interpreterInfos,
83-
Properties properties, List<Dependency> dependencies, InterpreterOption option, String path) {
94+
Object properties, List<Dependency> dependencies, InterpreterOption option, String path) {
8495
this(generateId(), name, group, interpreterInfos, properties, dependencies, option, path);
8596
}
8697

@@ -174,7 +185,7 @@ void closeAndRmoveAllInterpreterGroups() {
174185
}
175186
}
176187

177-
public Properties getProperties() {
188+
public Object getProperties() {
178189
return properties;
179190
}
180191

@@ -229,11 +240,7 @@ void setInterpreterOption(InterpreterOption interpreterOption) {
229240
this.option = interpreterOption;
230241
}
231242

232-
void updateProperties(Properties p) {
233-
this.properties.putAll(p);
234-
}
235-
236-
void setProperties(Properties p) {
243+
public void setProperties(Properties p) {
237244
this.properties = p;
238245
}
239246

‎zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import java.io.*;
2121
import java.util.ArrayList;
22+
import java.util.Collections;
2223
import java.util.LinkedList;
2324
import java.util.List;
2425
import java.util.Map;
@@ -165,12 +166,12 @@ public void testExceptions() throws InterpreterException, IOException, Repositor
165166
List<String> all = factory.getDefaultInterpreterSettingList();
166167
// add setting with null option & properties expected nullArgumentException.class
167168
try {
168-
factory.add("mock2", new ArrayList<InterpreterInfo>(), new LinkedList<Dependency>(), new InterpreterOption(false), new Properties(), "");
169+
factory.add("mock2", new ArrayList<InterpreterInfo>(), new LinkedList<Dependency>(), new InterpreterOption(false), Collections.EMPTY_MAP, "");
169170
} catch(NullArgumentException e) {
170171
assertEquals("Test null option" , e.getMessage(),new NullArgumentException("option").getMessage());
171172
}
172173
try {
173-
factory.add("mock2", new ArrayList<InterpreterInfo>(), new LinkedList<Dependency>(), new InterpreterOption(false), new Properties(), "");
174+
factory.add("mock2", new ArrayList<InterpreterInfo>(), new LinkedList<Dependency>(), new InterpreterOption(false), Collections.EMPTY_MAP, "");
174175
} catch (NullArgumentException e){
175176
assertEquals("Test null properties" , e.getMessage(),new NullArgumentException("properties").getMessage());
176177
}
@@ -199,10 +200,10 @@ public void testInterpreterAliases() throws IOException, RepositoryException {
199200
final InterpreterInfo info2 = new InterpreterInfo("className2", "name1", true, null);
200201
factory.add("group1", new ArrayList<InterpreterInfo>(){{
201202
add(info1);
202-
}}, new ArrayList<Dependency>(), new InterpreterOption(true), new Properties(), "/path1");
203+
}}, new ArrayList<Dependency>(), new InterpreterOption(true), Collections.EMPTY_MAP, "/path1");
203204
factory.add("group2", new ArrayList<InterpreterInfo>(){{
204205
add(info2);
205-
}}, new ArrayList<Dependency>(), new InterpreterOption(true), new Properties(), "/path2");
206+
}}, new ArrayList<Dependency>(), new InterpreterOption(true), Collections.EMPTY_MAP, "/path2");
206207

207208
final InterpreterSetting setting1 = factory.createNewSetting("test-group1", "group1", new ArrayList<Dependency>(), new InterpreterOption(true), new Properties());
208209
final InterpreterSetting setting2 = factory.createNewSetting("test-group2", "group1", new ArrayList<Dependency>(), new InterpreterOption(true), new Properties());
@@ -222,7 +223,7 @@ public void testMultiUser() throws IOException, RepositoryException {
222223
final InterpreterInfo info1 = new InterpreterInfo("className1", "name1", true, null);
223224
factory.add("group1", new ArrayList<InterpreterInfo>(){{
224225
add(info1);
225-
}}, new ArrayList<Dependency>(), new InterpreterOption(true), new Properties(), "/path1");
226+
}}, new ArrayList<Dependency>(), new InterpreterOption(true), Collections.EMPTY_MAP, "/path1");
226227

227228
InterpreterOption perUserInterpreterOption = new InterpreterOption(true, InterpreterOption.ISOLATED, InterpreterOption.SHARED);
228229
final InterpreterSetting setting1 = factory.createNewSetting("test-group1", "group1", new ArrayList<Dependency>(), perUserInterpreterOption, new Properties());

0 commit comments

Comments
 (0)