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

adding jupyterlab file paths for preview #925

Merged
merged 2 commits into from
Aug 15, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions binderhub/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ def get(self, provider_prefix, _unescaped_spec):
org, repo_name, ref = spec.split('/', 2)
# NOTE: tornado unquotes query arguments too -> notebooks%2Findex.ipynb becomes notebooks/index.ipynb
filepath = self.get_argument('filepath', '').lstrip('/')

# Check if we have a JupyterLab + file path, if so then use it for the filepath
urlpath = self.get_argument('urlpath', '').lstrip('/')
if 'lab/tree/' in urlpath:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if 'lab/tree/' in urlpath:
if urlpath.startswith("lab/tree"):

would this be an improvement/less prone to matching accidentally?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally doing it this way to allow for workspaces in the url too, but upon looking it seems like that'd actually be a more complicated rule. What if instead we did something like:

if urlpath.startswith("lab") and `/tree/` in urlpath:

and then do the split on /tree/ and take whatever is to the right of the first instance of /tree/?

Copy link
Member Author

@choldgraf choldgraf Aug 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is now in the latest push, it seemed to work for me locally (both regular lab URLs and workspace URLs)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What different kinds of URLs could we get here? I am not really familiar with what Jupyter Lab does/what workspaces are :-/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the link I sent above, that's what I was using to figure out what urls to expect

filepath = urlpath.split('lab/tree/')[-1]

blob_or_tree = 'blob' if filepath else 'tree'
nbviewer_url = f'{nbviewer_url}/{org}/{repo_name}/{blob_or_tree}/{ref}/{filepath}'
self.render_template(
Expand Down