Skip to content
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-1395] Local or Remote Interpreter by Configuration #1385

Closed

Conversation

echarles
Copy link
Member

What is this PR for?

The current remote interpreter (launched via shell) is scalable and open but suffers from 2 limitations:

  • It does not allow to easily debug in the IDE.
  • It does not allow to embedd Zeppelin (for small deployements) in a single jar.

A configuration per interpreter "Remote Interpreter" (true/false, true by default) is proposed.
An additional property zeppeline.interpreter.remote' in zeppelin-site.xml is also proposed (a new interpreter will be created with that value for itoption.remote` attribute).

What type of PR is it?

[Improvement]

Todos

  • - Value defined in zeppelin-site.xml is not honored when creating a new interepreter
  • - Unit Test
  • - Documentatoin

What is the Jira issue?

How should this be tested?

Load Zeppelin source in your IDE and start ZeppelinServer in debug mode
Put breakpoints in the interpreter sources, Set / Uset the "Remote Interpeter (see screeenshot).

Screenshots (if appropriate)

animated-gif

Questions:

  • Does the licenses files need update? N
  • Is there breaking changes for older versions? N
  • Does this needs documentation? Y

@jongyoul
Copy link
Member

ZeppelinConfiguration has a factory method, thus you don't have to pass it by parameter and you can use ZeppelinConfiguration.create()

…changes to InterpeterSetting method signatures
@astroshim
Copy link
Contributor

Please fix the CI-fails.

[INFO] There are 5 checkstyle errors.
[ERROR] InterpreterFactory.java[302] (sizes) LineLength: Line is longer than 100 characters (found 106).
[ERROR] InterpreterFactory.java[327] (sizes) LineLength: Line is longer than 100 characters (found 106).
[ERROR] InterpreterFactory.java[330] (indentation) Indentation: method def child at indentation level 6 not at correct indentation, 4
[ERROR] InterpreterFactory.java[332] (indentation) Indentation: for at indentation level 6 not at correct indentation, 4
[ERROR] InterpreterFactory.java[345] (sizes) LineLength: Line is longer than 100 characters (found 116).

@echarles
Copy link
Member Author

echarles commented Sep 2, 2016

@astroshim Thx for the review. I have pushed a fix.

@echarles
Copy link
Member Author

echarles commented Sep 2, 2016

@jongyoul ZeppelinConfiguration.create() is now used and method signature are now like before.

@@ -79,7 +81,7 @@ public InterpreterSetting(String name, String group, List<InterpreterInfo> inter
*
* @param o interpreterSetting from interpreterSettingRef
*/
public InterpreterSetting(InterpreterSetting o) {
public InterpreterSetting(InterpreterSetting o, ZeppelinConfiguration conf) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It remains ZeppelinConfiguration. Could you check it again?

@echarles
Copy link
Member Author

echarles commented Sep 10, 2016

@jongyoul ZeppelinConfiguration is now also removed on that method.

@asfgit asfgit closed this in c38a0a0 May 9, 2018
asfgit pushed a commit that referenced this pull request May 9, 2018
close #83
close #86
close #125
close #133
close #139
close #146
close #193
close #203
close #246
close #262
close #264
close #273
close #291
close #299
close #320
close #347
close #389
close #413
close #423
close #543
close #560
close #658
close #670
close #728
close #765
close #777
close #782
close #783
close #812
close #822
close #841
close #843
close #878
close #884
close #918
close #989
close #1076
close #1135
close #1187
close #1231
close #1304
close #1316
close #1361
close #1385
close #1390
close #1414
close #1422
close #1425
close #1447
close #1458
close #1466
close #1485
close #1492
close #1495
close #1497
close #1536
close #1545
close #1561
close #1577
close #1600
close #1603
close #1678
close #1695
close #1739
close #1748
close #1765
close #1767
close #1776
close #1783
close #1799
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants