Merge pull request #5116 from minrk/os_path

reorganize who knows what about paths

IPython/html/nbconvert/handlers.py

@@ -76,15 +76,12 b' 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)
 

IPython/html/notebookapp.py

@@ -88,7 +88,7 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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):

IPython/html/services/kernels/kernelmanager.py

@@ -16,6 +16,8 b' Authors:'
 # Imports
 #-----------------------------------------------------------------------------
 
+import os
+
 from tornado import web
 
 from IPython.kernel.multikernelmanager import MultiKernelManager
@@ -23,6 +25,9 b' 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 b' 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 b' 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 b' 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)

IPython/html/services/notebooks/filenbmanager.py

@@ -18,7 +18,6 b' Authors:'
 #-----------------------------------------------------------------------------
 
 import io
-import itertools
 import os
 import glob
 import shutil
@@ -28,8 +27,9 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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)

IPython/html/services/notebooks/nbmanager.py

@@ -22,8 +22,7 b' 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 b' 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 b' 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)
 

IPython/html/services/notebooks/tests/test_nbmanager.py

@@ -23,12 +23,6 b' 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 b' 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)
 

IPython/html/services/sessions/handlers.py

@@ -62,7 +62,7 b' 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))

IPython/html/tests/test_notebookapp.py

@@ -11,10 +11,16 b''
 # 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 b' 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 b' 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
+

IPython/html/utils.py

@@ -81,7 +81,7 b' 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 b" 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
+