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

Open ExtensionApps at extension's landing page #113

Merged
merged 2 commits into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
46 changes: 42 additions & 4 deletions jupyter_server/extension/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,38 @@ def static_url_prefix(self):
help=_("The default URL to redirect to from `/`")
)

custom_display_url = Unicode(u'', config=True,
help=_("""Override URL shown to users.

Replace actual URL, including protocol, address, port and base URL,
with the given value when displaying URL to the users. Do not change
the actual connection URL. If authentication token is enabled, the
token is added to the custom URL automatically.

This option is intended to be used when the URL to display to the user
cannot be determined reliably by the Jupyter server (proxified
or containerized setups for example).""")
)

@default('custom_display_url')
def _default_custom_display_url(self):
"""URL to display to the user."""
# Get url from server.
url = url_path_join(self.serverapp.base_url, self.default_url)
return self.serverapp.get_url(self.serverapp.ip, url)
Copy link
Member

Choose a reason for hiding this comment

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

Won't this fail if self.token is truthy?

Copy link
Member Author

Choose a reason for hiding this comment

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

In #115, this returns a url without a token. That's what we want here. The display_url property adds a token to custom_display_url.


def _write_browser_open_file(self, url, fh):
"""Use to hijacks the server's browser-open file and open at
the extension's homepage.
"""
# Ignore server's url
del url
path = url_path_join(self.serverapp.base_url, self.default_url)
url = self.serverapp.get_url(path=path)
jinja2_env = self.serverapp.web_app.settings['jinja2_env']
template = jinja2_env.get_template('browser-open.html')
fh.write(template.render(open_url=url))

def initialize_settings(self):
"""Override this method to add handling of settings."""
pass
Expand Down Expand Up @@ -256,15 +288,21 @@ def initialize(self, serverapp, argv=[]):
self._prepare_settings()
self._prepare_handlers()

def start(self, **kwargs):
def start(self):
"""Start the underlying Jupyter server.

Server should be started after extension is initialized.
"""
# Start the browser at this extensions default_url.
self.serverapp.default_url = self.default_url
# Override the browser open file to
# Override the server's display url to show extension's display URL.
self.serverapp.custom_display_url = self.custom_display_url
# Override the server's default option and open a broswer window.
self.serverapp.open_browser = True
# Hijack the server's browser-open file to land on
# the extensions home page.
self.serverapp._write_browser_open_file = self._write_browser_open_file
Copy link
Member

Choose a reason for hiding this comment

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

This seems like the wrong way to accomplish this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, maybe. Let me explain what my reason:

