You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2023/11/13 07:12:14 UTC
[PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
pan3793 opened a new pull request, #4692:
URL: https://github.com/apache/zeppelin/pull/4692
### What is this PR for?
This PR aims to merge `zeppelin-display` into `spark-interperter`, as suggested in https://github.com/apache/zeppelin/pull/4652#issuecomment-1739360154
### What type of PR is it?
Improvement
### Todos
* [ ] - Task
### What is the Jira issue?
ZEPPELIN-5980
### How should this be tested?
Pass GA.
### Screenshots (if appropriate)
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? Yes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692#issuecomment-1818462184
> > BTW, with the same reason, we can merge zeppelin-zengine to zeppelin-server. :-)
>
> Thanks for the information, I have not read through those codes, will take a look after the cleanup work of the interpreters is done.
I think it's actually pretty good that we have the two modules (zeppelin-zengine and zeppelin-server). Theoretically, it is possible to replace the application server (currently Jetty). But I don't know anyone who really does that. Nevertheless, it is important to me that we only use standardized configurations and do not rely on deep Jetty functions.
But I've already learned about some of the disadvantages. Maybe it makes sense to merge the two modules here. Even if this will certainly not be easy.
In principle, I am also thinking of a zeppelin-api module, where there are only interfaces so that you cannot access the class directly in "plugins".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692#issuecomment-1818444286
> BTW, with the same reason, we can merge zeppelin-zengine to zeppelin-server. :-)
Thanks for the information, I have not read through those codes, will take a look after the cleanup work of the interpreters is done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692#issuecomment-1818437511
@jongyoul thanks for the additional information.
One of the motivations for merging this module is to reduce the dependence of Scala in Zeppelin core modules (I mean modules other than interpreter and plugins), after this patch,
the global `scala-2.11` and `scala-2.12` does not take effect and we can remove it. Now,
- Cassandra interpreter is hardcoded with scala 2.12
- Spark interpreter is controlled by `spark-scala-2.12` and `spark-scala-2.13`
- Flink interpreter is going to be hardcoded with scala 2.12
- Livy interpreter itself does not depend on Scala, but the test deps depends on Scala 2.11(because Livy only published the Scala 2.11 jars to Maven Central)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692#issuecomment-1818426906
BTW, with the same reason, we can merge zeppelin-zengine to zeppelin-server. :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692#issuecomment-1818420839
> I don't like the mixing of changes in Spark and the Cassandra interpreter. Do you see a way to split this up?
Let me send a PR to upgrade the Cassandra part first.
> What IDE do you use where there is such good Scala and Java support?
I have used JetBrains IntelliJ IDEA Community Edition for a long time, I don't have much experience with other IDEs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692#issuecomment-1818474612
actually, I think `zeppelin-interpreter` should be kind of a lightweight API module, but I found packaging `zeppelin-interpreter-shaded` is extremely slow, and the shading process includes bunches of third-party jars, looks unreasonable ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692#issuecomment-1818502773
zeppelin-interpreter contains, among other things, the code to execute a Zeppelin interpreter remotely.
The configuration of Zeppelin is also included here. In my opinion, it belongs in the zeppelin-zengine module. The interpreter should only have a simple hashmap, which is transferred when the remote interpreter is started.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692#issuecomment-1817580578
cc @Reamer, this is ready for review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-5980] Merge zeppelin-display into spark-interperter [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul merged PR #4692:
URL: https://github.com/apache/zeppelin/pull/4692
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org