Code review changes.

Fixed handler issues and api tests from code review
from @minrk and @carreau.

IPython/html/services/kernels/tests/test_kernels_api.py

@@ -1,53 +1,54 b''
 """Test the kernels service API."""
 
 
 import os
 import sys
 import json
 
 import requests
 
+from IPython.html.utils import url_path_join
 from IPython.html.tests.launchnotebook import NotebookTestBase
 
 
 class KernelAPITest(NotebookTestBase):
     """Test the kernels web service API"""
 
     def base_url(self):
-        return super(KernelAPITest,self).base_url() + 'api/kernels'
+        return url_path_join(super(KernelAPITest,self).base_url(), 'api/kernels')
 
     def mkkernel(self):
         r = requests.post(self.base_url())
         return r.json()
 
     def test_no_kernels(self):
         """Make sure there are no kernels running at the start"""
         url = self.base_url()
         r = requests.get(url)
         self.assertEqual(r.json(), [])
 
     def test_main_kernel_handler(self):
         # POST request
         r = requests.post(self.base_url())
         data = r.json()
         assert isinstance(data, dict)
 
         # GET request
         r = requests.get(self.base_url())
         assert isinstance(r.json(), list)
         self.assertEqual(r.json()[0], data['id'])
 
     def test_kernel_handler(self):
         # GET kernel with id
         data = self.mkkernel()
         url = self.base_url() +'/' + data['id']
         r = requests.get(url)
         assert isinstance(r.json(), dict)
         self.assertIn('id', r.json())
         self.assertEqual(r.json()['id'], data['id'])
         
         # DELETE kernel with id
         r = requests.delete(url)
         self.assertEqual(r.status_code, 204)
         r = requests.get(self.base_url())
         self.assertEqual(r.json(), [])
  No newline at end of file

IPython/html/services/notebooks/filenbmanager.py

@@ -1,333 +1,361 b''
 """A notebook manager that uses the local file system for storage.
 
 Authors:
 
 * Brian Granger
 * Zach Sailer
 """
 
 #-----------------------------------------------------------------------------
 #  Copyright (C) 2011  The IPython Development Team
 #
 #  Distributed under the terms of the BSD License.  The full license is in
 #  the file COPYING, distributed as part of this software.
 #-----------------------------------------------------------------------------
 
 #-----------------------------------------------------------------------------
 # Imports
 #-----------------------------------------------------------------------------
 
 import datetime
 import io
 import os
 import glob
 import shutil
 
 from unicodedata import normalize
 
 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 import tz
 
 #-----------------------------------------------------------------------------
 # Classes
 #-----------------------------------------------------------------------------
 
 class FileNotebookManager(NotebookManager):
     
     save_script = Bool(False, config=True,
         help="""Automatically create a Python script when saving the notebook.
         
         For easier use of import, %run and %load across notebooks, a
         <notebook-name>.py script will be created next to any
         <notebook-name>.ipynb on each save.  This can also be set with the
         short `--script` flag.
         """
     )
     
     checkpoint_dir = Unicode(config=True,
         help="""The location in which to keep notebook checkpoints
         
         By default, it is notebook-dir/.ipynb_checkpoints
         """
     )
     def _checkpoint_dir_default(self):
         return os.path.join(self.notebook_dir, '.ipynb_checkpoints')
     
     def _checkpoint_dir_changed(self, name, old, new):
         """do a bit of validation of the checkpoint dir"""
         if not os.path.isabs(new):
             # If we receive a non-absolute path, make it absolute.
             abs_new = os.path.abspath(new)
             self.checkpoint_dir = abs_new
             return
         if os.path.exists(new) and not os.path.isdir(new):
             raise TraitError("checkpoint dir %r is not a directory" % new)
         if not os.path.exists(new):
             self.log.info("Creating checkpoint dir %s", new)
             try:
                 os.mkdir(new)
             except:
                 raise TraitError("Couldn't create checkpoint dir %r" % new)
     
     filename_ext = Unicode(u'.ipynb')
 
     def get_notebook_names(self, path):
         """List all notebook names in the notebook dir."""
         names = glob.glob(self.get_os_path('*'+self.filename_ext, path))
         names = [os.path.basename(name)
                  for name in names]
         return names
 
     def increment_filename(self, basename, path='/'):
         """Return a non-used filename of the form basename<int>.
         
         This searches through the filenames (basename0, basename1, ...)
         until is find one that is not already being used. It is used to
         create Untitled and Copy names that are unique.
         """
         i = 0
         while True:
             name = u'%s%i.ipynb' % (basename,i)
             os_path = self.get_os_path(name, path)
             if not os.path.isfile(os_path):
                 break
             else:
                 i = i+1
         return name
 
     def notebook_exists(self, name, path):
         """Returns a True if the notebook exists. Else, returns False.
 
         Parameters
         ----------
         name : string
             The name of the notebook you are checking.
         path : string
             The relative path to the notebook (with '/' as separator)
 
         Returns
         -------
         bool
         """
         path = self.get_os_path(name, path)
         return os.path.isfile(path)
 
     def list_notebooks(self, path):
-        """List all notebooks in the notebook dir."""
+        """Returns a list of dictionaries that are the standard model
+        for all notebooks in the relative 'path'.
+        
+        Parameters
+        ----------
+        path : str
+            the URL path that describes the relative path for the
+            listed notebooks
+        
+        Returns
+        -------
+        notebooks : list of dicts
+            a list of the notebook models without 'content'
+        """
         notebook_names = self.get_notebook_names(path)
         notebooks = []
         for name in notebook_names:
             model = self.get_notebook_model(name, path, content=False)
             notebooks.append(model)
         notebooks = sorted(notebooks, key=lambda item: item['name'])
         return notebooks
 
     def get_notebook_model(self, name, path='/', content=True):
-        """read a notebook object from a path"""
+        """ Takes a path and name for a notebook and returns it's model
+        
+        Parameters
+        ----------
+        name : str
+            the name of the notebook
+        path : str
+            the URL path that describes the relative path for
+            the notebook
+            
+        Returns
+        -------
+        model : dict
+            the notebook model. If contents=True, returns the 'contents' 
+            dict in the model as well.
+        """
         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' % name)
         info = os.stat(os_path)
         last_modified = tz.utcfromtimestamp(info.st_mtime)
         # Create the notebook model.
         model ={}
         model['name'] = name
         model['path'] = path
         model['last_modified'] = last_modified.ctime()
         if content is True:
             with open(os_path,'r') as f:
                 s = f.read()
                 try:
                     # v1 and v2 and json in the .ipynb files.
                     nb = current.reads(s, u'json')
                 except ValueError as e:
                     raise web.HTTPError(400, u"Unreadable Notebook: %s" % e)
             model['content'] = nb
         return model
 
     def save_notebook_model(self, model, name, path='/'):
         """Save the notebook model and return the model with no content."""
 
         if 'content' not in model:
             raise web.HTTPError(400, u'No notebook JSON data provided')
 
         new_path = model.get('path', path)
         new_name = model.get('name', name)
 
         if path != new_path or name != new_name:
             self.rename_notebook(name, path, new_name, new_path)
 
         # Save the notebook file
         ospath = self.get_os_path(new_name, new_path)
         nb = model['content']
         if 'name' in nb['metadata']:
             nb['metadata']['name'] = u''
         try:
             self.log.debug("Autosaving notebook %s", ospath)
             with open(ospath,'w') as f:
                 current.write(nb, f, u'json')
         except Exception as e:
             #raise web.HTTPError(400, u'Unexpected error while autosaving notebook: %s' % ospath)
             raise e
 
         # Save .py script as well
         if self.save_script:
             pypath = os.path.splitext(path)[0] + '.py'
             self.log.debug("Writing script %s", pypath)
             try:
                 with io.open(pypath, 'w', encoding='utf-8') as f:
                     current.write(model, f, u'py')
             except Exception as e:
                 raise web.HTTPError(400, u'Unexpected error while saving notebook as script: %s' % pypath)
 
         model = self.get_notebook_model(name, path, content=False)
         return model
 
     def update_notebook_model(self, model, name, path='/'):
         """Update the notebook's path and/or name"""
         new_name = model.get('name', name)
         new_path = model.get('path', path)
         if path != new_path or name != new_name:
             self.rename_notebook(name, path, new_name, new_path)
         model = self.get_notebook_model(new_name, new_path, content=False)
         return model
 
     def delete_notebook_model(self, name, path='/'):
         """Delete notebook by name and path."""
         nb_path = self.get_os_path(name, path)
         if not os.path.isfile(nb_path):
             raise web.HTTPError(404, u'Notebook does not exist: %s' % nb_path)
         
         # clear checkpoints
         for checkpoint in self.list_checkpoints(name):
             checkpoint_id = checkpoint['checkpoint_id']
             cp_path = self.get_checkpoint_path(checkpoint_id, name, path)
             self.log.debug(cp_path)
             if os.path.isfile(cp_path):
                 self.log.debug("Unlinking checkpoint %s", cp_path)
                 os.unlink(cp_path)
         
         self.log.debug("Unlinking notebook %s", nb_path)
         os.unlink(nb_path)
 
     def rename_notebook(self, old_name, old_path, new_name, new_path):
         """Rename a notebook."""
         if new_name == old_name and new_path == old_path:
             return
         
         new_full_path = self.get_os_path(new_name, new_path)
         old_full_path = self.get_os_path(old_name, old_path)
 
         # Should we proceed with the move?
         if os.path.isfile(new_full_path):
             raise web.HTTPError(409, u'Notebook with name already exists: ' % new_full_path)
         if self.save_script:
             old_pypath = os.path.splitext(old_full_path)[0] + '.py'
             new_pypath = os.path.splitext(new_full_path)[0] + '.py'
             if os.path.isfile(new_pypath):
                 raise web.HTTPError(409, u'Python script with name already exists: %s' % new_pypath)
 
         # Move the notebook file
         try:
             os.rename(old_full_path, new_full_path)
         except:
             raise web.HTTPError(400, u'Unknown error renaming notebook: %s' % old_full_path)
 
         # Move the checkpoints
         old_checkpoints = self.list_checkpoints(old_name, old_path)
         for cp in old_checkpoints:
             checkpoint_id = cp['checkpoint_id']
             old_cp_path = self.get_checkpoint_path(checkpoint_id, old_name, path)
             new_cp_path = self.get_checkpoint_path(checkpoint_id, new_name, path)
             if os.path.isfile(old_cp_path):
                 self.log.debug("Renaming checkpoint %s -> %s", old_cp_path, new_cp_path)
                 os.rename(old_cp_path, new_cp_path)
 
         # Move the .py script
         if self.save_script:
             os.rename(old_pypath, new_pypath)
   
     # Checkpoint-related utilities
     
     def get_checkpoint_path(self, checkpoint_id, name, path='/'):
         """find the path to a checkpoint"""
         filename = u"{name}-{checkpoint_id}{ext}".format(
             name=name,
             checkpoint_id=checkpoint_id,
             ext=self.filename_ext,
         )
         cp_path = os.path.join(path, self.checkpoint_dir, filename)
         return cp_path
 
     def get_checkpoint_model(self, checkpoint_id, name, path='/'):
         """construct the info dict for a given checkpoint"""
         cp_path = self.get_checkpoint_path(checkpoint_id, name, path)
         stats = os.stat(cp_path)
         last_modified = tz.utcfromtimestamp(stats.st_mtime)
         info = dict(
             checkpoint_id = checkpoint_id,
             last_modified = last_modified,
         )
         return info
         
     # public checkpoint API
     
     def create_checkpoint(self, name, path='/'):
         """Create a checkpoint from the current state of a notebook"""
         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)
         self.log.debug("creating checkpoint for notebook %s", name)
         if not os.path.exists(self.checkpoint_dir):
             os.mkdir(self.checkpoint_dir)
         shutil.copy2(nb_path, cp_path)
         
         # return the checkpoint info
         return self.get_checkpoint_model(checkpoint_id, name, path)
     
     def list_checkpoints(self, name, path='/'):
         """list the checkpoints for a given notebook
         
         This notebook manager currently only supports one checkpoint per notebook.
         """
         checkpoint_id = "checkpoint"
         path = self.get_checkpoint_path(checkpoint_id, name, path)
         if not os.path.exists(path):
             return []
         else:
             return [self.get_checkpoint_model(checkpoint_id, name, path)]
         
     
     def restore_checkpoint(self, checkpoint_id, name, path='/'):
         """restore a notebook to a checkpointed state"""
         self.log.info("restoring Notebook %s from checkpoint %s", name, checkpoint_id)
         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)
             raise web.HTTPError(404,
                 u'Notebook checkpoint does not exist: %s-%s' % (name, checkpoint_id)
             )
         # ensure notebook is readable (never restore from an unreadable notebook)
         with file(cp_path, 'r') as f:
             nb = current.read(f, u'json')
         shutil.copy2(cp_path, nb_path)
         self.log.debug("copying %s -> %s", cp_path, nb_path)
     
     def delete_checkpoint(self, checkpoint_id, name, path='/'):
         """delete a notebook's checkpoint"""
         cp_path = self.get_checkpoint_path(checkpoint_id, name, path)
         if not os.path.isfile(cp_path):
             raise web.HTTPError(404,
                 u'Notebook checkpoint does not exist: %s-%s' % (name, checkpoint_id)
             )
         self.log.debug("unlinking %s", cp_path)
         os.unlink(cp_path)
     
     def info_string(self):
         return "Serving notebooks from local directory: %s" % self.notebook_dir

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