I found two ways for an extension to reroute the server landing page.

  1. Change the server's default_url and have it land on the extension. There were two challenges with this approach:
    • The server's default_url needs to be set to <extension>.default_url before the server.initialize() is called, otherwise we have to change a bunch of downstream trails of default_url in serverapp. The issue is that <extension>.default_url doesn't get set until after server is initialized.
    • The server's default_url truly is a different trait than the extension's default_url. In my mind, they could/should point to two different urls (though, I don't have an example of when someone might want to do this right now).
  2. Change the server's browser open file. I chose this approach because:
    • The server doesn't open a browser window by default, anyways. In fact, a browser window doesn't really make sense without an ExtensionApp.
    • This file is only created if the server was started by an extension. In that case, I think it makes sense that the server's browser file always opens to the extension's homepage.
    • The ServerApp.default_url and ExtensionApp.default_url can be pointing to different URLs (maybe a plus?).

Copy link
Member

Choose a reason for hiding this comment

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

When I wrote "wrong way', I meant to overwrite a protected method on the app. This is basically monkey-patching, and it means that there isn't really a clear API boundary between the two objects.

# Start the server.
self.serverapp.start(**kwargs)
self.serverapp.start()

def stop(self):
"""Stop the underlying Jupyter server.
Expand Down
55 changes: 39 additions & 16 deletions jupyter_server/serverapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import time
import warnings
import webbrowser
import urllib

from base64 import encodebytes
from jinja2 import Environment, FileSystemLoader
Expand Down Expand Up @@ -1357,31 +1358,53 @@ def init_webapp(self):
@property
def display_url(self):
if self.custom_display_url:
url = self.custom_display_url
if not url.endswith('/'):
url += '/'
parts = urllib.parse.urlparse(self.custom_display_url)
path = parts.path
ip = parts.hostname
else:
path = None
if self.ip in ('', '0.0.0.0'):
ip = "%s" % socket.gethostname()
else:
ip = self.ip
url = self._url(ip)

token = None
if self.token:
# Don't log full token if it came from config
token = self.token if self._token_generated else '...'
url = (url_concat(url, {'token': token})
+ '\n or '
+ url_concat(self._url('127.0.0.1'), {'token': token}))

url = (
self.get_url(ip=ip, path=path, token=token)
+ '\n or '
+ self.get_url(ip='127.0.0.1', path=path, token=token)
)
return url

@property
def connection_url(self):
ip = self.ip if self.ip else 'localhost'
return self._url(ip)
return self.get_url(ip=ip)

def _url(self, ip):
proto = 'https' if self.certfile else 'http'
return "%s://%s:%i%s" % (proto, ip, self.port, self.base_url)
def get_url(self, ip=None, path=None, token=None):
"""Build a url for the application.
"""
if not ip:
ip = self.ip
Copy link
Member

Choose a reason for hiding this comment

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

This should probably raise if not self.ip, or default to localhost.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

if not path:
path = url_path_join(self.base_url, self.default_url)
# Build query string.
if self.token:
token = urllib.parse.urlencode({'token': token})
Copy link
Member

Choose a reason for hiding this comment

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

This should probably raise an error if token is None or default to '...'

Copy link
Member Author

Choose a reason for hiding this comment

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

What if token is actually None, though?

Before this PR, token=None returned a url without a token. This PR maintains that pattern. Do we always want to require a token and change previous behavior in the PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh wait, I missed your actual point... 🤦‍♂

Yes, this will return token=None in the url. Good catch!

Copy link
Member Author

Choose a reason for hiding this comment

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

The conditional should be if token, not if self.token. That was a typo.

ie.

if token:
    token = urllib.parse.urlencode({'token': token})

There are cases where we don't want the token, for example in self.connection_url.

# Build the URL Parts to dump.
urlparts = urllib.parse.ParseResult(
scheme='https' if self.certfile else 'http',
netloc="{ip}:{port}".format(ip=ip, port=self.port),
path=path,
params=None,
query=token,
fragment=None
)
return urlparts.geturl()

def init_terminals(self):
if not self.terminals_enabled:
Expand Down Expand Up @@ -1429,7 +1452,7 @@ def _confirm_exit(self):
"""
info = self.log.info
info(_('interrupted'))
print(self.notebook_info())
print(self.running_server_info())
yes = _('y')
no = _('n')
sys.stdout.write(_("Shutdown this Jupyter server (%s/[%s])? ") % (yes, no))
Expand Down Expand Up @@ -1457,7 +1480,7 @@ def _signal_stop(self, sig, frame):
self.io_loop.add_callback_from_signal(self.io_loop.stop)

def _signal_info(self, sig, frame):
print(self.notebook_info())
print(self.running_server_info())

def init_components(self):
"""Check the components submodule, and warn if it's unclean"""
Expand Down Expand Up @@ -1586,7 +1609,7 @@ def cleanup_kernels(self):
self.log.info(kernel_msg % n_kernels)
self.kernel_manager.shutdown_all()

def notebook_info(self, kernel_count=True):
def running_server_info(self, kernel_count=True):
"Return the current working directory and the server url information"
info = self.contents_manager.info_string() + "\n"
if kernel_count:
Expand Down Expand Up @@ -1715,7 +1738,7 @@ def start(self):
self.exit(1)

info = self.log.info
for line in self.notebook_info(kernel_count=False).split("\n"):
for line in self.running_server_info(kernel_count=False).split("\n"):
info(line)
info(_("Use Control-C to stop this server and shut down all kernels (twice to skip confirmation)."))
if 'dev' in jupyter_server.__version__:
Expand All @@ -1735,7 +1758,7 @@ def start(self):
# with auth info.
self.log.critical('\n'.join([
'\n',
'To access the notebook, open this file in a browser:',
'To access the server, open this file in a browser:',
' %s' % urljoin('file:', pathname2url(self.browser_open_file)),
'Or copy and paste one of these URLs:',
' %s' % self.display_url,
Expand Down