WIP: Cleanup of handlers/urls #20
Closed
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
|
Why not have it support an overrideable login handler like the server does, then the default login handler can just do no auth. |
voila/app.py
Outdated
| (url_path_join(base_url, r'/api/kernels/%s' % _kernel_id_regex), KernelHandler), | ||
| (url_path_join(base_url, r'/api/kernels/%s/channels' % _kernel_id_regex), ZMQChannelsHandler), | ||
| (url_path_join(base_url, r'/voila/api/kernels/%s' % _kernel_id_regex), KernelHandler), | ||
| (url_path_join(base_url, r'/voila/api/kernels/%s/channels' % _kernel_id_regex), ZMQChannelsHandler), |
There was a problem hiding this comment.
👍 it should indeed not be the same handler as for the notebook.
maartenbreddels
changed the title
maartenbreddels
force-pushed
the
no_authentication
branch
from
cae2287 to
3994e81
Compare
maartenbreddels
force-pushed
the
no_authentication
branch
from
3994e81 to
11a7d06
Compare
|
@timkpaine yes that was a good idea actually I started doing that, but decided to first get to a clean PR that sets up the handlers correctly and cleans up the code. I'll open a PR later for the auth. @SylvainCorlay if you decide to merge it, just squash it, I just want to keep the history of this branch so I didn't squash it myself. |
maartenbreddels
commented
Nov 20, 2018
|
|
||
| def add_base_url_to_handlers(base_url, handlers): | ||
| """Adds the base_url in front of the urls in the Tornado handlers""" | ||
| return [tuple([url_path_join(base_url, url)] + list(tail)) for url, *tail in handlers] |
There was a problem hiding this comment.
btw, this is py3 only, I guess that is ok
|
Replaced bu #41 |
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.
For voila, I think it does not make sense (at least by default, or optional) to have authentication. At least to have a different behaviour for the notebook and the voila server extension.
What this allows, is to have a notebook server (normal authentication) but serve notebooks via voila without. This is very much a proof of concept and deserves more consideration. It exposes some of the issues that with notebook/jupyter_server has when you want to do this. For instance, to make
ZMQChannelsHandlernot check auth, we need to return a fake user.I think in general, voila shakes the box a bit, because instead of exposing kernels/notebooks as a security issue, now all of the sudden a kernel can be safe to expose (since it does not allow execution). Also to keep in mind is that multiuser all of the sudden makes sense.
Long term vision/idea: You could have a notebook run under your account, it ran a simulation or report for ~2 hours. Now users can connect to voila, they just want to see this report, which can be without authentication, or with authentication that has nothing to do with the notebook authentication.