@@ -1,112 +1,113 b''
 """Test the notebooks webservice API."""
 
 
 import os
 import sys
 import json
 from zmq.utils import jsonapi
 
 import requests
 
+from IPython.html.utils import url_path_join
 from IPython.html.tests.launchnotebook import NotebookTestBase
 
 class APITest(NotebookTestBase):
     """Test the kernels web service API"""
 
     def notebook_url(self):
-        return super(APITest,self).base_url() + 'api/notebooks'
+        return url_path_join(super(APITest,self).base_url(), 'api/notebooks')
 
     def mknb(self, name='', path='/'):
         url = self.notebook_url() + path
         return url, requests.post(url)
 
     def delnb(self, name, path='/'):
         url = self.notebook_url() + path + name
         r = requests.delete(url)
         return r.status_code
 
     def test_notebook_handler(self):
         # POST a notebook and test the dict thats returned.
         #url, nb = self.mknb()
         url = self.notebook_url()
         nb = requests.post(url+'/')
         print nb.text
         data = nb.json()
         assert isinstance(data, dict)
         self.assertIn('name', data)
         self.assertIn('path', data)
         self.assertEqual(data['name'], u'Untitled0.ipynb')
         self.assertEqual(data['path'], u'/')
 
         # GET list of notebooks in directory.
         r = requests.get(url)
         assert isinstance(r.json(), list)
         assert isinstance(r.json()[0], dict)
         
         self.delnb('Untitled0.ipynb')
         
         # GET with a notebook name.
         url, nb = self.mknb()
         data = nb.json()
         url = self.notebook_url() + '/Untitled0.ipynb'
         r = requests.get(url)
         assert isinstance(data, dict)
 
         # PATCH (rename) request.
         new_name = {'name':'test.ipynb'}
         r = requests.patch(url, data=jsonapi.dumps(new_name))
         data = r.json()
         assert isinstance(data, dict)
 
         # make sure the patch worked.
         new_url = self.notebook_url() + '/test.ipynb'
         r = requests.get(new_url)
         assert isinstance(r.json(), dict)
 
         # GET bad (old) notebook name.
         r = requests.get(url)
         self.assertEqual(r.status_code, 404)
 
         # POST notebooks to folders one and two levels down.
         os.makedirs(os.path.join(self.notebook_dir.name, 'foo'))
         os.makedirs(os.path.join(self.notebook_dir.name, 'foo','bar'))
         assert os.path.isdir(os.path.join(self.notebook_dir.name, 'foo'))
         url, nb = self.mknb(path='/foo/')
         url2, nb2 = self.mknb(path='/foo/bar/')
         data = nb.json()
         data2 = nb2.json()
         assert isinstance(data, dict)
         assert isinstance(data2, dict)
         self.assertIn('name', data)
         self.assertIn('path', data)
         self.assertEqual(data['name'], u'Untitled0.ipynb')
         self.assertEqual(data['path'], u'/foo/')
         self.assertIn('name', data2)
         self.assertIn('path', data2)
         self.assertEqual(data2['name'], u'Untitled0.ipynb')
         self.assertEqual(data2['path'], u'/foo/bar/')
 
         # GET request on notebooks one and two levels down.
         r = requests.get(url+'/Untitled0.ipynb')
         r2 = requests.get(url2+'/Untitled0.ipynb')
         assert isinstance(r.json(), dict)
         assert isinstance(r2.json(), dict)
 
         # PATCH notebooks that are one and two levels down.
         new_name = {'name': 'testfoo.ipynb'}
         r = requests.patch(url+'/Untitled0.ipynb', data=jsonapi.dumps(new_name))
         r = requests.get(url+'/testfoo.ipynb')
         data = r.json()
         assert isinstance(data, dict)
         self.assertIn('name', data)
         self.assertEqual(data['name'], 'testfoo.ipynb')
         r = requests.get(url+'/Untitled0.ipynb')
         self.assertEqual(r.status_code, 404)
         
         # DELETE notebooks
         r0 = self.delnb('test.ipynb')
         r1 = self.delnb('testfoo.ipynb', '/foo/')
         r2 = self.delnb('Untitled0.ipynb', '/foo/bar/')
         self.assertEqual(r0, 204)
         self.assertEqual(r1, 204)
         self.assertEqual(r2, 204)

IPython/html/services/sessions/tests/test_sessions_api.py

@@ -1,95 +1,95 b''
 """Test the sessions web service API."""
 
 
 import os
 import sys
 import json
 from zmq.utils import jsonapi
 
 import requests
 
+from IPython.html.utils import url_path_join
 from IPython.html.tests.launchnotebook import NotebookTestBase
 
-
 class SessionAPITest(NotebookTestBase):
     """Test the sessions web service API"""
     
     def notebook_url(self):
-        return super(SessionAPITest,self).base_url() + 'api/notebooks'
+        return url_path_join(super(SessionAPITest,self).base_url(), 'api/notebooks')
 
     def session_url(self):
         return super(SessionAPITest,self).base_url() + 'api/sessions'
 
     def mknb(self, name='', path='/'):
         url = self.notebook_url() + path
         return url, requests.post(url)
 
     def delnb(self, name, path='/'):
         url = self.notebook_url() + path + name
         r = requests.delete(url)
         return r.status_code
 
     def test_no_sessions(self):
         """Make sure there are no sessions running at the start"""
         url = self.session_url()
         r = requests.get(url)
         self.assertEqual(r.json(), [])
 
     def test_session_root_handler(self):
         # POST a session
         url, nb = self.mknb()
         notebook = nb.json()
         param = {'notebook_path': notebook['path'] + notebook['name']}
         r = requests.post(self.session_url(), params=param)
         data = r.json()
         assert isinstance(data, dict)
         self.assertIn('name', data)
         self.assertEqual(data['name'], notebook['name'])
 
         # GET sessions
         r = requests.get(self.session_url())
         assert isinstance(r.json(), list)
         assert isinstance(r.json()[0], dict)
         self.assertEqual(r.json()[0]['id'], data['id'])
 
         # Clean up
         self.delnb('Untitled0.ipynb')
         sess_url = self.session_url() +'/'+data['id']
         r = requests.delete(sess_url)
         self.assertEqual(r.status_code, 204)
 
     def test_session_handler(self):
         # Create a session
         url, nb = self.mknb()
         notebook = nb.json()
         param = {'notebook_path': notebook['path'] + notebook['name']}
         r = requests.post(self.session_url(), params=param)
         session = r.json()
 
         # GET a session
         sess_url = self.session_url() + '/' + session['id']
         r = requests.get(sess_url)
         assert isinstance(r.json(), dict)
         self.assertEqual(r.json(), session)
 
         # PATCH a session
         data = {'notebook_path': 'test.ipynb'}
         r = requests.patch(sess_url, data=jsonapi.dumps(data))
         # Patching the notebook webservice too (just for consistency)
         requests.patch(self.notebook_url() + '/Untitled0.ipynb', 
             data=jsonapi.dumps({'name':'test.ipynb'}))
         assert isinstance(r.json(), dict)
         self.assertIn('name', r.json())
         self.assertIn('id', r.json())
         self.assertEqual(r.json()['name'], 'test.ipynb')
         self.assertEqual(r.json()['id'], session['id'])
 
         # DELETE a session
         r = requests.delete(sess_url)
         self.assertEqual(r.status_code, 204)
         r = requests.get(self.session_url())
         self.assertEqual(r.json(), [])
         
         # Clean up
         r = self.delnb('test.ipynb')
         self.assertEqual(r, 204)
  No newline at end of file