From cc15133f9582caf7c0e750ee5558582eecfa18ce 2013-10-17 21:09:10 From: Zachary Sailer Date: 2013-10-17 21:09:10 Subject: [PATCH] review fixes on tests, add extra kernel api test --- diff --git a/IPython/html/base/handlers.py b/IPython/html/base/handlers.py index 3b37833..fcd79b3 100644 --- a/IPython/html/base/handlers.py +++ b/IPython/html/base/handlers.py @@ -218,10 +218,6 @@ class IPythonHandler(AuthenticatedHandler): return self.settings['session_manager'] @property - def content_manager(self): - return self.settings['content_manager'] - - @property def project(self): return self.notebook_manager.notebook_dir diff --git a/IPython/html/services/kernels/tests/test_kernels_api.py b/IPython/html/services/kernels/tests/test_kernels_api.py index e144085..1322878 100644 --- a/IPython/html/services/kernels/tests/test_kernels_api.py +++ b/IPython/html/services/kernels/tests/test_kernels_api.py @@ -16,11 +16,15 @@ class KernelAPITest(NotebookTestBase): def base_url(self): return 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) - assert r.json() == [] + self.assertEqual(r.json(), []) def test_main_kernel_handler(self): # POST request @@ -33,4 +37,17 @@ class KernelAPITest(NotebookTestBase): assert isinstance(r.json(), list) self.assertEqual(r.json()[0], data['id']) - \ No newline at end of file + 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 diff --git a/IPython/html/services/notebooks/filenbmanager.py b/IPython/html/services/notebooks/filenbmanager.py index 2391e25..326bf2f 100644 --- a/IPython/html/services/notebooks/filenbmanager.py +++ b/IPython/html/services/notebooks/filenbmanager.py @@ -91,7 +91,7 @@ class FileNotebookManager(NotebookManager): notebooks.append(model) return notebooks - def change_notebook(self, data, notebook_name, notebook_path='/'): + def update_notebook(self, data, notebook_name, notebook_path='/'): """Changes notebook""" changes = data.keys() for change in changes: diff --git a/IPython/html/services/notebooks/handlers.py b/IPython/html/services/notebooks/handlers.py index be64528..40ae7e3 100644 --- a/IPython/html/services/notebooks/handlers.py +++ b/IPython/html/services/notebooks/handlers.py @@ -16,127 +16,117 @@ Authors: # Imports #----------------------------------------------------------------------------- -from tornado import web +import json -from zmq.utils import jsonapi +from tornado import web +from ...utils import url_path_join from IPython.utils.jsonutil import date_default -from ...base.handlers import IPythonHandler +from ...base.handlers import IPythonHandler, json_errors #----------------------------------------------------------------------------- # Notebook web service handlers #----------------------------------------------------------------------------- -class NotebookRootHandler(IPythonHandler): - - @web.authenticated - def get(self): - """get returns a list of notebooks from the location - where the server was started.""" - nbm = self.notebook_manager - notebooks = nbm.list_notebooks("/") - self.finish(jsonapi.dumps(notebooks)) - - @web.authenticated - def post(self): - """post creates a notebooks in the directory where the - server was started""" - nbm = self.notebook_manager - self.log.info(nbm.notebook_dir) - body = self.request.body.strip() - format = self.get_argument('format', default='json') - name = self.get_argument('name', default=None) - if body: - fname = nbm.save_new_notebook(body, notebook_path='/', name=name, format=format) - else: - fname = nbm.new_notebook(notebook_path='/') - self.set_header('Location', nbm.notebook_dir + fname) - model = nbm.notebook_model(fname) - self.set_header('Location', '{0}api/notebooks/{1}'.format(self.base_project_url, fname)) - self.finish(jsonapi.dumps(model)) - class NotebookHandler(IPythonHandler): - SUPPORTED_METHODS = ('GET', 'PUT', 'PATCH', 'POST','DELETE') + SUPPORTED_METHODS = (u'GET', u'PUT', u'PATCH', u'POST', u'DELETE') + + def notebook_location(self, name, path): + """Return the full URL location of a notebook based. + + Parameters + ---------- + name : unicode + The name of the notebook like "foo.ipynb". + path : unicode + The URL path of the notebook. + """ + return url_path_join(self.base_project_url, u'/api/notebooks', path, name) @web.authenticated + @json_errors def get(self, notebook_path): """get checks if a notebook is not named, an returns a list of notebooks in the notebook path given. If a name is given, return the notebook representation""" nbm = self.notebook_manager + # path will have leading and trailing slashes, such as '/foo/bar/' name, path = nbm.named_notebook_path(notebook_path) # Check to see if a notebook name was given if name is None: # List notebooks in 'notebook_path' notebooks = nbm.list_notebooks(path) - self.finish(jsonapi.dumps(notebooks)) + self.finish(json.dumps(notebooks, default=date_default)) else: # get and return notebook representation - format = self.get_argument('format', default='json') - download = self.get_argument('download', default='False') - model = nbm.notebook_model(name, path) - last_mod, representation, name = nbm.get_notebook(name, path, format) - self.set_header('Last-Modified', last_mod) - - if download == 'True': - if format == u'json': - self.set_header('Content-Type', 'application/json') - self.set_header('Content-Disposition','attachment; filename="%s.ipynb"' % name) - self.finish(representation) - elif format == u'py': - self.set_header('Content-Type', 'application/x-python') - self.set_header('Content-Disposition','attachment; filename="%s.py"' % name) - self.finish(representation) - else: - self.finish(jsonapi.dumps(model)) + model = nbm.get_notebook_model(name, path) + self.set_header(u'Last-Modified', model[u'last_modified']) + self.finish(json.dumps(model, default=date_default)) @web.authenticated + # @json_errors def patch(self, notebook_path): """patch is currently used strictly for notebook renaming. Changes the notebook name to the name given in data.""" nbm = self.notebook_manager + # path will have leading and trailing slashes, such as '/foo/bar/' name, path = nbm.named_notebook_path(notebook_path) - data = jsonapi.loads(self.request.body) - model = nbm.change_notebook(data, name, path) - self.finish(jsonapi.dumps(model)) + if name is None: + raise web.HTTPError(400, u'Notebook name missing') + model = self.get_json_body() + if model is None: + raise web.HTTPError(400, u'JSON body missing') + model = nbm.update_notebook_model(model, name, path) + if model[u'name'] != name or model[u'path'] != path: + self.set_status(301) + location = self.notebook_location(model[u'name'], model[u'path']) + self.set_header(u'Location', location) + self.set_header(u'Last-Modified', model[u'last_modified']) + self.finish(json.dumps(model, default=date_default)) @web.authenticated - def post(self,notebook_path): + @json_errors + def post(self, notebook_path): """Create a new notebook in the location given by 'notebook_path'.""" nbm = self.notebook_manager - fname, path = nbm.named_notebook_path(notebook_path) - body = self.request.body.strip() - format = self.get_argument('format', default='json') - name = self.get_argument('name', default=None) - if body: - fname = nbm.save_new_notebook(body, notebook_path=path, name=name, format=format) - else: - fname = nbm.new_notebook(notebook_path=path) - self.set_header('Location', nbm.notebook_dir + path + fname) - model = nbm.notebook_model(fname, path) - self.finish(jsonapi.dumps(model)) + # path will have leading and trailing slashes, such as '/foo/bar/' + name, path = nbm.named_notebook_path(notebook_path) + model = self.get_json_body() + if name is not None: + raise web.HTTPError(400, 'No name can be provided when POSTing a new notebook.') + model = nbm.create_notebook_model(model, path) + location = nbm.notebook_dir + model[u'path'] + model[u'name'] + location = self.notebook_location(model[u'name'], model[u'path']) + self.set_header(u'Location', location) + self.set_header(u'Last-Modified', model[u'last_modified']) + self.set_status(201) + self.finish(json.dumps(model, default=date_default)) @web.authenticated + @json_errors def put(self, notebook_path): """saves the notebook in the location given by 'notebook_path'.""" nbm = self.notebook_manager + # path will have leading and trailing slashes, such as '/foo/bar/' name, path = nbm.named_notebook_path(notebook_path) - format = self.get_argument('format', default='json') - nbm.save_notebook(self.request.body, notebook_path=path, name=name, format=format) - model = nbm.notebook_model(name, path) - self.set_status(204) - self.finish(jsonapi.dumps(model)) + model = self.get_json_body() + if model is None: + raise web.HTTPError(400, u'JSON body missing') + nbm.save_notebook_model(model, name, path) + self.finish(json.dumps(model, default=date_default)) @web.authenticated + @json_errors def delete(self, notebook_path): - """delete rmoves the notebook in the given notebook path""" + """delete the notebook in the given notebook path""" nbm = self.notebook_manager + # path will have leading and trailing slashes, such as '/foo/bar/' name, path = nbm.named_notebook_path(notebook_path) - nbm.delete_notebook(name, path) + nbm.delete_notebook_model(name, path) self.set_status(204) self.finish() @@ -146,29 +136,28 @@ class NotebookCheckpointsHandler(IPythonHandler): SUPPORTED_METHODS = ('GET', 'POST') @web.authenticated + @json_errors def get(self, notebook_path): """get lists checkpoints for a notebook""" nbm = self.notebook_manager + # path will have leading and trailing slashes, such as '/foo/bar/' name, path = nbm.named_notebook_path(notebook_path) checkpoints = nbm.list_checkpoints(name, path) - data = jsonapi.dumps(checkpoints, default=date_default) + data = json.dumps(checkpoints, default=date_default) self.finish(data) @web.authenticated + @json_errors def post(self, notebook_path): """post creates a new checkpoint""" nbm = self.notebook_manager name, path = nbm.named_notebook_path(notebook_path) + # path will have leading and trailing slashes, such as '/foo/bar/' checkpoint = nbm.create_checkpoint(name, path) - data = jsonapi.dumps(checkpoint, default=date_default) - if path == None: - self.set_header('Location', '{0}notebooks/{1}/checkpoints/{2}'.format( - self.base_project_url, name, checkpoint['checkpoint_id'] - )) - else: - self.set_header('Location', '{0}notebooks/{1}/{2}/checkpoints/{3}'.format( - self.base_project_url, path, name, checkpoint['checkpoint_id'] - )) + data = json.dumps(checkpoint, default=date_default) + location = url_path_join(self.base_project_url, u'/api/notebooks', + path, name, '/checkpoints', checkpoint[u'checkpoint_id']) + self.set_header(u'Location', location) self.finish(data) @@ -177,20 +166,24 @@ class ModifyNotebookCheckpointsHandler(IPythonHandler): SUPPORTED_METHODS = ('POST', 'DELETE') @web.authenticated + @json_errors def post(self, notebook_path, checkpoint_id): """post restores a notebook from a checkpoint""" nbm = self.notebook_manager + # path will have leading and trailing slashes, such as '/foo/bar/' name, path = nbm.named_notebook_path(notebook_path) - nbm.restore_checkpoint(name, checkpoint_id, path) + nbm.restore_checkpoint(checkpoint_id, name, path) self.set_status(204) self.finish() @web.authenticated + @json_errors def delete(self, notebook_path, checkpoint_id): """delete clears a checkpoint for a given notebook""" nbm = self.notebook_manager + # path will have leading and trailing slashes, such as '/foo/bar/' name, path = nbm.named_notebook_path(notebook_path) - nbm.delete_checkpoint(name, checkpoint_id, path) + nbm.delete_checkpoint(checkpoint_id, name, path) self.set_status(204) self.finish() @@ -199,19 +192,15 @@ class ModifyNotebookCheckpointsHandler(IPythonHandler): #----------------------------------------------------------------------------- -_notebook_path_regex = r"(?P.+)" +_notebook_path_regex = r"(?P.*)" _checkpoint_id_regex = r"(?P[\w-]+)" default_handlers = [ - (r"api/notebooks/%s/checkpoints" % _notebook_path_regex, NotebookCheckpointsHandler), - (r"api/notebooks/%s/checkpoints/%s" % (_notebook_path_regex, _checkpoint_id_regex), + (r"/api/notebooks/%s/checkpoints" % _notebook_path_regex, NotebookCheckpointsHandler), + (r"/api/notebooks/%s/checkpoints/%s" % (_notebook_path_regex, _checkpoint_id_regex), ModifyNotebookCheckpointsHandler), - (r"api/notebooks/%s/" % _notebook_path_regex, NotebookHandler), - (r"api/notebooks/%s" % _notebook_path_regex, NotebookHandler), - (r"api/notebooks/", NotebookRootHandler), - (r"api/notebooks", NotebookRootHandler), + (r"/api/notebooks%s" % _notebook_path_regex, NotebookHandler), ] - diff --git a/IPython/html/services/notebooks/nbmanager.py b/IPython/html/services/notebooks/nbmanager.py index d610aad..4ac2598 100644 --- a/IPython/html/services/notebooks/nbmanager.py +++ b/IPython/html/services/notebooks/nbmanager.py @@ -155,12 +155,12 @@ class NotebookManager(LoggingConfigurable): """ raise NotImplementedError('must be implemented in a subclass') - def notebook_model(self, notebook_name, notebook_path='/', content=True): + def notebook_model(self, name, path='/', content=True): """ Creates the standard notebook model """ - last_modified, contents = self.read_notebook_object(notebook_name, notebook_path) - model = {"name": notebook_name, - "path": notebook_path, - "last_modified (UTC)": last_modified.ctime()} + last_modified, contents = self.read_notebook_model(name, path) + model = {"name": name, + "path": path, + "last_modified": last_modified.ctime()} if content is True: model['content'] = contents return model @@ -180,10 +180,22 @@ class NotebookManager(LoggingConfigurable): name = nb.metadata.get('name', 'notebook') return last_mod, representation, name - def read_notebook_object(self, notebook_name, notebook_path='/'): + def read_notebook_model(self, notebook_name, notebook_path='/'): """Get the object representation of a notebook by notebook_id.""" raise NotImplementedError('must be implemented in a subclass') + def save_notebook(self, model, name=None, path='/'): + """Save the Notebook""" + if name is None: + name = self.increment_filename('Untitled', path) + if 'content' not in model: + metadata = current.new_metadata(name=name) + nb = current.new_notebook(metadata=metadata) + else: + nb = model['content'] + self.write_notebook_object() + + def save_new_notebook(self, data, notebook_path='/', name=None, format=u'json'): """Save a new notebook and return its name. @@ -208,7 +220,7 @@ class NotebookManager(LoggingConfigurable): notebook_name = self.write_notebook_object(nb, notebook_path=notebook_path) return notebook_name - def save_notebook(self, data, notebook_path='/', name=None, new_name=None, format=u'json'): + def save_notebook(self, data, notebook_path='/', name=None, format=u'json'): """Save an existing notebook by notebook_name.""" if format not in self.allowed_formats: raise web.HTTPError(415, u'Invalid notebook format: %s' % format) @@ -222,7 +234,7 @@ class NotebookManager(LoggingConfigurable): nb.metadata.name = name self.write_notebook_object(nb, name, notebook_path, new_name) - def write_notebook_object(self, nb, notebook_name='/', notebook_path='/', new_name=None): + def write_notebook_model(self, model): """Write a notebook object and return its notebook_name. If notebook_name is None, this method should create a new notebook_name. diff --git a/IPython/html/services/notebooks/tests/test_notebooks_api.py b/IPython/html/services/notebooks/tests/test_notebooks_api.py index f11cb3f..6d44a93 100644 --- a/IPython/html/services/notebooks/tests/test_notebooks_api.py +++ b/IPython/html/services/notebooks/tests/test_notebooks_api.py @@ -25,15 +25,15 @@ class APITest(NotebookTestBase): r = requests.delete(url) return r.status_code - def test_notebook_root_handler(self): + 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) data = nb.json() assert isinstance(data, dict) - assert data.has_key("name") - assert data.has_key("path") + self.assertIn('name', data) + self.assertIn('path', data) self.assertEqual(data['name'], u'Untitled0.ipynb') self.assertEqual(data['path'], u'/') @@ -43,8 +43,7 @@ class APITest(NotebookTestBase): assert isinstance(r.json()[0], dict) self.delnb('Untitled0.ipynb') - - def test_notebook_handler(self): + # GET with a notebook name. url, nb = self.mknb() data = nb.json() @@ -79,18 +78,18 @@ class APITest(NotebookTestBase): data2 = nb2.json() assert isinstance(data, dict) assert isinstance(data2, dict) - assert data.has_key("name") - assert data.has_key("path") + self.assertIn('name', data) + self.assertIn('path', data) self.assertEqual(data['name'], u'Untitled0.ipynb') self.assertEqual(data['path'], u'/foo/') - assert data2.has_key("name") - assert data2.has_key("path") + 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') + r = requests.get(url+'/Untitled0.ipynb') + r2 = requests.get(url2+'/Untitled0.ipynb') assert isinstance(r.json(), dict) self.assertEqual(r.json(), data) assert isinstance(r2.json(), dict) @@ -98,13 +97,13 @@ class APITest(NotebookTestBase): # 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') + r = requests.patch(url+'/Untitled0.ipynb', data=jsonapi.dumps(new_name)) + r = requests.get(url+'/testfoo.ipynb') data = r.json() assert isinstance(data, dict) - assert data.has_key('name') + self.assertIn('name', data) self.assertEqual(data['name'], 'testfoo.ipynb') - r = requests.get(url+'Untitled0.ipynb') + r = requests.get(url+'/Untitled0.ipynb') self.assertEqual(r.status_code, 404) # DELETE notebooks diff --git a/IPython/html/services/sessions/tests/test_sessions_api.py b/IPython/html/services/sessions/tests/test_sessions_api.py index b781387..eca503a 100644 --- a/IPython/html/services/sessions/tests/test_sessions_api.py +++ b/IPython/html/services/sessions/tests/test_sessions_api.py @@ -43,7 +43,7 @@ class SessionAPITest(NotebookTestBase): r = requests.post(self.session_url(), params=param) data = r.json() assert isinstance(data, dict) - assert data.has_key('name') + self.assertIn('name', data) self.assertEqual(data['name'], notebook['name']) # GET sessions @@ -79,8 +79,8 @@ class SessionAPITest(NotebookTestBase): requests.patch(self.notebook_url() + '/Untitled0.ipynb', data=jsonapi.dumps({'name':'test.ipynb'})) assert isinstance(r.json(), dict) - assert r.json().has_key('name') - assert r.json().has_key('id') + self.assertIn('name', r.json()) + self.assertIn('id', r.json()) self.assertEqual(r.json()['name'], 'test.ipynb') self.assertEqual(r.json()['id'], session['id']) @@ -88,8 +88,8 @@ class SessionAPITest(NotebookTestBase): r = requests.delete(sess_url) self.assertEqual(r.status_code, 204) r = requests.get(self.session_url()) - assert r.json() == [] + self.assertEqual(r.json(), []) # Clean up r = self.delnb('test.ipynb') - assert r == 204 \ No newline at end of file + self.assertEqual(r, 204) \ No newline at end of file diff --git a/IPython/html/tests/launchnotebook.py b/IPython/html/tests/launchnotebook.py index f93e209..0cc195c 100644 --- a/IPython/html/tests/launchnotebook.py +++ b/IPython/html/tests/launchnotebook.py @@ -2,6 +2,7 @@ import sys import time +import requests from subprocess import Popen, PIPE from unittest import TestCase @@ -17,6 +18,26 @@ class NotebookTestBase(TestCase): port = 1234 + def wait_till_alive(self): + url = 'http://localhost:%i/' % self.port + while True: + time.sleep(.1) + try: + r = requests.get(url + 'api/notebooks') + break + except requests.exceptions.ConnectionError: + pass + + def wait_till_dead(self): + url = 'http://localhost:%i/' % self.port + while True: + time.sleep(.1) + try: + r = requests.get(url + 'api/notebooks') + continue + except requests.exceptions.ConnectionError: + break + def setUp(self): self.ipython_dir = TemporaryDirectory() self.notebook_dir = TemporaryDirectory() @@ -27,15 +48,16 @@ class NotebookTestBase(TestCase): '--no-browser', '--ipython-dir=%s' % self.ipython_dir.name, '--notebook-dir=%s' % self.notebook_dir.name - ] + ] self.notebook = Popen(notebook_args, stdout=PIPE, stderr=PIPE) - time.sleep(3.0) + self.wait_till_alive() + #time.sleep(3.0) def tearDown(self): self.notebook.terminate() self.ipython_dir.cleanup() self.notebook_dir.cleanup() - time.sleep(3.0) - + self.wait_till_dead() + def base_url(self): return 'http://localhost:%i/' % self.port