-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Check dashboard server side #159
Merged
ian-r-rose
merged 6 commits into
dask:master
from
ian-r-rose:check-dashboard-server-side
Dec 15, 2020
Merged
Check dashboard server side #159
ian-r-rose
merged 6 commits into
dask:master
from
ian-r-rose:check-dashboard-server-side
Dec 15, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
side. This allows us to avoid CORS or mixed content errors when determining whether a user-provided dashboard is valid.
that is captured in `effectiveUrl`, which is used to construct dashboard chart urls.
Close nonexistent panes, rename ones that have a different label.
Woo!
…On Wed, Dec 9, 2020, 2:21 PM Ian Rose ***@***.***> 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 <#158>, fixes
#137 <#137>, probably
fixes #32 <#32>
------------------------------
You can view, comment on, or merge this pull request online at:
#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
- *M* dask_labextension/__init__.py
<https://github.com/dask/dask-labextension/pull/159/files#diff-8ad0c90bbd53b41ae1b5bba85518dea7d9a597c4af2e44c5c36b293fc8c500bb>
(6)
- *M* dask_labextension/clusterhandler.py
<https://github.com/dask/dask-labextension/pull/159/files#diff-450f68aa4a55b9547bafa19a575518e46b352b8cd9aa55df738fb6b9b248f400>
(2)
- *M* dask_labextension/dashboardhandler.py
<https://github.com/dask/dask-labextension/pull/159/files#diff-5f5e4c1ea4d990dafe0fdfe2e98140f0150cf0b727e88880e71280feda3afb57>
(53)
- *M* src/dashboard.tsx
<https://github.com/dask/dask-labextension/pull/159/files#diff-57ea6663adc6a185d3a797c096e3bacbd9e857ba18cb37077f4ed849c751ff81>
(209)
- *M* src/index.ts
<https://github.com/dask/dask-labextension/pull/159/files#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80>
(48)
Patch Links:
- https://github.com/dask/dask-labextension/pull/159.patch
- https://github.com/dask/dask-labextension/pull/159.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#159>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACKZTBUHZ2Y2TBPP3LKNI3ST72ADANCNFSM4UUENWAA>
.
|
Unless there are objections, I'll merge this today |
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 #158) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
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