search

dask / dask-labextension

JupyterLab extension for Dask
BSD 3-Clause "New" or "Revised" License
311 stars 62 forks source link

Check dashboard server side #159

Closed ian-r-rose closed 3 years ago

ian-r-rose commented 3 years ago

This should resolve a number of long-running issues around user-provided dashboard URLs (that is to say, ones that are entered into the URL box, not those managed by the cluster manager). Previously we did a couple of things client-side:

  1. Check to see if the url is valid
  2. Load the list of individual plots from the bokeh server

This is for primarily historical reasons, as the server-side component of this extension did not always exist. It wound up causing a number of issues around CORS and mixed content. This moves the above checks to a new tornado handler on the server side, which have fewer such restrictions. It also allows us to more easily follow redirects.

Note: this still embeds the iframes directly from the bokeh server. We may want to also proxy those under the notebook server (as is done with the clusters launched by the labextension itself), but that can be in a follow-up, I think.

Fixes #158, fixes #137, probably fixes #32

mrocklin commented 3 years ago

Woo!

On Wed, Dec 9, 2020, 2:21 PM Ian Rose notifications@github.com wrote:

This should resolve a number of long-running issues around user-provided dashboard URLs (that is to say, ones that are entered into the URL box, not those managed by the cluster manager). Previously we did a couple of things client-side:

  1. Check to see if the url is valid
  2. Load the list of individual plots from the bokeh server

This is for primarily historical reasons, as the server-side component of this extension did not always exist. It wound up causing a number of issues around CORS and mixed content. This moves the above checks to a new tornado handler on the server side, which have fewer such restrictions. It also allows us to more easily follow redirects.

Note: this still embeds the iframes directly from the bokeh server. We may want to also proxy those under the notebook server (as is done with the clusters launched by the labextension itself), but that can be in a follow-up, I think.

Fixes #158 https://github.com/dask/dask-labextension/issues/158, fixes

137 https://github.com/dask/dask-labextension/issues/137, probably

fixes #32 https://github.com/dask/dask-labextension/issues/32

You can view, comment on, or merge this pull request online at:

https://github.com/dask/dask-labextension/pull/159 Commit Summary

  • Add tornado handler for checking the validity of a dashboard URL server
  • Check for dashboard validity through the server. If there are redirects,
  • Always get individual-plots from the bokeh server.
  • Clean up control flow a little bit when getting dashboard listing from
  • Handle dynamic changes to the listing of dashboard charts a bit better.

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dask/dask-labextension/pull/159, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKZTBUHZ2Y2TBPP3LKNI3ST72ADANCNFSM4UUENWAA .

ian-r-rose commented 3 years ago

Unless there are objections, I'll merge this today

jrbourbeau commented 3 years ago

FWIW I just tested out the changes here using a Coiled cluster and everything worked as expected (i.e. we followed the redirect mentioned in https://github.com/dask/dask-labextension/issues/158)