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
[Draft] [ZEPPELIN-5703] Support to run another notebook in notebook inline #4339
base: master
Are you sure you want to change the base?
Conversation
16861c8
to
d0f8ea9
Compare
@@ -975,6 +975,9 @@ public void restart(String settingId, String user, String noteId) throws Interpr | |||
|
|||
// restart in note page | |||
public void restart(String settingId, ExecutionContext executionContext) throws InterpreterException { | |||
if (settingId == null || settingId.startsWith("__internal__")) { |
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.
How about adding comment to describe __internal__
?
if (settingId == null || settingId.startsWith("__internal__")) { | |
// __internal__ is set by RunNotebookInterpreter | |
if (settingId == null || settingId.startsWith("__internal__")) { |
import org.apache.zeppelin.interpreter.InterpreterSetting; | ||
import org.apache.zeppelin.interpreter.InterpreterSettingManager; | ||
import org.apache.zeppelin.interpreter.ManagedInterpreterGroup; | ||
import org.apache.zeppelin.interpreter.*; |
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'm not sure about the current style rule but might be replaced by IDE automatically?
} | ||
|
||
@VisibleForTesting | ||
public static Map<String, String> parseParams(String text) throws InvalidParameterFormatException { |
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.
Just curious. Why don't we use regex to handle it? It also handle some exceptional cases but I wonder if we need to handle that case or not.
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.
LGTM except for some trivial comments.
@zjffdu PR itself looks good to me but I have a question about this idea itself. We had a plan to provide job manager which handle DAG for paragraphs but it's not proceeded well. You PR looks a different way to handle a similar request. Then, can we remote job manager on Zeppelin? It shows some status but it's not that helpful even the quest is too complicated. WDYT? |
838fc18
to
c992c60
Compare
d6a424f
to
bbbfcb4
Compare
What is this PR for?
A few sentences describing the overall goals of the pull request's commits.
First time? Check out the contributing guide - https://zeppelin.apache.org/contribution/contributions.html
What type of PR is it?
[Bug Fix | Improvement | Feature | Documentation | Hot Fix | Refactoring]
Todos
What is the Jira issue?
How should this be tested?
Screenshots (if appropriate)
Questions: