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

Enable extensions to control the file_to_run #415

Merged
merged 1 commit into from
Feb 18, 2021
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
5 changes: 4 additions & 1 deletion jupyter_server/extension/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ def get_extension_point(cls):
def _default_url(self):
return self.extension_url

file_url_prefix = Unicode('notebooks')

# Is this linked to a serverapp yet?
_linked = Bool(False)

Expand Down Expand Up @@ -337,7 +339,8 @@ def _jupyter_server_config(self):
base_config = {
"ServerApp": {
"default_url": self.default_url,
"open_browser": self.open_browser
"open_browser": self.open_browser,
"file_url_prefix": self.file_url_prefix
}
}
base_config["ServerApp"].update(self.serverapp_config)
Expand Down
4 changes: 1 addition & 3 deletions jupyter_server/pytest_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,7 @@ def jp_serverapp(
"""Starts a Jupyter Server instance based on the established configuration values."""
app = jp_configurable_serverapp(config=jp_server_config, argv=jp_argv)
yield app
app.remove_server_info_file()
app.remove_browser_open_file()
app.cleanup_kernels()
app._cleanup()


@pytest.fixture
Expand Down
166 changes: 127 additions & 39 deletions jupyter_server/serverapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import webbrowser
import urllib
import inspect
import pathlib

from base64 import encodebytes
try:
Expand Down Expand Up @@ -102,7 +103,13 @@
from jupyter_server._sysinfo import get_sys_info

from ._tz import utcnow, utcfromtimestamp
from .utils import url_path_join, check_pid, url_escape, urljoin, pathname2url
from .utils import (
url_path_join,
check_pid,
url_escape,
urljoin,
pathname2url
)

from jupyter_server.extension.serverextension import ServerExtensionApp
from jupyter_server.extension.manager import ExtensionManager
Expand Down Expand Up @@ -620,10 +627,15 @@ def _default_log_format(self):
return u"%(color)s[%(levelname)1.1s %(asctime)s.%(msecs).03d %(name)s]%(end_color)s %(message)s"

# file to be opened in the Jupyter server
file_to_run = Unicode('', config=True)
file_to_run = Unicode('',
help="Open the named file when the application is launched."
).tag(config=True)

# Network related information
file_url_prefix = Unicode('notebooks',
help="The URL prefix where files are opened directly."
).tag(config=True)

# Network related information
allow_origin = Unicode('', config=True,
help="""Set the Access-Control-Allow-Origin header

Expand Down Expand Up @@ -1195,6 +1207,13 @@ def _default_browser_open_file(self):
basename = "jpserver-%s-open.html" % os.getpid()
return os.path.join(self.runtime_dir, basename)

browser_open_file_to_run = Unicode()

@default('browser_open_file_to_run')
def _default_browser_open_file_to_run(self):
basename = "jpserver-file-to-run-%s-open.html" % os.getpid()
return os.path.join(self.runtime_dir, basename)

pylab = Unicode('disabled', config=True,
help=_("""
DISABLED: use %pylab or %matplotlib in the notebook to enable matplotlib.
Expand Down Expand Up @@ -1254,7 +1273,7 @@ def _root_dir_validate(self, proposal):
# If we receive a non-absolute path, make it absolute.
value = os.path.abspath(value)
if not os.path.isdir(value):
raise TraitError(trans.gettext("No such notebook dir: '%r'") % value)
raise TraitError(trans.gettext("No such directory: '%r'") % value)
return value

@observe('root_dir')
Expand Down Expand Up @@ -1874,10 +1893,71 @@ def remove_server_info_file(self):
os.unlink(self.info_file)
except OSError as e:
if e.errno != errno.ENOENT:
raise
raise;

def _resolve_file_to_run_and_root_dir(self):
"""Returns a relative path from file_to_run
to root_dir. If root_dir and file_to_run
are incompatible, i.e. on different subtrees,
crash the app and log a critical message. Note
that if root_dir is not configured and file_to_run
is configured, root_dir will be set to the parent
directory of file_to_run.
"""
rootdir_abspath = pathlib.Path(self.root_dir).resolve()
file_rawpath = pathlib.Path(self.file_to_run)
combined_path = (rootdir_abspath / file_rawpath).resolve()
is_child = str(combined_path).startswith(str(rootdir_abspath))

if is_child:
if combined_path.parent != rootdir_abspath:
self.log.debug(
"The `root_dir` trait is set to a directory that's not "
"the immediate parent directory of `file_to_run`. Note that "
"the server will start at `root_dir` and open the "
"the file from the relative path to the `root_dir`."
)
return str(combined_path.relative_to(rootdir_abspath))

self.log.critical(
"`root_dir` and `file_to_run` are incompatible. They "
"don't share the same subtrees. Make sure `file_to_run` "
"is on the same path as `root_dir`."
)
self.exit(1)

def _write_browser_open_file(self, url, fh):
if self.token:
url = url_concat(url, {'token': self.token})
url = url_path_join(self.connection_url, url)

jinja2_env = self.web_app.settings['jinja2_env']
template = jinja2_env.get_template('browser-open.html')
fh.write(template.render(open_url=url, base_url=self.base_url))

def write_browser_open_files(self):
"""Write an `browser_open_file` and `browser_open_file_to_run` files

This can be used to open a file directly in a browser.
"""
# default_url contains base_url, but so does connection_url
self.write_browser_open_file()

# Create a second browser open file if
# file_to_run is set.
if self.file_to_run:
# Make sure file_to_run and root_dir are compatible.
file_to_run_relpath = self._resolve_file_to_run_and_root_dir()

file_open_url = url_escape(
url_path_join(self.file_url_prefix, *file_to_run_relpath.split(os.sep))
)

with open(self.browser_open_file_to_run, 'w', encoding='utf-8') as f:
self._write_browser_open_file(file_open_url, f)

def write_browser_open_file(self):
"""Write an nbserver-<pid>-open.html file
"""Write an jpserver-<pid>-open.html file

This can be used to open the notebook in a browser
"""
Expand All @@ -1887,17 +1967,21 @@ def write_browser_open_file(self):
with open(self.browser_open_file, 'w', encoding='utf-8') as f:
self._write_browser_open_file(open_url, f)

def _write_browser_open_file(self, url, fh):
if self.token:
url = url_concat(url, {'token': self.token})
url = url_path_join(self.connection_url, url)
def remove_browser_open_files(self):
"""Remove the `browser_open_file` and `browser_open_file_to_run` files
created for this server.

jinja2_env = self.web_app.settings['jinja2_env']
template = jinja2_env.get_template('browser-open.html')
fh.write(template.render(open_url=url, base_url=self.base_url))
Ignores the error raised when the file has already been removed.
"""
self.remove_browser_open_file()
try:
os.unlink(self.browser_open_file_to_run)
except OSError as e:
if e.errno != errno.ENOENT:
raises

def remove_browser_open_file(self):
"""Remove the nbserver-<pid>-open.html file created for this server.
"""Remove the jpserver-<pid>-open.html file created for this server.

Ignores the error raised when the file has already been removed.
"""
Expand All @@ -1907,42 +1991,40 @@ def remove_browser_open_file(self):
if e.errno != errno.ENOENT:
raise

def launch_browser(self):
try:
browser = webbrowser.get(self.browser or None)
except webbrowser.Error as e:
self.log.warning(_('No web browser found: %s.') % e)
browser = None

if not browser:
return

def _prepare_browser_open(self):
if not self.use_redirect_file:
uri = self.default_url[len(self.base_url):]

if self.token:
uri = url_concat(uri, {'token': self.token})

if self.file_to_run:
if not os.path.exists(self.file_to_run):
self.log.critical(_("%s does not exist") % self.file_to_run)
self.exit(1)

relpath = os.path.relpath(self.file_to_run, self.root_dir)
uri = url_escape(url_path_join('notebooks', *relpath.split(os.sep)))

# Write a temporary file to open in the browser
fd, open_file = tempfile.mkstemp(suffix='.html')
with open(fd, 'w', encoding='utf-8') as fh:
self._write_browser_open_file(uri, fh)
# Create a separate, temporary open-browser-file
# pointing at a specific file.
open_file = self.browser_open_file_to_run
else:
# otherwise, just return the usual open browser file.
open_file = self.browser_open_file

if self.use_redirect_file:
assembled_url = urljoin('file:', pathname2url(open_file))
else:
assembled_url = url_path_join(self.connection_url, uri)

return assembled_url, open_file

def launch_browser(self):
try:
browser = webbrowser.get(self.browser or None)
except webbrowser.Error as e:
self.log.warning(_('No web browser found: %s.') % e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi,

When I start jupyter from CUI, the program will stop with the following error.

...
  File "/root/.pyenv/versions/3.7.3/lib/python3.7/site-packages/jupyter_server/serverapp.py", line 2020, in launch_browser
    self.log.warning(_('No web browser found: %s.') % e)
UnboundLocalError: local variable '_' referenced before assignment

As shown in the error message, by removing the _ from the relevant part, jupyter can be executed successfully.

Looking at this script, it seems that underscores _ are inserted in self.log.info() and so on.
I am not familiar with this notation, so I would like to confirm it.

Thanks in advance,

Copy link
Member

Choose a reason for hiding this comment

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

Notebook server and Jupyter Server (due to its derivation from the former) define _ to be the method trans.gettext such that any strings wrapped in _() are candidates for translation lookups. However, this precludes the common use of _ to represent an ignored value in the same method - as is the case here (a few lines below):

        assembled_url, _ = self._prepare_browser_open()

Seems like we should no longer return open_file from _prepare_browser_open() (since this is the only call to it) or adjust the name for the ignored value.

I don't know the history of using _ for trans.gettext but using something like _i18n instead seems more clear while not being too intrusive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification!

Now I see how _ is used here, but as it is pointed out, it is neither clear nor safe usage, I guess...
This is a common way to represent ignored variables with _ in python, so I agree with @kevin-bates's suggestion!

Copy link
Member

@kevin-bates kevin-bates Feb 22, 2021

Choose a reason for hiding this comment

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

I've opened #424 to help avoid this in the future.

browser = None

if not browser:
return

assembled_url, _ = self._prepare_browser_open()

b = lambda: browser.open(assembled_url, new=self.webbrowser_open_new)
threading.Thread(target=b).start()

Expand Down Expand Up @@ -1970,7 +2052,7 @@ def start_app(self):
"resources section at https://jupyter.org/community.html."))

self.write_server_info_file()
self.write_browser_open_file()
self.write_browser_open_files()

# Handle the browser opening.
if self.open_browser:
Expand All @@ -1987,6 +2069,14 @@ def start_app(self):
' %s' % self.display_url,
]))

