Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

data_server_proxied URL prefix shouldn't be ../ #40

Closed
dlukes opened this issue Apr 4, 2020 · 4 comments
Closed

data_server_proxied URL prefix shouldn't be ../ #40

dlukes opened this issue Apr 4, 2020 · 4 comments

Comments

@dlukes
Copy link
Contributor

dlukes commented Apr 4, 2020

The data_server_proxied URL currently starts with ../, which I think is wrong (see also vega/altair#611 (comment)): a URL like /foo/bar/lab is in the bar "directory", not the lab directory (it would need a trailing slash for that), so navigating to ../proxy/whatever from /foo/bar/lab lands you actually in /foo/proxy/whatever instead of /foo/bar/proxy/whatever, which is the original intent as far as I can tell.

I can't seem to find any authoritative reference on this, but try running the following in this page's console:

// window.location is https://github.com/altair-viz/altair_data_server/issues/40

// this fetches https://github.com/altair-viz/altair_data_server/issues/, not
// https://github.com/altair-viz/altair_data_server/issues/40
fetch(".").then(r => console.log(r))

// this fetches https://github.com/altair-viz/altair_data_server/
fetch("..").then(r => console.log(r))

The problem probably doesn't manifest in a locally run JupyterLab where /lab is directly at the root, so there's no way to navigate up a "directory" and the proxied request ends up at /proxy/whatever. But when JupyterLab is running under JupyterHub and there's an additional /user/username/ prefix, then it makes the altair_data_server request land at /user/proxy/whatever instead of the correct /user/username/proxy/whatever.

One solution would be to get rid of the leading ../, but that won't play nicely with JupyterLab workspaces, because then the URL looks like .../lab/workspaces/workspace-name, in which case the correct relative prefix would actually be ../../. I'm not sure whether there's a way for altair_data_server to detect whether a workspace is active; I would tend to think not. Plus in theory, the same notebook+kernel can be opened both in the default workspace (with the plain URL ending in /lab) and in a named one, so altair_data_server would have to switch the prefix dynamically with each call.

I would instead suggest trying this:

  • make the data_server_proxied data transformer URL start with /proxy/...
  • add a data_server_jupyterhub data transformer where the URL starts with /user-redirect/proxy/..., which should correctly resolve to /user/username/proxy/... of whichever user is currently logged in

The con is that this assumes that JupyterLab (in the former case) or JupyterHub (in the latter) is actually running at the domain's root, but that could be solved by an optional prefix parameter:

alt.data_transformers.enable("data_server_proxied", prefix="/foo")

Maybe this would also solve #8 (though I'm not sure I entirely understand that issue, navigating to subfolders in JupyterLab doesn't change the URL for me, so I'm not sure how that can affect how the data_server_proxied URLs resolve).

@dlukes
Copy link
Contributor Author

dlukes commented Apr 4, 2020

Also, maybe it would make sense for the /user-redirect thing to be directly added to data_server_proxied and not create a separate data_server_jupyterhub data transformer -- I guess the proxied version only really makes sense on a JupyterHub instance facing the public internet which doesn't externally expose its ports. Everyone else can happily use the data_server data transformer.

@jakevdp
Copy link
Member

jakevdp commented Apr 5, 2020

I don't see any good answer here. I'm getting to the point where I'm ready to wipe my hands of this, remove all attempts at providing Jupyter compatibility, and just say it's up to the user to figure it out ¯_(ツ)_/¯

@dlukes
Copy link
Contributor Author

dlukes commented Apr 5, 2020

I understand the frustration... And I'm sorry for communicating this in a way that triggered it :) I was optimizing for writing speed and facts, not empathy (I was trying to get to bed relatively early, our baby daughter sleeps best during the first part of the night).

just say it's up to the user to figure it out ¯_(ツ)_/¯

Then please consider at least dropping the ../ prefix, which really shouldn't be there imho (see arguments above), starting the URL with /proxy/..., and providing a url_prefix argument which allows the user to prepend whatever they need to figure it out on their own, along with documenting the typical use cases:

  • Running whatever setup on localhost? Just use data_server to directly access the port the service is running on.
  • Running standalone JupyterLab under the default /lab prefix? data_server_proxy should work out-of-the-box, provided jupyter-server-proxy is installed.
  • Running standalone JupyterLab under a custom prefix, e.g. /foo/bar/lab? Use data_server_proxy and set url_prefix="/foo/bar".
  • Running JupyterLab under JupyterHub? Use data_server_proxy and set url_prefix="/user-redirect", or possibly url_prefix="/foo/bar/user-redirect" if running under a /foo/bar prefix.

Or something along those lines.

I'll happily come up with a pull request in the next few days if it's OK with you and you don't really want to deal with this right now. And thank you for all your hard work on everything Python-related, not just Altair! It's always a pleasure to read your writing or watch a conference talk of yours.

@jakevdp
Copy link
Member

jakevdp commented Apr 5, 2020

Thanks – a PR would be great.

dlukes added a commit to dlukes/altair_data_server that referenced this issue Apr 5, 2020
Suggests a fix for altair-viz#40 and documents how to best use data_server_proxied
with JupyterHub.
dlukes added a commit to dlukes/altair_data_server that referenced this issue Apr 11, 2020
Suggests a fix for altair-viz#40 and documents how to best use data_server_proxied
with JupyterHub.
dlukes added a commit to dlukes/altair_data_server that referenced this issue Apr 11, 2020
Suggests a fix for altair-viz#40 and documents how to best use data_server_proxied
with JupyterHub.
dlukes added a commit to dlukes/altair_data_server that referenced this issue Apr 12, 2020
Suggests a fix for altair-viz#40 and documents how to best use data_server_proxied
with JupyterHub.
@dlukes dlukes closed this as completed Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants