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

Support for JupyterHub proxy #193

Closed
bnouvelbmll opened this issue Apr 29, 2020 · 10 comments
Closed

Support for JupyterHub proxy #193

bnouvelbmll opened this issue Apr 29, 2020 · 10 comments

Comments

@bnouvelbmll
Copy link

JupyterHub has an extension that allows to proxy port for user.
https://github.com/jupyterhub/jupyter-server-proxy

It seems it would be a much more appropriate way to deploy dtale on jupyterhub on k8s.
However, it requires to change the base URL of the application so it supports the proxied URL
which looks like - this implies tweaking the flask/dash deployment, and ensuring all redirect and static resources are working correctly with it. It seems it is mostly tweak for app.py.

https://myjupyterlab.com/user/myuserlogin/proxy/40000/dtale/main/1

I had a quick look at it but I haven't managed to get it to work within that setup yet.

@aschonfeld
Copy link
Collaborator

@bnouvelbmll thanks for this! It's definitely very cool. I've done some digging though and D-Tale falls over really quick with this because its using relative URLs. For example when loading JS or CSS its using the following within its jinja template:
"{{ url_for('static', filename='css/main.css') }}"
which equates to:
http://[hostname]:[dtale-port]/css/main.css

But when you use jupyter-server-proxy that URL will resolve to:
http://[jupyterhub-server/port]/css/main.css

The way that we got around this at the time was by having our proxy URL simply mask the base
portion of the URL so the routes still worked. So if there was some way to encode the user/[jupyterhub-user]/proxy/[dtale-port] within the base URL it would work out of the box.

That being said, I am working on exposing the static data (JS, CSS, Flask endpoints) in a more relative way. So prefacing any endpoints with ../../ and it looks like it works. I just need to see if using relative endpoints will break anything else.

@bnouvelbmll
Copy link
Author

url_for static can return the right url if configured/implemented/patch adequately.

I had a look at the code.

The two main issues for me seem to be some absolute link to "/" for instance where the code create html link to "/chart" without considering the possibilty of a prefix .

And possibly some confusion in some dash libraries between routes_pathname_prefix and 'APPLICATION_ROOT' for flask.

More precisely, setting APPLICATION_ROOT mostly is usable to move the application in the right folder, but in the browser it seems that many assets are still queried at the root or in routes_pathname_prefix.

@bnouvelbmll
Copy link
Author

More precisely, using draft code below, it seems that it we can monkey patch for most issues from the python generated content but it seems that there are still absolute link referring to '/' in html and javascript. It seems that you may need to add tests to ensure the dtale also work when APPLICATION_ROOT is changed.

import flask
import os
from flask import helpers

APP_ROOT='/user/bnouvelbmll/proxy/40000'

if "oflask" not in globals():
   oflask= flask.Flask

if "ourl_for" not in globals():
   ourl_for= helpers.url_for

class NFlask(oflask):
    def __init__(self, *args, **kwargs):
        kwargs['static_url_path']=''
        super().__init__(*args,**kwargs)
        self.config["APPLICATION_ROOT"]=APP_ROOT
        self.jinja_env.globals["url_for"]=url_for
    
    def update_template_context(self, context):
        super().update_template_context(context)
        print(context)
        context["url_for"]=url_for
    
    
        
def url_for(endpoint, *args, **kwargs):    
    if endpoint == 'static':
        if "filename" in kwargs:
            return  APP_ROOT+'/'+kwargs["filename"]
        return APP_ROOT+'/'+args[0]
    print(args,kwargs)
    return ourl_for(endpoint, *args, **kwargs)
        
        
helpers.url_for=url_for
flask.url_for=url_for
flask.Flask=NFlask

import dash


if "odash" not in globals():
   odash= dash.Dash

class NDash(odash):
    def __init__(self, *args, **kwargs):
        print("***")        
        print(kwargs)
        p=APP_ROOT
        kwargs['routes_pathname_prefix']='/charts/'         #
        kwargs['requests_pathname_prefix']=p+'/charts/'
        kwargs['external_stylesheets']= [p+'/css/main.css', p+'/css/dash.css']
        kwargs['external_scripts'] =[p+'/dash/components_bundle.js', p+'/dash/custom_bundle.js', p+'/dist/base_styles_bundle.js']
        kwargs['assets_url_path']=p
        kwargs['assets_external_path']=p+'/assets'
        super().__init__(*args,**kwargs)
        
        
    def get_asset_url(self, path):        
        return APP_ROOT+'/charts'+path

    def get_relative_path(self, path):        
        return APP_ROOT+'/charts'+path
    
dash.Dash=NDash
helpers.url_for=url_for
flask.url_for=url_for


import dtale        

@aschonfeld
Copy link
Collaborator

So I've started a branch for this and I'm getting closer. Definitely needs some more cleanup on the front-end stuff.

https://github.com/man-group/dtale/tree/jupyter-server-proxy-updates

@bnouvelbmll
Copy link
Author

Thanks, nice. I gave it a try, but I have not managed to get it working by specifying app_root. Not sure why. When retrying with the patching, I noticed that the charts were working, but I still could not get the grid to work.

@aschonfeld
Copy link
Collaborator

So it the grid and the charts were working for me (I managed to get jupyterhub w/ jupyter-server-proxy running) with that branch I just needed to fix how the endpoints on calls back to the server from the React code were working. Maybe take a stab on sunday after I get this latest release out. Thanks for all your hard work on this. 🙏

@bnouvelbmll
Copy link
Author

I repulled and I managed indeed to get the grid and plots working directly with the code in the branch. Great work! It is nice to see that the most important features are already there.

It is also nice that the part of the url related to the port is added automatically to the app_root. It is useful in this precise scenario where we use jupyter_proxy. Now, to cover all types of deployments its probably a good idea to keep this as a toggle so that people that don't use jupyter_server_proxy but still want to deploy to alternate root can do it.

@aschonfeld
Copy link
Collaborator

@bnouvelbmll sorry been away moving for the last week. Just pushed a bunch of updates to my branch. I think I'm very close if not done. Just need to do some testing around the use case where you don't specify an app_root parameter.

Also, I still need to get the jupyter notebook cell to print the correct url.

@aschonfeld
Copy link
Collaborator

@bnouvelbmll I believe I have put the finishing touches on my branch so that when specifying app_root it will behave the exact same as when you don't (including opening your session directly within your notebook):

image

Let me know if you have any issues. I'm going to do some more testing and add some tests and if all is still good I'll release a new version with these changes.

aschonfeld added a commit that referenced this issue May 20, 2020
* #193: Support for JupyterHub Proxy
aschonfeld added a commit that referenced this issue May 20, 2020
* #193: Support for JupyterHub Proxy
@aschonfeld aschonfeld mentioned this issue May 20, 2020
aschonfeld added a commit that referenced this issue May 20, 2020
* #193: Support for JupyterHub Proxy
@aschonfeld
Copy link
Collaborator

This functionality has been released in v1.8.13, here's the documentation on how it works. Thanks for all your help with this!

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

No branches or pull requests

2 participants