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

Add js-plugins pre-built support #3056

Merged
merged 6 commits into from
Nov 3, 2022

Conversation

devinrsmith
Copy link
Member

Fast and simple js plugin support via the configuration property deephaven.jsPlugins.resourceBase - assumes the user has already built a js plugins compatible directory to the necessary specifications (the server transparently passes through the data with no knowledge of the structure).

Niceties can be added as documentation and additional follow-up. For example, the user can extend from the Deephaven official images and use the web-plugin-packager or similar construction in a multi-stage Dockerfile build to add the necessary structure to their own image.

@devinrsmith
Copy link
Member Author

Here was my local workflow to get up and running:

mkdir /tmp/js-plugins
cd /tmp/js-plugins
wget https://registry.npmjs.org/@deephaven/js-plugin-plotly/-/js-plugin-plotly-0.1.0.tgz
tar xzvf js-plugin-plotly-0.1.0.tgz
rm js-plugin-plotly-0.1.0.tgz
mkdir @deephaven
mv package @deephaven/js-plugin-plotly
echo '{"plugins":[{"name":"@deephaven/js-plugin-plotly","version":"0.1.0","main":"dist/index.js"}]}' > manifest.json
./gradlew py-server:assemble server-jetty-app:installDist
python3 -m venv /tmp/dh
source /tmp/dh/bin/activate
pip install py/server/build/wheel/deephaven_core-0.18.0-py3-none-any.whl
pip install deephaven-plugin-plotly
START_OPTS="-Ddeephaven.jsPlugins.resourceBase=/tmp/js-plugins" ./server/jetty-app/build/install/server-jetty/bin/start

(Executed in web ui)

import plotly.express as px
df = px.data.iris()
fig = px.scatter(df, x="sepal_width", y="sepal_length", color="species", size='petal_length', hover_data=['petal_width'])

@devinrsmith
Copy link
Member Author

I've published an example image (not sure how long it will be around), but from a docker users perspective, it may look like:

# syntax=docker/dockerfile:1.4

FROM ghcr.io/deephaven/web-plugin-packager:latest as build
RUN ./pack-plugins.sh @deephaven/js-plugin-plotly

FROM ghcr.io/deephaven/server:js-plugins-static
COPY --link --from=build js-plugins/ /js-plugins
RUN pip install deephaven-plugin-plotly
ENV START_OPTS="-Ddeephaven.jsPlugins.resourceBase=/js-plugins"
docker build -t my-deephaven-image .
docker run --rm -p 10000:10000 my-deephaven-image

@mofojed
Copy link
Member

mofojed commented Nov 2, 2022

Tested further by adding the matplotlib from a locally built js plugin as well. So:

pip install deephaven-plugin-matplotlib

Then cloned the deephaven-js-plugins repo: https://github.com/deephaven/deephaven-js-plugins
In that directory, ran:

npm install
npm run build

Add a link to the directory:

ln -s /home/bender/dev/deephaven/deephaven-js-plugins/matplotlib /tmp/js-plugins/@deephaven/js-plugin-matplotlib

Update the manifest:

echo '{"plugins":[{"name":"@deephaven/js-plugin-plotly","version":"0.1.0","main":"dist/index.js"},{"name":"@deephaven/js-plugin-matplotlib","version":"0.1.0","main":"dist/index.js"}]}' > manifest.json

Starting up the server as mentioned by Devin above.
Then running the code snippet:

import matplotlib.pyplot as plt
fig = plt.figure()
ax = fig.subplots()  # Create a figure containing a single axes.
ax.plot([1, 2, 3, 4], [4, 2, 6, 7])  # Plot some data on the axes.

However was getting a 404 when trying to load js-plugin matplotlib. Seems to work fine if I copy instead of making a symlink. Is there a reason why a symlink doesn't work here, guessing how Jetty loads resources? Would be handy for dev... though could have a build script to copy it as well.
image

Also - anyway to make it so manifest.json gets updated automatically?

@devinrsmith
Copy link
Member Author

I haven't looked into symlinks, may be related to jetty/jetty.project#8259? I can dig in...

@devinrsmith
Copy link
Member Author

As far as "manifest.json gets updated automatically" - with respect to this PR, no.

As part of the alternative PR (#2908), yes, that PR makes the server responsible for manifest.json.

@devinrsmith
Copy link
Member Author

I'm not able to get symlinks working either - the code suggests that it should work, but there is probably some missing context.

I've tried context.clearAliasChecks();

By default, the context is created with the {@link AllowedResourceAliasChecker} which is configured to allow symlinks. If this alias checker is not required, then {@link #clearAliasChecks()} or {@link #setAliasChecks(List)} should be called.

Maybe Colin knows?

@mofojed
Copy link
Member

mofojed commented Nov 3, 2022

I don't think the symlink thing should block this PR, since the workaround is just copy, or even the following:

  1. Clone the deephaven-js-plugins repo
  2. Add a manifest.json with the plugins you want to load, e.g. echo '{"plugins":[{"name":"matplotlib","version":"0.1.0","main":"dist/index.js"},{"name":"plotly","version":"0.1.0","main":"dist/index.js"}]}' > manifest.json
  3. Start up pointing to the deephaven-js-plugins directory as the js-plugins root, e.g. START_OPTS="-Ddeephaven.jsPlugins.resourceBase=/home/bender/dev/deephaven/deephaven-js-plugins" ./server/jetty-app/build/install/server-jetty/bin/start

That works well... however there still is a caching issue - it's just loading the js plugins from the disk cache. I think we need to add different caching headers there.

@devinrsmith
Copy link
Member Author

Ah yes, I think we should add the nocache filter for these paths - will do that.

@devinrsmith
Copy link
Member Author

Hmmm... I pushed a new commit, but it's not taking to this PR yet. Odd.

Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Err re-verting approval until PR updates wtih the latest but the code works

mofojed
mofojed previously approved these changes Nov 3, 2022
Copy link
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

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

Approved, review over slack

@devinrsmith devinrsmith merged commit 751a963 into deephaven:main Nov 3, 2022
@devinrsmith devinrsmith deleted the js-plugins-static branch November 3, 2022 21:38
@github-actions github-actions bot locked and limited conversation to collaborators Nov 3, 2022
@deephaven-internal
Copy link
Contributor

Labels indicate documentation is required. Issues for documentation have been opened:

How-to: https://github.com/deephaven/deephaven.io/issues/1859
Conceptual: https://github.com/deephaven/deephaven.io/issues/1857
Reference: https://github.com/deephaven/deephaven.io/issues/1856
Blog: https://github.com/deephaven/deephaven.io/issues/1858

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants