-
Notifications
You must be signed in to change notification settings - Fork 2.8k
ZEPPELIN-3085 Introduce generic ConfInterpreter for more fine-grained control of interpreter setting #2692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ZEPPELIN-3085 Introduce generic ConfInterpreter for more fine-grained control of interpreter setting #2692
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| /* | ||
| * 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.interpreter; | ||
|
|
||
| import org.apache.commons.lang.exception.ExceptionUtils; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.StringReader; | ||
| import java.util.Properties; | ||
|
|
||
| /** | ||
| * Special Interpreter for Interpreter Configuration customization. It is attached to each | ||
| * InterpreterGroup implicitly by Zeppelin. | ||
| */ | ||
| public class ConfInterpreter extends Interpreter { | ||
|
|
||
| private static Logger LOGGER = LoggerFactory.getLogger(ConfInterpreter.class); | ||
|
|
||
| private String interpreterGroupId; | ||
| private InterpreterSetting interpreterSetting; | ||
|
|
||
|
|
||
| public ConfInterpreter(Properties properties, | ||
| String interpreterGroupId, | ||
| InterpreterSetting interpreterSetting) { | ||
| super(properties); | ||
| this.interpreterGroupId = interpreterGroupId; | ||
| this.interpreterSetting = interpreterSetting; | ||
| } | ||
|
|
||
| @Override | ||
| public void open() throws InterpreterException { | ||
|
|
||
| } | ||
|
|
||
| @Override | ||
| public void close() throws InterpreterException { | ||
|
|
||
| } | ||
|
|
||
| @Override | ||
| public InterpreterResult interpret(String st, InterpreterContext context) | ||
| throws InterpreterException { | ||
|
|
||
| try { | ||
| Properties finalProperties = new Properties(); | ||
| finalProperties.putAll(getProperties()); | ||
| Properties newProperties = new Properties(); | ||
| newProperties.load(new StringReader(st)); | ||
| finalProperties.putAll(newProperties); | ||
| LOGGER.debug("Properties for InterpreterGroup: " + interpreterGroupId + " is " | ||
| + finalProperties); | ||
| interpreterSetting.setInterpreterGroupProperties(interpreterGroupId, finalProperties); | ||
| return new InterpreterResult(InterpreterResult.Code.SUCCESS); | ||
| } catch (IOException e) { | ||
| LOGGER.error("Fail to update interpreter setting", e); | ||
| return new InterpreterResult(InterpreterResult.Code.ERROR, ExceptionUtils.getStackTrace(e)); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void cancel(InterpreterContext context) throws InterpreterException { | ||
|
|
||
| } | ||
|
|
||
| @Override | ||
| public FormType getFormType() throws InterpreterException { | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public int getProgress(InterpreterContext context) throws InterpreterException { | ||
| return 0; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,9 +138,11 @@ public class InterpreterSetting { | |
| // launcher in future when we have other launcher implementation. e.g. third party launcher | ||
| // service like livy | ||
| private transient InterpreterLauncher launcher; | ||
| /////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| private transient LifecycleManager lifecycleManager; | ||
| /////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
|
|
||
|
|
||
| /** | ||
| * Builder class for InterpreterSetting | ||
|
|
@@ -648,12 +650,11 @@ public void clearNoteIdAndParaMap() { | |
| /////////////////////////////////////////////////////////////////////////////////////// | ||
| // This is the only place to create interpreters. For now we always create multiple interpreter | ||
| // together (one session). We don't support to create single interpreter yet. | ||
| List<Interpreter> createInterpreters(String user, String sessionId) { | ||
| List<Interpreter> createInterpreters(String user, String interpreterGroupId, String sessionId) { | ||
| List<Interpreter> interpreters = new ArrayList<>(); | ||
| List<InterpreterInfo> interpreterInfos = getInterpreterInfos(); | ||
| for (InterpreterInfo info : interpreterInfos) { | ||
| Interpreter interpreter = null; | ||
| interpreter = new RemoteInterpreter(getJavaProperties(), sessionId, | ||
| Interpreter interpreter = new RemoteInterpreter(getJavaProperties(), sessionId, | ||
| info.getClassName(), user, lifecycleManager); | ||
| if (info.isDefaultInterpreter()) { | ||
| interpreters.add(0, interpreter); | ||
|
|
@@ -663,15 +664,17 @@ List<Interpreter> createInterpreters(String user, String sessionId) { | |
| LOGGER.info("Interpreter {} created for user: {}, sessionId: {}", | ||
| interpreter.getClassName(), user, sessionId); | ||
| } | ||
| interpreters.add(new ConfInterpreter(getJavaProperties(), interpreterGroupId, this)); | ||
| return interpreters; | ||
| } | ||
|
|
||
| synchronized RemoteInterpreterProcess createInterpreterProcess() throws IOException { | ||
| synchronized RemoteInterpreterProcess createInterpreterProcess(Properties properties) | ||
| throws IOException { | ||
| if (launcher == null) { | ||
| createLauncher(); | ||
| } | ||
| InterpreterLaunchContext launchContext = new | ||
| InterpreterLaunchContext(getJavaProperties(), option, interpreterRunner, id, group, name); | ||
| InterpreterLaunchContext(properties, option, interpreterRunner, id, group, name); | ||
| RemoteInterpreterProcess process = (RemoteInterpreterProcess) launcher.launch(launchContext); | ||
| process.setRemoteInterpreterEventPoller( | ||
| new RemoteInterpreterEventPoller(remoteInterpreterProcessListener, appEventListener)); | ||
|
|
@@ -716,6 +719,11 @@ private String getInterpreterClassFromInterpreterSetting(String replName) { | |
| return info.getClassName(); | ||
| } | ||
| } | ||
| //TODO(zjffdu) It requires user can not create interpreter with name `conf`, | ||
| // conf is a reserved word of interpreter name | ||
| if (replName.equals("conf")) { | ||
| return ConfInterpreter.class.getName(); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
|
|
@@ -728,6 +736,29 @@ private ManagedInterpreterGroup createInterpreterGroup(String groupId) { | |
| return interpreterGroup; | ||
| } | ||
|
|
||
| /** | ||
| * Throw exception when interpreter process has already launched | ||
| * | ||
| * @param interpreterGroupId | ||
| * @param properties | ||
| * @throws IOException | ||
| */ | ||
| public void setInterpreterGroupProperties(String interpreterGroupId, Properties properties) | ||
| throws IOException { | ||
| ManagedInterpreterGroup interpreterGroup = this.interpreterGroups.get(interpreterGroupId); | ||
| for (List<Interpreter> session : interpreterGroup.sessions.values()) { | ||
| for (Interpreter intp : session) { | ||
| if (!intp.getProperties().equals(properties) && | ||
| interpreterGroup.getRemoteInterpreterProcess() != null && | ||
| interpreterGroup.getRemoteInterpreterProcess().isRunning()) { | ||
| throw new IOException("Can not change interpreter properties when interpreter process " + | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit worry about this? I think this is the way why
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, this is what I mention in the documentation. This generic |
||
| "has already been launched"); | ||
| } | ||
| intp.setProperties(properties); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private void loadInterpreterDependencies() { | ||
| setStatus(Status.DOWNLOADING_DEPENDENCIES); | ||
| setErrorReason(null); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we have to hardcode this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I make
confas a reserved interpreter name for this genericConfInterpreterso that all the interpreters can use it. e.g.%md.conf,%sh.conf,%spark.conf