ZEPPELIN-546 Enables interpreter library loading from maven repository by minahlee · Pull Request #590 · apache/zeppelin

minahlee · 2016-01-02T07:28:37Z

What is this PR for?

This PR enables library loading from maven repository to load interpreter binaries.
To leverage current spark interpreter's library loading, moved org.apache.zeppelin.spark.dep(under spark) to org.apache.zeppelin.dep(under zeppelin-interpreter).
Making REST API and loading interpreter property on runtime will be the next step to complete ZEPPELIN-546 and will be handled in different PR.

What type of PR is it?

Feature

Is there a relevant Jira issue?

ZEPPELIN-546

Questions:

Does the licenses files need update? No (dependencies added to zeppelin-interpreter/pom.xml is already used in other module)
Is there breaking changes for older versions? No
Does this needs documentation? Yes (will be addressed in another PR)

bzz · 2016-01-05T08:51:12Z

Looks great! A quick question if it's ready to merge: zeppelin-interpreter/pom.xml was changed with dependencies added but you think there is no need to mention any of those in licence files?

Also, is there a reason not to handle documentation update in the same PR as feature and it's tests?

minahlee · 2016-01-05T18:20:18Z

@bzz Thanks for review. Dependencies that added to zeppelin-interpreter/pom.xml is already used in spark/pom.xml, so I answered 'no' for license update question. I update PR description for the ones who might be confused like you. Basic test is already included in this PR. About the docs @AhyoungRyu will contribute it but if she commits on this PR, her contribution won't be counted. That's the only reason we separate the PR for docs.

AhyoungRyu · 2016-01-05T19:59:24Z

@bzz Yes, @minahlee is right. I'm working on it : )
I'll commit PR for docs soon, after this PR merged.

corneadoug · 2016-01-05T21:50:28Z

@minahlee still doesn't make much sense to separate the docs into a different PR.
At least create both PR so that we can merge both at the same time.

Leemoonsoo · 2016-01-05T22:25:54Z

I think how to split large work into small peices of PRs is up to contributors. (although smaller PR helps review process)
If necessary documentation for this PR comes with different PR, we can wait for that and merge both PR together.

corneadoug · 2016-01-06T01:32:28Z

@Leemoonsoo Yes, it will be better to have both at the same time to merge them together.
Its just to keep features documented as we include them in the code, that way people know about it and also know how to use it. It can also help a lot for the reviewer to test the feature. (Its not really explained right now)

bzz · 2016-01-06T01:52:02Z

@minahlee @AhyoungRyu - thanks for explanations, makes perfect sense to have 2 PRs for 2 ppl!

Looks good to me.

jongyoul · 2016-01-10T05:48:42Z

LGTM!! It also makes any kind of jars loaded easily. I could load JDBC libraries using this feature.

- Move org.apache.zeppelin.spark.dep package from zeppelin-spark to zeppelin-interpreter - Rename DependencyResolver/DependencyContext to SparkDependencyResolver/SparkDependencyContext - Add general DependencyResolver

minahlee · 2016-01-13T00:37:52Z

Rebased master branch and CI build passed

Leemoonsoo · 2016-01-13T01:55:23Z

Great. LGTM

…m maven repository ### What is this PR for? This PR is related to #590. This documentation may explain the overall process of **Dynamic Interpreter Loading** in [Helium Proposal](https://issues.apache.org/jira/browse/ZEPPELIN-533). Moreover, at the last this documentation, Zeppelin users can get the basic information about step by step of interpreter setting & configuring & binding. Since I assumed that they are novice at Zeppelin. ### What type of PR is it? Documentation ### Todos * [x] - Add docs image under `/docs/assets/themes/zeppelin/img/docs-img/` * [x] - Add dynamicinterpreter.md * [x] - Add dynamicinterpreter.html location to `docs/_includes/themes/zeppelin/_navigation.html` ### Is there a relevant Jira issue? 1. [ZEPPELIN-533 Helium](https://issues.apache.org/jira/browse/ZEPPELIN-533) 2. [ZEPPELIN-546](https://issues.apache.org/jira/browse/ZEPPELIN-546) ### How should this be tested? I add a link of this documentation to `interpreter tab` in Zeppelin web page. ![screen shot 2016-01-07 at 12 39 54 pm](https://cloud.githubusercontent.com/assets/10060731/12182295/20711f30-b53c-11e5-8369-2ec42c99f4e2.png) ### Screenshots (if appropriate) Hopefully, below image helps you to understand this process : ) ![zeppelin_user](https://cloud.githubusercontent.com/assets/10060731/12180839/b0240d30-b533-11e5-97e1-87c7833ee47f.png) ### Questions: * Does the licenses files need update? No. * Is there breaking changes for older versions? No. * Does this needs documentation? No. Author: Ryu Ah young <[email protected]> Closes #609 from AhyoungRyu/ZEPPELIN-546-docs and squashes the following commits: d0675e0 [Ryu Ah young] ZEPPELIN-546-docs: Fix some sentences 266dac0 [Ryu Ah young] ZEPPELIN-546-docs: Add content for licence e180a8e [Ryu Ah young] ZEPPELIN-546-docs: Fix typo again d3cc50f [Ryu Ah young] ZEPPELIN-546-docs: Fix typo a9ec7d7 [Ryu Ah young] ZEPPELIN-546-docs: Add dynamicinterpreterload.html location to _navigation.html 16b46f4 [Ryu Ah young] ZEPPELIN-546-docs: Add dynamicinterpreterload.md 3067025 [Ryu Ah young] ZEPPELIN-546-docs: Add docs image