def _cleanup(self):
"""General cleanup of files and kernels created
by this instance ServerApp.
"""
self.remove_server_info_file()
self.remove_browser_open_files()
self.cleanup_kernels()

def start_ioloop(self):
"""Start the IO Loop."""
self.io_loop = ioloop.IOLoop.current()
Expand All @@ -2000,9 +2090,7 @@ def start_ioloop(self):
except KeyboardInterrupt:
self.log.info(_("Interrupted..."))
finally:
self.remove_server_info_file()
self.remove_browser_open_file()
self.cleanup_kernels()
self._cleanup()

def start(self):
""" Start the Jupyter server app, after initialization
Expand Down
91 changes: 91 additions & 0 deletions tests/test_serverapp.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import getpass
import pathlib
import pytest
import logging
from unittest.mock import patch
Expand Down Expand Up @@ -117,3 +118,93 @@ def test_list_running_servers(jp_serverapp, jp_web_app):
servers = list(list_running_servers(jp_serverapp.runtime_dir))
assert len(servers) >= 1


@pytest.fixture
def prefix_path(jp_root_dir, tmp_path):
"""If a given path is prefixed with the literal
strings `/jp_root_dir` or `/tmp_path`, replace those
strings with these fixtures.

Returns a pathlib Path object.
"""
def _inner(rawpath):
path = pathlib.PurePosixPath(rawpath)
if rawpath.startswith('/jp_root_dir'):
path = jp_root_dir.joinpath(*path.parts[2:])
elif rawpath.startswith('/tmp_path'):
path = tmp_path.joinpath(*path.parts[2:])
return pathlib.Path(path)
return _inner


@pytest.mark.parametrize(
"root_dir,file_to_run,expected_output",
[
(
None,
'notebook.ipynb',
'notebook.ipynb'
),
(
None,
'/tmp_path/path/to/notebook.ipynb',
'notebook.ipynb'
),
(
'/jp_root_dir',
'/tmp_path/path/to/notebook.ipynb',
SystemExit
),
(
'/tmp_path',
'/tmp_path/path/to/notebook.ipynb',
'path/to/notebook.ipynb'
),
(
'/jp_root_dir',
'notebook.ipynb',
'notebook.ipynb'
),
(
'/jp_root_dir',
'path/to/notebook.ipynb',
'path/to/notebook.ipynb'
),
]
)
def test_resolve_file_to_run_and_root_dir(
prefix_path,
root_dir,
file_to_run,
expected_output
):
# Verify that the Singleton instance is cleared before the test runs.
ServerApp.clear_instance()

# Setup the file_to_run path, in case the server checks
# if the directory exists before initializing the server.
file_to_run = prefix_path(file_to_run)
if file_to_run.is_absolute():
file_to_run.parent.mkdir(parents=True, exist_ok=True)
kwargs = {"file_to_run": str(file_to_run)}

# Setup the root_dir path, in case the server checks
# if the directory exists before initializing the server.
if root_dir:
root_dir = prefix_path(root_dir)
if root_dir.is_absolute():
root_dir.parent.mkdir(parents=True, exist_ok=True)
kwargs["root_dir"] = str(root_dir)

# Create the notebook in the given location
serverapp = ServerApp.instance(**kwargs)

if expected_output is SystemExit:
with pytest.raises(SystemExit):
serverapp._resolve_file_to_run_and_root_dir()
else:
relpath = serverapp._resolve_file_to_run_and_root_dir()
assert relpath == str(pathlib.Path(expected_output))

# Clear the singleton instance after each run.
ServerApp.clear_instance()