From 6b11b3564de7c8a682ec2725ddadd975a98e8156 2014-02-22 00:27:05 From: Brian E. Granger Date: 2014-02-22 00:27:05 Subject: [PATCH] Merge pull request #5116 from minrk/os_path reorganize who knows what about paths --- diff --git a/IPython/html/nbconvert/handlers.py b/IPython/html/nbconvert/handlers.py index 2fb6f45..2910205 100644 --- a/IPython/html/nbconvert/handlers.py +++ b/IPython/html/nbconvert/handlers.py @@ -76,15 +76,12 @@ class NbconvertFileHandler(IPythonHandler): exporter = get_exporter(format, config=self.config) path = path.strip('/') - os_path = self.notebook_manager.get_os_path(name, path) - if not os.path.isfile(os_path): - raise web.HTTPError(404, u'Notebook does not exist: %s' % name) + model = self.notebook_manager.get_notebook(name=name, path=path) - info = os.stat(os_path) - self.set_header('Last-Modified', tz.utcfromtimestamp(info.st_mtime)) + self.set_header('Last-Modified', model['last_modified']) try: - output, resources = exporter.from_filename(os_path) + output, resources = exporter.from_notebook_node(model['content']) except Exception as e: raise web.HTTPError(500, "nbconvert failed: %s" % e) diff --git a/IPython/html/notebookapp.py b/IPython/html/notebookapp.py index 683aac9..5ec9414 100644 --- a/IPython/html/notebookapp.py +++ b/IPython/html/notebookapp.py @@ -88,7 +88,7 @@ from IPython.utils.localinterfaces import localhost from IPython.utils import submodule from IPython.utils.traitlets import ( Dict, Unicode, Integer, List, Bool, Bytes, - DottedObjectName + DottedObjectName, TraitError, ) from IPython.utils import py3compat from IPython.utils.path import filefind, get_ipython_dir @@ -201,8 +201,11 @@ class NotebookWebApplication(web.Application): handlers.extend(load_handlers('services.clusters.handlers')) handlers.extend(load_handlers('services.sessions.handlers')) handlers.extend(load_handlers('services.nbconvert.handlers')) - handlers.extend([ - (r"/files/(.*)", AuthenticatedFileHandler, {'path' : settings['notebook_manager'].notebook_dir}), + # FIXME: /files/ should be handled by the Contents service when it exists + nbm = settings['notebook_manager'] + if hasattr(nbm, 'notebook_dir'): + handlers.extend([ + (r"/files/(.*)", AuthenticatedFileHandler, {'path' : nbm.notebook_dir}), (r"/nbextensions/(.*)", FileFindHandler, {'path' : settings['nbextensions_path']}), ]) # prepend base_url onto the patterns that we match @@ -278,7 +281,7 @@ aliases.update({ 'transport': 'KernelManager.transport', 'keyfile': 'NotebookApp.keyfile', 'certfile': 'NotebookApp.certfile', - 'notebook-dir': 'NotebookManager.notebook_dir', + 'notebook-dir': 'NotebookApp.notebook_dir', 'browser': 'NotebookApp.browser', }) @@ -507,6 +510,24 @@ class NotebookApp(BaseIPythonApplication): def _info_file_default(self): info_file = "nbserver-%s.json"%os.getpid() return os.path.join(self.profile_dir.security_dir, info_file) + + notebook_dir = Unicode(py3compat.getcwd(), config=True, + help="The directory to use for notebooks and kernels." + ) + + def _notebook_dir_changed(self, name, old, new): + """Do a bit of validation of the notebook dir.""" + if not os.path.isabs(new): + # If we receive a non-absolute path, make it absolute. + self.notebook_dir = os.path.abspath(new) + return + if not os.path.isdir(new): + raise TraitError("No such notebook dir: %r" % new) + + # setting App.notebook_dir implies setting notebook and kernel dirs as well + self.config.FileNotebookManager.notebook_dir = new + self.config.MappingKernelManager.root_dir = new + def parse_command_line(self, argv=None): super(NotebookApp, self).parse_command_line(argv) @@ -519,7 +540,7 @@ class NotebookApp(BaseIPythonApplication): self.log.critical("No such file or directory: %s", f) self.exit(1) if os.path.isdir(f): - self.config.FileNotebookManager.notebook_dir = f + self.notebook_dir = f elif os.path.isfile(f): self.file_to_run = f @@ -730,7 +751,7 @@ class NotebookApp(BaseIPythonApplication): 'port': self.port, 'secure': bool(self.certfile), 'base_url': self.base_url, - 'notebook_dir': os.path.abspath(self.notebook_manager.notebook_dir), + 'notebook_dir': os.path.abspath(self.notebook_dir), } def write_server_info_file(self): diff --git a/IPython/html/services/kernels/kernelmanager.py b/IPython/html/services/kernels/kernelmanager.py index 44409a3..a2fb8ff 100644 --- a/IPython/html/services/kernels/kernelmanager.py +++ b/IPython/html/services/kernels/kernelmanager.py @@ -16,6 +16,8 @@ Authors: # Imports #----------------------------------------------------------------------------- +import os + from tornado import web from IPython.kernel.multikernelmanager import MultiKernelManager @@ -23,6 +25,9 @@ from IPython.utils.traitlets import ( Dict, List, Unicode, ) +from IPython.html.utils import to_os_path +from IPython.utils.py3compat import getcwd + #----------------------------------------------------------------------------- # Classes #----------------------------------------------------------------------------- @@ -35,6 +40,17 @@ class MappingKernelManager(MultiKernelManager): return "IPython.kernel.ioloop.IOLoopKernelManager" kernel_argv = List(Unicode) + + root_dir = Unicode(getcwd(), config=True) + + def _root_dir_changed(self, name, old, new): + """Do a bit of validation of the root dir.""" + if not os.path.isabs(new): + # If we receive a non-absolute path, make it absolute. + self.root_dir = os.path.abspath(new) + return + if not os.path.exists(new) or not os.path.isdir(new): + raise TraitError("kernel root dir %r is not a directory" % new) #------------------------------------------------------------------------- # Methods for managing kernels and sessions @@ -44,8 +60,17 @@ class MappingKernelManager(MultiKernelManager): """notice that a kernel died""" self.log.warn("Kernel %s died, removing from map.", kernel_id) self.remove_kernel(kernel_id) - - def start_kernel(self, kernel_id=None, **kwargs): + + def cwd_for_path(self, path): + """Turn API path into absolute OS path.""" + os_path = to_os_path(path, self.root_dir) + # in the case of notebooks and kernels not being on the same filesystem, + # walk up to root_dir if the paths don't exist + while not os.path.exists(os_path) and os_path != self.root_dir: + os_path = os.path.dirname(os_path) + return os_path + + def start_kernel(self, kernel_id=None, path=None, **kwargs): """Start a kernel for a session an return its kernel_id. Parameters @@ -54,9 +79,14 @@ class MappingKernelManager(MultiKernelManager): The uuid to associate the new kernel with. If this is not None, this kernel will be persistent whenever it is requested. + path : API path + The API path (unicode, '/' delimited) for the cwd. + Will be transformed to an OS path relative to root_dir. """ if kernel_id is None: kwargs['extra_arguments'] = self.kernel_argv + if path is not None: + kwargs['cwd'] = self.cwd_for_path(path) kernel_id = super(MappingKernelManager, self).start_kernel(**kwargs) self.log.info("Kernel started: %s" % kernel_id) self.log.debug("Kernel args: %r" % kwargs) diff --git a/IPython/html/services/notebooks/filenbmanager.py b/IPython/html/services/notebooks/filenbmanager.py index e6fd5f1..8d4d68a 100644 --- a/IPython/html/services/notebooks/filenbmanager.py +++ b/IPython/html/services/notebooks/filenbmanager.py @@ -18,7 +18,6 @@ Authors: #----------------------------------------------------------------------------- import io -import itertools import os import glob import shutil @@ -28,8 +27,9 @@ from tornado import web from .nbmanager import NotebookManager from IPython.nbformat import current from IPython.utils.traitlets import Unicode, Dict, Bool, TraitError +from IPython.utils.py3compat import getcwd from IPython.utils import tz -from IPython.html.utils import is_hidden +from IPython.html.utils import is_hidden, to_os_path #----------------------------------------------------------------------------- # Classes @@ -46,7 +46,17 @@ class FileNotebookManager(NotebookManager): short `--script` flag. """ ) + notebook_dir = Unicode(getcwd(), config=True) + def _notebook_dir_changed(self, name, old, new): + """Do a bit of validation of the notebook dir.""" + if not os.path.isabs(new): + # If we receive a non-absolute path, make it absolute. + self.notebook_dir = os.path.abspath(new) + return + if not os.path.exists(new) or not os.path.isdir(new): + raise TraitError("notebook dir %r is not a directory" % new) + checkpoint_dir = Unicode(config=True, help="""The location in which to keep notebook checkpoints @@ -75,9 +85,9 @@ class FileNotebookManager(NotebookManager): def get_notebook_names(self, path=''): """List all notebook names in the notebook dir and path.""" path = path.strip('/') - if not os.path.isdir(self.get_os_path(path=path)): + if not os.path.isdir(self._get_os_path(path=path)): raise web.HTTPError(404, 'Directory not found: ' + path) - names = glob.glob(self.get_os_path('*'+self.filename_ext, path)) + names = glob.glob(self._get_os_path('*'+self.filename_ext, path)) names = [os.path.basename(name) for name in names] return names @@ -97,7 +107,7 @@ class FileNotebookManager(NotebookManager): Whether the path is indeed a directory. """ path = path.strip('/') - os_path = self.get_os_path(path=path) + os_path = self._get_os_path(path=path) return os.path.isdir(os_path) def is_hidden(self, path): @@ -116,10 +126,10 @@ class FileNotebookManager(NotebookManager): """ path = path.strip('/') - os_path = self.get_os_path(path=path) + os_path = self._get_os_path(path=path) return is_hidden(os_path, self.notebook_dir) - def get_os_path(self, name=None, path=''): + def _get_os_path(self, name=None, path=''): """Given a notebook name and a URL path, return its file system path. @@ -138,12 +148,9 @@ class FileNotebookManager(NotebookManager): server started), the relative path, and the filename with the current operating system's url. """ - parts = path.strip('/').split('/') - parts = [p for p in parts if p != ''] # remove duplicate splits if name is not None: - parts.append(name) - path = os.path.join(self.notebook_dir, *parts) - return path + path = path + '/' + name + return to_os_path(path, self.notebook_dir) def notebook_exists(self, name, path=''): """Returns a True if the notebook exists. Else, returns False. @@ -160,7 +167,7 @@ class FileNotebookManager(NotebookManager): bool """ path = path.strip('/') - nbpath = self.get_os_path(name, path=path) + nbpath = self._get_os_path(name, path=path) return os.path.isfile(nbpath) # TODO: Remove this after we create the contents web service and directories are @@ -168,13 +175,13 @@ class FileNotebookManager(NotebookManager): def list_dirs(self, path): """List the directories for a given API style path.""" path = path.strip('/') - os_path = self.get_os_path('', path) + os_path = self._get_os_path('', path) if not os.path.isdir(os_path) or is_hidden(os_path, self.notebook_dir): raise web.HTTPError(404, u'directory does not exist: %r' % os_path) dir_names = os.listdir(os_path) dirs = [] for name in dir_names: - os_path = self.get_os_path(name, path) + os_path = self._get_os_path(name, path) if os.path.isdir(os_path) and not is_hidden(os_path, self.notebook_dir): try: model = self.get_dir_model(name, path) @@ -189,7 +196,7 @@ class FileNotebookManager(NotebookManager): def get_dir_model(self, name, path=''): """Get the directory model given a directory name and its API style path""" path = path.strip('/') - os_path = self.get_os_path(name, path) + os_path = self._get_os_path(name, path) if not os.path.isdir(os_path): raise IOError('directory does not exist: %r' % os_path) info = os.stat(os_path) @@ -245,7 +252,7 @@ class FileNotebookManager(NotebookManager): path = path.strip('/') if not self.notebook_exists(name=name, path=path): raise web.HTTPError(404, u'Notebook does not exist: %s' % name) - os_path = self.get_os_path(name, path) + os_path = self._get_os_path(name, path) info = os.stat(os_path) last_modified = tz.utcfromtimestamp(info.st_mtime) created = tz.utcfromtimestamp(info.st_ctime) @@ -284,7 +291,7 @@ class FileNotebookManager(NotebookManager): self.rename_notebook(name, path, new_name, new_path) # Save the notebook file - os_path = self.get_os_path(new_name, new_path) + os_path = self._get_os_path(new_name, new_path) nb = current.to_notebook_json(model['content']) self.check_and_sign(nb, new_path, new_name) @@ -324,7 +331,7 @@ class FileNotebookManager(NotebookManager): def delete_notebook(self, name, path=''): """Delete notebook by name and path.""" path = path.strip('/') - os_path = self.get_os_path(name, path) + os_path = self._get_os_path(name, path) if not os.path.isfile(os_path): raise web.HTTPError(404, u'Notebook does not exist: %s' % os_path) @@ -346,8 +353,8 @@ class FileNotebookManager(NotebookManager): if new_name == old_name and new_path == old_path: return - new_os_path = self.get_os_path(new_name, new_path) - old_os_path = self.get_os_path(old_name, old_path) + new_os_path = self._get_os_path(new_name, new_path) + old_os_path = self._get_os_path(old_name, old_path) # Should we proceed with the move? if os.path.isfile(new_os_path): @@ -409,7 +416,7 @@ class FileNotebookManager(NotebookManager): def create_checkpoint(self, name, path=''): """Create a checkpoint from the current state of a notebook""" path = path.strip('/') - nb_path = self.get_os_path(name, path) + nb_path = self._get_os_path(name, path) # only the one checkpoint ID: checkpoint_id = u"checkpoint" cp_path = self.get_checkpoint_path(checkpoint_id, name, path) @@ -439,7 +446,7 @@ class FileNotebookManager(NotebookManager): """restore a notebook to a checkpointed state""" path = path.strip('/') self.log.info("restoring Notebook %s from checkpoint %s", name, checkpoint_id) - nb_path = self.get_os_path(name, path) + nb_path = self._get_os_path(name, path) cp_path = self.get_checkpoint_path(checkpoint_id, name, path) if not os.path.isfile(cp_path): self.log.debug("checkpoint file does not exist: %s", cp_path) diff --git a/IPython/html/services/notebooks/nbmanager.py b/IPython/html/services/notebooks/nbmanager.py index a661838..a9d397d 100644 --- a/IPython/html/services/notebooks/nbmanager.py +++ b/IPython/html/services/notebooks/nbmanager.py @@ -22,8 +22,7 @@ import os from IPython.config.configurable import LoggingConfigurable from IPython.nbformat import current, sign -from IPython.utils import py3compat -from IPython.utils.traitlets import Instance, Unicode, TraitError +from IPython.utils.traitlets import Instance, Unicode #----------------------------------------------------------------------------- # Classes @@ -31,16 +30,6 @@ from IPython.utils.traitlets import Instance, Unicode, TraitError class NotebookManager(LoggingConfigurable): - # Todo: - # The notebook_dir attribute is used to mean a couple of different things: - # 1. Where the notebooks are stored if FileNotebookManager is used. - # 2. The cwd of the kernel for a project. - # Right now we use this attribute in a number of different places and - # we are going to have to disentangle all of this. - notebook_dir = Unicode(py3compat.getcwd(), config=True, help=""" - The directory to use for notebooks. - """) - filename_ext = Unicode(u'.ipynb') notary = Instance(sign.NotebookNotary) @@ -251,19 +240,4 @@ class NotebookManager(LoggingConfigurable): if not trusted: self.log.warn("Notebook %s/%s is not trusted", path, name) self.notary.mark_cells(nb, trusted) - - def _notebook_dir_changed(self, name, old, new): - """Do a bit of validation of the notebook dir.""" - if not os.path.isabs(new): - # If we receive a non-absolute path, make it absolute. - self.notebook_dir = os.path.abspath(new) - return - if os.path.exists(new) and not os.path.isdir(new): - raise TraitError("notebook dir %r is not a directory" % new) - if not os.path.exists(new): - self.log.info("Creating notebook dir %s", new) - try: - os.mkdir(new) - except: - raise TraitError("Couldn't create notebook dir %r" % new) diff --git a/IPython/html/services/notebooks/tests/test_nbmanager.py b/IPython/html/services/notebooks/tests/test_nbmanager.py index a41f32b..fa37169 100644 --- a/IPython/html/services/notebooks/tests/test_nbmanager.py +++ b/IPython/html/services/notebooks/tests/test_nbmanager.py @@ -23,12 +23,6 @@ class TestFileNotebookManager(TestCase): fm = FileNotebookManager(notebook_dir=td) self.assertEqual(fm.notebook_dir, td) - def test_create_nb_dir(self): - with TemporaryDirectory() as td: - nbdir = os.path.join(td, 'notebooks') - fm = FileNotebookManager(notebook_dir=nbdir) - self.assertEqual(fm.notebook_dir, nbdir) - def test_missing_nb_dir(self): with TemporaryDirectory() as td: nbdir = os.path.join(td, 'notebook', 'dir', 'is', 'missing') @@ -42,20 +36,20 @@ class TestFileNotebookManager(TestCase): # full filesystem path should be returned with correct operating system # separators. with TemporaryDirectory() as td: - nbdir = os.path.join(td, 'notebooks') + nbdir = td fm = FileNotebookManager(notebook_dir=nbdir) - path = fm.get_os_path('test.ipynb', '/path/to/notebook/') + path = fm._get_os_path('test.ipynb', '/path/to/notebook/') rel_path_list = '/path/to/notebook/test.ipynb'.split('/') fs_path = os.path.join(fm.notebook_dir, *rel_path_list) self.assertEqual(path, fs_path) fm = FileNotebookManager(notebook_dir=nbdir) - path = fm.get_os_path('test.ipynb') + path = fm._get_os_path('test.ipynb') fs_path = os.path.join(fm.notebook_dir, 'test.ipynb') self.assertEqual(path, fs_path) fm = FileNotebookManager(notebook_dir=nbdir) - path = fm.get_os_path('test.ipynb', '////') + path = fm._get_os_path('test.ipynb', '////') fs_path = os.path.join(fm.notebook_dir, 'test.ipynb') self.assertEqual(path, fs_path) diff --git a/IPython/html/services/sessions/handlers.py b/IPython/html/services/sessions/handlers.py index 7ea476d..c874d9c 100644 --- a/IPython/html/services/sessions/handlers.py +++ b/IPython/html/services/sessions/handlers.py @@ -62,7 +62,7 @@ class SessionRootHandler(IPythonHandler): if sm.session_exists(name=name, path=path): model = sm.get_session(name=name, path=path) else: - kernel_id = km.start_kernel(cwd=nbm.get_os_path(path)) + kernel_id = km.start_kernel(path=path) model = sm.create_session(name=name, path=path, kernel_id=kernel_id) location = url_path_join(self.base_url, 'api', 'sessions', model['id']) self.set_header('Location', url_escape(location)) diff --git a/IPython/html/tests/test_notebookapp.py b/IPython/html/tests/test_notebookapp.py index 4422da9..408bc00 100644 --- a/IPython/html/tests/test_notebookapp.py +++ b/IPython/html/tests/test_notebookapp.py @@ -11,10 +11,16 @@ # Imports #----------------------------------------------------------------------------- +import os +from tempfile import NamedTemporaryFile + import nose.tools as nt +from IPython.utils.tempdir import TemporaryDirectory +from IPython.utils.traitlets import TraitError import IPython.testing.tools as tt from IPython.html import notebookapp +NotebookApp = notebookapp.NotebookApp #----------------------------------------------------------------------------- # Test functions @@ -25,7 +31,7 @@ def test_help_output(): tt.help_all_output_test('notebook') def test_server_info_file(): - nbapp = notebookapp.NotebookApp(profile='nbserver_file_test') + nbapp = NotebookApp(profile='nbserver_file_test') def get_servers(): return list(notebookapp.list_running_servers(profile='nbserver_file_test')) nbapp.initialize(argv=[]) @@ -38,4 +44,30 @@ def test_server_info_file(): nt.assert_equal(get_servers(), []) # The ENOENT error should be silenced. - nbapp.remove_server_info_file() \ No newline at end of file + nbapp.remove_server_info_file() + +def test_nb_dir(): + with TemporaryDirectory() as td: + app = NotebookApp(notebook_dir=td) + nt.assert_equal(app.notebook_dir, td) + +def test_no_create_nb_dir(): + with TemporaryDirectory() as td: + nbdir = os.path.join(td, 'notebooks') + app = NotebookApp() + with nt.assert_raises(TraitError): + app.notebook_dir = nbdir + +def test_missing_nb_dir(): + with TemporaryDirectory() as td: + nbdir = os.path.join(td, 'notebook', 'dir', 'is', 'missing') + app = NotebookApp() + with nt.assert_raises(TraitError): + app.notebook_dir = nbdir + +def test_invalid_nb_dir(): + with NamedTemporaryFile() as tf: + app = NotebookApp() + with nt.assert_raises(TraitError): + app.notebook_dir = tf + diff --git a/IPython/html/utils.py b/IPython/html/utils.py index 6edc1e4..52988d5 100644 --- a/IPython/html/utils.py +++ b/IPython/html/utils.py @@ -81,7 +81,7 @@ def url_unescape(path): ]) def is_hidden(abs_path, abs_root=''): - """Is a file is hidden or contained in a hidden directory. + """Is a file hidden or contained in a hidden directory? This will start with the rightmost path element and work backwards to the given root to see if a path is hidden or in a hidden directory. Hidden is @@ -112,3 +112,14 @@ def is_hidden(abs_path, abs_root=''): return False +def to_os_path(path, root=''): + """Convert an API path to a filesystem path + + If given, root will be prepended to the path. + root must be a filesystem path already. + """ + parts = path.strip('/').split('/') + parts = [p for p in parts if p != ''] # remove duplicate splits + path = os.path.join(root, *parts) + return path +