Skip to content

[ZEPPELIN-5019] Enable configurable HTML addons#3892

Closed
weand wants to merge 1 commit intoapache:masterfrom
weand:ZEPPELIN-5019
Closed

[ZEPPELIN-5019] Enable configurable HTML addons#3892
weand wants to merge 1 commit intoapache:masterfrom
weand:ZEPPELIN-5019

Conversation

@weand
Copy link
Contributor

@weand weand commented Aug 27, 2020

What is this PR for?

Currently its hard to integrate certain plotting libraries in one single zeppelin notebook efficiently. efficiently here means in the sense of how often library code needs to be embedded into a single notebook. while few plotting libraries provide APIs to properly enable lazy loading of their JS library (e.g. bokeh), others do not (e.g. plotly), which results in large overhead in notebooks with couple of plots, where each plot embeds the library code again and again. attempts to only embed library code in the first paragraph is not feasible, as its not guaranteed, that this paragraph will be loaded at first by the browser. Any attempt to load libraries via paragraph code will end up in such synchronization issues when the library does not support synchronization on its own.

A general solution for that problem is to enable the loading of 3rd party JS libraries within the root HTML.

Hence this issue will introduce a new optional zeppelin configuration that allows administrators to add global addons to the zeppelin root HTML, so analysts can rely on that the JS library is already loaded for every paragraph.

As there exist two places in HTML where initial loading may be placed, the issue will introduce the following two configuration properties:

  • zeppelin.server.html.head.addon > addon html code to be placed at the end of the html->head section 
  • zeppelin.server.html.body.addon > addon html code to be placed at the end of the html->body section

Each property may contain any valid HTML code, e.g.

<script defer src="https://url/to/my/lib.min.js" /><script defer src="https://url/to/other/lib.min.js" />

What type of PR is it?

Improvement

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-5019

How should this be tested?

  • Unit Test added for index.html of zeppelin-web
  • can also be manually tested by adding config properties mentioned above. Then check the resulting source of the zeppelin web application (old and new ui)

Screenshots (if appropriate)

Questions:

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

@weand
Copy link
Contributor Author

weand commented Aug 27, 2020

@zjffdu, @Leemoonsoo can you have a look here, and review.

I've tested it manually in development and normal (war) mode, and with old and new zeppelin UI successfully

thanks in advance.

@Leemoonsoo
Copy link
Member

Thank @weand for the contribution.
It looks good to me!

Just a side question, although spell is not designed for this, it basically installs & loads javascript packages in front-end. Do you think it makes sense to leverage this kind of plugin mechanism to install javascript in the future, or it is overkill?

@weand
Copy link
Contributor Author

weand commented Aug 28, 2020

Just a side question, although spell is not designed for this, it basically installs & loads javascript packages in front-end. Do you think it makes sense to leverage this kind of plugin mechanism to install javascript in the future, or it is overkill?

I'm not sure about existing use cases, but for me a helium spell plugin seems to be very limited in general, because they can't be integrated with Server Side Interpreters (e.g. %pyspark). Just for installing additional javascript code / libraries they are really overkill. Especially because of additional maintenance / versioning effort when just delivering libraries.

If there are no other comments within next few days, could you then please merge.

@weand
Copy link
Contributor Author

weand commented Aug 28, 2020

@zjffdu @Leemoonsoo I also thought about adding unit test, which would rely on existence of zeppelin-web/dist/index.html and/or zeppelin-web-angular/dist/index.html. Seems for test of core-modules travis only builds zeppelin-web and not zeppelin-web-angular yet. could that module be activated for core tests as well, or is that no good idea?

@weand
Copy link
Contributor Author

weand commented Aug 31, 2020

Tests added.

Added @Ignore to Test for zeppelin-web-angular, as its not build yet for core tests)

Tests showed, that my manual tests with zeppelin-web were not that good. Additional logic was required for the case, when closing html tags do not exist (which is the case for build results of zeppelin-web).

@weand
Copy link
Contributor Author

weand commented Sep 1, 2020

@Leemoonsoo

squashed and rebased the changes. could you have a quick look again. and merge if there are no more comments.

@Leemoonsoo
Copy link
Member

Thank @weand! looks good to me. Merging this to master and branch-0.9

@asfgit asfgit closed this in cb06507 Sep 4, 2020
asfgit pushed a commit that referenced this pull request Sep 4, 2020
### What is this PR for?
Currently its hard to integrate certain plotting libraries in one single zeppelin notebook efficiently. efficiently here means in the sense of how often library code needs to be embedded into a single notebook. while few plotting libraries provide APIs to properly enable lazy loading of their JS library (e.g. bokeh), others do not (e.g. plotly), which results in large overhead in notebooks with couple of plots, where each plot embeds the library code again and again. attempts to only embed library code in the first paragraph is not feasible, as its not guaranteed, that this paragraph will be loaded at first by the browser. Any attempt to load libraries via paragraph code will end up in such synchronization issues when the library does not support synchronization on its own.

A general solution for that problem is to enable the loading of 3rd party JS libraries within the root HTML.

Hence this issue will introduce a new optional zeppelin configuration that allows administrators to add global addons to the zeppelin root HTML, so analysts can rely on that the JS library is already loaded for every paragraph.

As there exist two places in HTML where initial loading may be placed, the issue will introduce the following two configuration properties:

- `zeppelin.server.html.head.addon` > addon html code to be placed at the end of the html->head section 
- `zeppelin.server.html.body.addon` > addon html code to be placed at the end of the html->body section

Each property may contain any valid HTML code, e.g.

```
<script defer src="https://url/to/my/lib.min.js" /><script defer src="https://url/to/other/lib.min.js" />
```

### What type of PR is it?
Improvement

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-5019

### How should this be tested?
* Unit Test added for index.html of `zeppelin-web`
* can also be manually tested by adding config properties mentioned above. Then check the resulting source of the zeppelin web application (old and new ui)

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? yes

Author: Andreas Weise <a.weise@avm.de>

Closes #3892 from weand/ZEPPELIN-5019 and squashes the following commits:

29e7a02 [Andreas Weise] ZEPPELIN-5019 Enable configurable HTML addons

(cherry picked from commit cb06507)
Signed-off-by: Lee moon soo <moon@apache.org>
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.

2 participants