Merge pull request ipython#5116 from minrk/os_path

reorganize who knows what about paths

IPython/html/nbconvert/handlers.py

@@ -76,15 +76,12 @@ def get(self, format, path='', name=None):
         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)
 

IPython/html/notebookapp.py

@@ -88,7 +88,7 @@
 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 @@ def init_handlers(self, settings):
         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 @@ def start(self):
     '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 @@ def _mathjax_url_changed(self, name, old, new):
     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 @@ def parse_command_line(self, argv=None):
                 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 @@ def server_info(self):
                 '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):

IPython/html/services/kernels/kernelmanager.py

@@ -16,13 +16,18 @@
 # Imports
 #-----------------------------------------------------------------------------
 
+import os
+
 from tornado import web
 
 from IPython.kernel.multikernelmanager import MultiKernelManager
 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 @@ def _kernel_manager_class_default(self):
         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 @@ def _handle_kernel_died(self, kernel_id):
         """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 @@ def start_kernel(self, kernel_id=None, **kwargs):
             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)

IPython/html/services/notebooks/filenbmanager.py

@@ -18,7 +18,6 @@
 #-----------------------------------------------------------------------------
 
 import io
-import itertools
 import os
 import glob
 import shutil
@@ -28,8 +27,9 @@
 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 @@ def _checkpoint_dir_changed(self, name, old, new):
     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 @@ def path_exists(self, path):
             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 @@ def is_hidden(self, path):
         
         """
         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 @@ def get_os_path(self, name=None, path=''):
             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,21 +167,21 @@ def notebook_exists(self, name, path=''):
         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
     # no longer listed by the notebook web service.
     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 @@ def list_dirs(self, path):
     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 @@ def get_notebook(self, name, path='', content=True):
         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 @@ def save_notebook(self, model, name='', path=''):
             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 @@ def update_notebook(self, model, name, path=''):
     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 @@ def rename_notebook(self, old_name, old_path, new_name, new_path):
         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 @@ def get_checkpoint_model(self, checkpoint_id, name, path=''):
     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 @@ def restore_checkpoint(self, checkpoint_id, name, path=''):
         """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)

IPython/html/services/notebooks/nbmanager.py

@@ -22,25 +22,14 @@
 
 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
 #-----------------------------------------------------------------------------
 
 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 @@ def mark_trusted_cells(self, nb, path, name):
         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)