pooja155 · 2018-08-28T08:30:54Z

zeppelin-interpreter/src/main/java/org/apache/zeppelin/dep/DependencyResolver.java

+  Logger logger = LoggerFactory.getLogger(DependencyResolver.class);
+
+  private final String[] exclusions = new String[] {"org.apache.zeppelin:zeppelin-zengine",
+                                                    "org.apache.zeppelin:zeppelin-interpreter",


hi, was there any reason to not include all the exclusions mentioned in SparkDependencyResolver.java here?
"org.scala-lang:scala-library","org.scala-lang:scala-compiler","org.scala-lang:scala-reflect","org.scala-lang:scalap" - these are present in the exclusions in SparkDependencyResolver.java but not here.

AFAIK, spark already has scala related packages but this doesn't have them.

minahlee changed the title ~~ZEPPELIN-546 Enables interpreter library loading from maven repository~~ [WIP] ZEPPELIN-546 Enables interpreter library loading from maven repository Jan 5, 2016

minahlee changed the title ~~[WIP] ZEPPELIN-546 Enables interpreter library loading from maven repository~~ ZEPPELIN-546 Enables interpreter library loading from maven repository Jan 5, 2016

minahlee force-pushed the ZEPPELIN-546 branch 2 times, most recently from 8b1f240 to 2af3bea Compare January 5, 2016 22:07

AhyoungRyu mentioned this pull request Jan 7, 2016

[Zeppelin 546](documentation) Enables interpreter library loading from maven repository #609

Closed

3 tasks

minahlee added 4 commits January 11, 2016 14:08

[ZEPPELIN-546] Load interpreter from maven repository

aab2d04

- Move org.apache.zeppelin.spark.dep package from zeppelin-spark to zeppelin-interpreter - Rename DependencyResolver/DependencyContext to SparkDependencyResolver/SparkDependencyContext - Add general DependencyResolver

[ZEPPELIN-546] Pass DependencyResolver to InterpreterFactory

4baf271

[ZEPPELIN-546] Fix tests

039f5fa

[ZEPPELIN-546] Add test

72ad12c

minahlee force-pushed the ZEPPELIN-546 branch 4 times, most recently from 39ecf9d to 72ad12c Compare January 13, 2016 00:11

[ZEPPELIN-546] Refactoring

cd190d4

asfgit closed this in bc71551 Jan 13, 2016

ramaswamydevarajan mentioned this pull request Jan 13, 2016

ZEPPELIN-546 Enables interpreter library loading from maven repository ramaswamydevarajan/incubator-zeppelin#3

Merged

minahlee deleted the ZEPPELIN-546 branch August 8, 2016 02:14

pooja155 reviewed Aug 28, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZEPPELIN-546 Enables interpreter library loading from maven repository#590

ZEPPELIN-546 Enables interpreter library loading from maven repository#590
minahlee wants to merge 5 commits intoapache:masterfrom
minahlee:ZEPPELIN-546

minahlee commented Jan 2, 2016

Uh oh!

bzz commented Jan 5, 2016

Uh oh!

minahlee commented Jan 5, 2016

Uh oh!

AhyoungRyu commented Jan 5, 2016

Uh oh!

corneadoug commented Jan 5, 2016

Uh oh!

Leemoonsoo commented Jan 5, 2016

Uh oh!

corneadoug commented Jan 6, 2016

Uh oh!

bzz commented Jan 6, 2016

Uh oh!

jongyoul commented Jan 10, 2016

Uh oh!

minahlee commented Jan 13, 2016

Uh oh!

Leemoonsoo commented Jan 13, 2016

Uh oh!

pooja155 Aug 28, 2018

Uh oh!

jongyoul Aug 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

minahlee commented Jan 2, 2016

What is this PR for?

What type of PR is it?

Is there a relevant Jira issue?

Questions:

Uh oh!

bzz commented Jan 5, 2016

Uh oh!

minahlee commented Jan 5, 2016

Uh oh!

AhyoungRyu commented Jan 5, 2016

Uh oh!

corneadoug commented Jan 5, 2016

Uh oh!

Leemoonsoo commented Jan 5, 2016

Uh oh!

corneadoug commented Jan 6, 2016

Uh oh!

bzz commented Jan 6, 2016

Uh oh!

jongyoul commented Jan 10, 2016

Uh oh!

minahlee commented Jan 13, 2016

Uh oh!

Leemoonsoo commented Jan 13, 2016

Uh oh!

pooja155 Aug 28, 2018

Choose a reason for hiding this comment

Uh oh!

jongyoul Aug 28, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants