Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#22488 closed defect (fixed)

Make Three.js work offline in Jupyter notebooks

Reported by: paulmasson Owned by:
Priority: blocker Milestone: sage-7.6
Component: packages: standard Keywords:
Cc: novoselt, egourgoulhon, kcrisman, tmonteil Merged in:
Authors: Paul Masson Reviewers: Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 63f725d (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by paulmasson)

Both the Jupyter and legacy Sage notebooks display files from a local web server that does not have access by default to the file system.

This ticket adds a symbolic link to allow Jupyter notebooks access to the Three.js files and updates the links in the generated HTML according to the current Sage backend.

For Sage notebooks a pull request has been created to add the server link. In the meantime Sage notebooks default to using the CDN.

Change History (22)

comment:1 Changed 4 years ago by paulmasson

  • Branch set to u/paulmasson/22488

comment:2 Changed 4 years ago by paulmasson

  • Cc novoselt egourgoulhon kcrisman tmonteil added
  • Commit set to 7c0c9cf8e7ccb3d9b09020ddf11615fcfac61780
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

efbd877Add symlink to threejs
7c0c9cfAdd local Jupyter path

comment:3 follow-up: Changed 4 years ago by paulmasson

Since I'm unsure as to when a new version of the legacy notebook will be available, the ticket could be restricted to getting the viewer working in Jupyter notebooks.

comment:4 Changed 4 years ago by egourgoulhon

  • Priority changed from major to blocker

comment:5 in reply to: ↑ 3 Changed 4 years ago by egourgoulhon

  • Reviewers set to Eric Gourgoulhon
  • Status changed from needs_review to positive_review

Replying to paulmasson:

Since I'm unsure as to when a new version of the legacy notebook will be available, the ticket could be restricted to getting the viewer working in Jupyter notebooks.

I agree.

Thanks for this ticket; I've tested it in console mode and in Jupyter notebooks, both with and without internet connection. Everything seems OK.

comment:6 Changed 4 years ago by egourgoulhon

  • Status changed from positive_review to needs_info

I've just noticed some regression with respect to Sage 7.5.1: with the latter, the three.js output could be rendered by http://nbviewer.jupyter.org from a notebook publicly available (for instance stored on GitHub), which is pretty cool! Here is an example. This does no longer work for a notebook produced with the current branch: see the same sphere example here. Can this be corrected in the current ticket?

Last edited 4 years ago by egourgoulhon (previous) (diff)

comment:7 follow-up: Changed 4 years ago by paulmasson

The Three.js backend creates a standalone HTML file that requires <script> tags linking to the JavaScript files. The changes in #22389 default this to the local standard package, since that was requested by a couple reviewers. The current ticket changes the <script> tags to link to the local standard package via Jupyter's /nbextensions directory, which you can see by inspecting the generated output in your brower's console.

If the user knows ahead of time that the generated HTML will be viewed online, where this is no access to the local standard package, there is now a keyword option online=true that alters the <script> tags to use a publicly available content delivery network (CDN). If you know you will publish a notebook, you can add that keyword to a dictionary of options for all 3D plots.

If the notebook publishing site has an equivalent to /nbextensions, then you could alternately copy the two JavaScript files to a directory /threejs under it and things would work, but you probably aren't given access like that to their server.

A third option (but a bit messier) is to add two sets of <script> tags to the generated HTML. Then the page would fall back on the CDN when the local standard package is unavailable. That would make your notebook publishing process work transparently, but would quietly add a possible "leak" to the Internet that another reviewer wanted me to avoid. Is it worth doing this rather than use the online=true keyword?

I can go either way on this, and in fact I really prefer just using the CDN all the time. Input from others would be appreciated.

comment:8 Changed 4 years ago by paulmasson

  • Summary changed from Make Three.js work offline in notebooks to Make Three.js work offline in Jupyter notebooks

comment:9 in reply to: ↑ 7 ; follow-up: Changed 4 years ago by egourgoulhon

Replying to paulmasson:

If the user knows ahead of time that the generated HTML will be viewed online, where this is no access to the local standard package, there is now a keyword option online=true that alters the <script> tags to use a publicly available content delivery network (CDN). If you know you will publish a notebook, you can add that keyword to a dictionary of options for all 3D plots.

I did not realize this, thanks! It works!

If the notebook publishing site has an equivalent to /nbextensions, then you could alternately copy the two JavaScript files to a directory /threejs under it and things would work, but you probably aren't given access like that to their server.

Indeed...

A third option (but a bit messier) is to add two sets of <script> tags to the generated HTML. Then the page would fall back on the CDN when the local standard package is unavailable. That would make your notebook publishing process work transparently, but would quietly add a possible "leak" to the Internet that another reviewer wanted me to avoid. Is it worth doing this rather than use the online=true keyword?

I would vote for this one, since it sounds awkward to create multiple versions of a notebook or to have to know in advance that you will publish it at some point... Even if you know that and have added online=True everywhere, this becomes very annoying if you work on the notebook and for some reason you don't have any internet connection.

comment:10 in reply to: ↑ 9 ; follow-up: Changed 4 years ago by paulmasson

Replying to egourgoulhon:

Replying to paulmasson:

A third option (but a bit messier) is to add two sets of <script> tags to the generated HTML. Then the page would fall back on the CDN when the local standard package is unavailable. That would make your notebook publishing process work transparently, but would quietly add a possible "leak" to the Internet that another reviewer wanted me to avoid. Is it worth doing this rather than use the online=true keyword?

I would vote for this one, since it sounds awkward to create multiple versions of a notebook or to have to know in advance that you will publish it at some point... Even if you know that and have added online=True everywhere, this becomes very annoying if you work on the notebook and for some reason you don't have any internet connection.

Then the question arises as to whether to do this in all cases or just for the Jupyter notebooks. That is:

1) Remove the online keyword and always provide two sets of tags in all generated content. For command-line users this would potentially expose some local file system information without their knowledge, but would make the HTML files immediately portable.

2) Keep the online keyword to allow command-line users to explicitly generate public online files without any local computer information embedded, but have the two-set fallback for Jupyter notebook users. The /nbextensions link is generic and reveals nothing about the local computer.

I favor the latter. And for future reference, can Sage legacy notebooks be published in the same way or do they always stay on the local server?

comment:11 in reply to: ↑ 10 Changed 4 years ago by egourgoulhon

Replying to paulmasson:

Then the question arises as to whether to do this in all cases or just for the Jupyter notebooks. That is:

1) Remove the online keyword and always provide two sets of tags in all generated content. For command-line users this would potentially expose some local file system information without their knowledge, but would make the HTML files immediately portable.

2) Keep the online keyword to allow command-line users to explicitly generate public online files without any local computer information embedded, but have the two-set fallback for Jupyter notebook users. The /nbextensions link is generic and reveals nothing about the local computer.

I favor the latter.

Yes, this seems a good idea.

And for future reference, can Sage legacy notebooks be published in the same way or do they always stay on the local server?

Since Jupyter will be the default notebook soon (Sage 8.0, immediately after Sage 7.6), with a converter from legacy notebooks, I don't see the point in publishing legacy notebooks. But probably, this should be discussed in another ticket.

comment:12 Changed 4 years ago by git

  • Commit changed from 7c0c9cf8e7ccb3d9b09020ddf11615fcfac61780 to 63f725d65fcd99c3a8669534efdd3114e31bf1ff

Branch pushed to git repo; I updated commit sha1. New commits:

63f725dFallback to CDN when online

comment:13 follow-up: Changed 4 years ago by paulmasson

  • Status changed from needs_info to needs_review

This should take care of the outstanding issue. Please try publishing a notebook without the online keyword.

I've also added a few double quotes, to prevent a possible future error and to make things read consistently.

Last edited 4 years ago by paulmasson (previous) (diff)

comment:14 in reply to: ↑ 13 Changed 4 years ago by egourgoulhon

  • Status changed from needs_review to positive_review

Replying to paulmasson:

This should take care of the outstanding issue.

Thanks!

Please try publishing a notebook without the online keyword.

It works!

I've also added a few double quotes, to prevent a possible future error and to make things read consistently.

Very good.

I've performed a few other checks (online/offline in console/Jupyter notebook); everything seems OK. Thank you!

comment:15 Changed 4 years ago by vbraun

  • Branch changed from u/paulmasson/22488 to 63f725d65fcd99c3a8669534efdd3114e31bf1ff
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:16 follow-up: Changed 4 years ago by novoselt

  • Commit 63f725d65fcd99c3a8669534efdd3114e31bf1ff deleted

Sorry for not looking at this closer when it was still for review.

I very much dislike the logic for generating different files relying on instance check of the backend. These backend-specific things should be written in the backend files and pulled via the same methods here. So I would very much like to get rid of this line

backend = get_display_manager()._backend

together with dependent ones and instead have something like

scripts = get_display_manager().threejs_scripts(options['online'])

with default behaviour of serving CDN links for True and failing for False but letting individual backends override False however they wish. In particular it will allow me to has SageMathCell backend do what is needed without making tweaks to Sage itself. Thoughts?

comment:17 follow-up: Changed 4 years ago by novoselt

No clue why commit got deleted...

comment:18 in reply to: ↑ 17 Changed 4 years ago by egourgoulhon

Replying to novoselt:

No clue why commit got deleted...

I think this is harmless; it seems to occur anytime a comment is added to a closed ticket.

comment:19 in reply to: ↑ 16 Changed 4 years ago by paulmasson

Replying to novoselt:

Sorry for not looking at this closer when it was still for review.

I very much dislike the logic for generating different files relying on instance check of the backend. These backend-specific things should be written in the backend files and pulled via the same methods here. So I would very much like to get rid of this line

backend = get_display_manager()._backend

together with dependent ones and instead have something like

scripts = get_display_manager().threejs_scripts(options['online'])

with default behaviour of serving CDN links for True and failing for False but letting individual backends override False however they wish. In particular it will allow me to has SageMathCell backend do what is needed without making tweaks to Sage itself. Thoughts?

Point well taken. Discussion moved to #22670.

comment:20 Changed 3 years ago by enriqueartal

After these changes threejs does not work any more with jupyterhub. I tried some workarounds whitout success. It is probably difficult to have a solution for both offline jupyter notebook and jupyterhub, but a hint would help. Thanks.

comment:21 follow-up: Changed 3 years ago by paulmasson

The behavior for Jupyter notebooks includes a fallback to an online CDN when the offline JavaScript files are unavailable. This was tested in Comment 14 above.

Does Jupyterhub block access to the Internet? Are you deliberately blocking Internet access?

comment:22 in reply to: ↑ 21 Changed 3 years ago by enriqueartal

Replying to paulmasson:

The behavior for Jupyter notebooks includes a fallback to an online CDN when the offline JavaScript files are unavailable. This was tested in Comment 14 above.

Does Jupyterhub block access to the Internet? Are you deliberately blocking Internet access?

No, Jupyterhub is not blocking internet access (it is done in order to offer internet access to jupyter notebook); the problem is that it looks for the scripts in "https://ip_address:port/nbextensions/threejs/three.min.js" and it fails to find them (no problem if jupyter is launched locally). I found a workaround redefining the function threejs_offline_scripts (which appears twice in backend_ipython.py) asking jupyter to look for the scripts in rawgit. Maybe a global solution should not be done here but in jupyterhub.

Note: See TracTickets for help on using tickets.