updates per review

- clarified docstrings and errors
- still more notebook/file renames
- configurable untitled names
- copy_from can be full path
- fix running, upload, new-tab behaviors in dashboard

Yay, review!
MinRK 12 years ago
parent d4fe081fe5
commit 3c26b079f0

@ -54,7 +54,7 @@ class FileContentsManager(ContentsManager):
self.log.debug("copystat on %s failed", dest, exc_info=True)
def _get_os_path(self, name=None, path=''):
"""Given a filename and a URL path, return its file system
"""Given a filename and API path, return its file system
path.
Parameters
@ -62,8 +62,7 @@ class FileContentsManager(ContentsManager):
name : string
A filename
path : string
The relative URL path (with '/' as separator) to the named
file.
The relative API path to the named file.
Returns
-------
@ -77,6 +76,8 @@ class FileContentsManager(ContentsManager):
def path_exists(self, path):
"""Does the API-style path refer to an extant directory?
API-style wrapper for os.path.isdir
Parameters
----------
path : string
@ -114,6 +115,8 @@ class FileContentsManager(ContentsManager):
def file_exists(self, name, path=''):
"""Returns True if the file exists, else returns False.
API-style wrapper for os.path.isfile
Parameters
----------
name : string
@ -123,7 +126,8 @@ class FileContentsManager(ContentsManager):
Returns
-------
bool
exists : bool
Whether the file exists.
"""
path = path.strip('/')
nbpath = self._get_os_path(name, path=path)
@ -132,6 +136,8 @@ class FileContentsManager(ContentsManager):
def exists(self, name=None, path=''):
"""Returns True if the path [and name] exists, else returns False.
API-style wrapper for os.path.exists
Parameters
----------
name : string
@ -141,7 +147,8 @@ class FileContentsManager(ContentsManager):
Returns
-------
bool
exists : bool
Whether the target exists.
"""
path = path.strip('/')
os_path = self._get_os_path(name, path=path)
@ -246,7 +253,7 @@ class FileContentsManager(ContentsManager):
name : str
the name of the target
path : str
the URL path that describes the relative path for the target
the API path that describes the relative path for the target
Returns
-------
@ -344,7 +351,11 @@ class FileContentsManager(ContentsManager):
return model
def update(self, model, name, path=''):
"""Update the file's path and/or name"""
"""Update the file's path and/or name
For use in PATCH requests, to enable renaming a file without
re-uploading its contents. Only used for renaming at the moment.
"""
path = path.strip('/')
new_name = model.get('name', name)
new_path = model.get('path', path).strip('/')
@ -393,7 +404,7 @@ class FileContentsManager(ContentsManager):
# Should we proceed with the move?
if os.path.isfile(new_os_path):
raise web.HTTPError(409, u'Notebook with name already exists: %s' % new_os_path)
raise web.HTTPError(409, u'File with name already exists: %s' % new_os_path)
# Move the file
try:

@ -54,16 +54,16 @@ class ContentsHandler(IPythonHandler):
@web.authenticated
@json_errors
def get(self, path='', name=None):
"""Return a file or list of files.
"""Return a model for a file or directory.
* GET with path and no filename lists files in a directory
* GET with path and filename returns file contents model
A directory model contains a list of models (without content)
of the files and directories it contains.
"""
path = path or ''
model = self.contents_manager.get_model(name=name, path=path)
if model['type'] == 'directory':
# group listing by type, then by name (case-insensitive)
# FIXME: front-ends shouldn't rely on this sorting
# FIXME: sorting should be done in the frontends
model['content'].sort(key=sort_key)
self._finish_model(model, location=False)
@ -81,22 +81,22 @@ class ContentsHandler(IPythonHandler):
self._finish_model(model)
def _copy(self, copy_from, path, copy_to=None):
"""Copy a file in path, optionally specifying the new name.
Only support copying within the same directory.
"""Copy a file, optionally specifying the new name.
"""
self.log.info(u"Copying from %s/%s to %s/%s",
path, copy_from,
path, copy_to or '',
)
self.log.info(u"Copying {copy_from} to {path}/{copy_to}".format(
copy_from=copy_from,
path=path,
copy_to=copy_to or '',
))
model = self.contents_manager.copy(copy_from, copy_to, path)
self.set_status(201)
self._finish_model(model)
def _upload(self, model, path, name=None):
"""Upload a file
"""Handle upload of a new file
If name specified, create it in path/name.
If name specified, create it in path/name,
otherwise create a new untitled file in path.
"""
self.log.info(u"Uploading file to %s/%s", path, name or '')
if name:
@ -151,7 +151,7 @@ class ContentsHandler(IPythonHandler):
cm = self.contents_manager
if cm.file_exists(path):
raise web.HTTPError(400, "Only POST to directories. Use PUT for full names.")
raise web.HTTPError(400, "Cannot POST to existing files, use PUT instead.")
if not cm.path_exists(path):
raise web.HTTPError(404, "No such directory: %s" % path)
@ -184,11 +184,17 @@ class ContentsHandler(IPythonHandler):
Save notebook at ``path/Name.ipynb``. Notebook structure is specified
in `content` key of JSON request body. If content is not specified,
create a new empty notebook.
PUT /api/contents/path/Name.ipynb?copy=OtherNotebook.ipynb
PUT /api/contents/path/Name.ipynb
with JSON body::
{
"copy_from" : "[path/to/]OtherNotebook.ipynb"
}
Copy OtherNotebook to Name
"""
if name is None:
raise web.HTTPError(400, "Only PUT to full names. Use POST for directories.")
raise web.HTTPError(400, "name must be specified with PUT.")
model = self.get_json_body()
if model:

@ -15,6 +15,31 @@ from IPython.utils.traitlets import Instance, Unicode, List
class ContentsManager(LoggingConfigurable):
"""Base class for serving files and directories.
This serves any text or binary file,
as well as directories,
with special handling for JSON notebook documents.
Most APIs take a path argument,
which is always an API-style unicode path,
and always refers to a directory.
- unicode, not url-escaped
- '/'-separated
- leading and trailing '/' will be stripped
- if unspecified, path defaults to '',
indicating the root path.
name is also unicode, and refers to a specfic target:
- unicode, not url-escaped
- must not contain '/'
- It refers to an individual filename
- It may refer to a directory name,
in the case of listing or creating directories.
"""
notary = Instance(sign.NotebookNotary)
def _notary_default(self):
@ -27,12 +52,26 @@ class ContentsManager(LoggingConfigurable):
Glob patterns to hide in file and directory listings.
""")
untitled_notebook = Unicode("Untitled", config=True,
help="The base name used when creating untitled notebooks."
)
untitled_file = Unicode("untitled", config=True,
help="The base name used when creating untitled files."
)
untitled_directory = Unicode("Untitled Folder", config=True,
help="The base name used when creating untitled directories."
)
# ContentsManager API part 1: methods that must be
# implemented in subclasses.
def path_exists(self, path):
"""Does the API-style path (directory) actually exist?
Like os.path.isdir
Override this method in subclasses.
Parameters
@ -58,14 +97,18 @@ class ContentsManager(LoggingConfigurable):
Returns
-------
exists : bool
hidden : bool
Whether the path is hidden.
"""
raise NotImplementedError
def file_exists(self, name, path=''):
"""Returns a True if the file exists. Else, returns False.
"""Does a file exist at the given name and path?
Like os.path.isfile
Override this method in subclasses.
Parameters
----------
@ -76,20 +119,29 @@ class ContentsManager(LoggingConfigurable):
Returns
-------
bool
exists : bool
Whether the file exists.
"""
raise NotImplementedError('must be implemented in a subclass')
def list(self, path=''):
"""Return a list of contents dicts without content.
def exists(self, name, path=''):
"""Does a file or directory exist at the given name and path?
This returns a list of dicts
Like os.path.exists
This list of dicts should be sorted by name::
Parameters
----------
name : string
The name of the file you are checking.
path : string
The relative path to the file's directory (with '/' as separator)
data = sorted(data, key=lambda item: item['name'])
Returns
-------
exists : bool
Whether the target exists.
"""
raise NotImplementedError('must be implemented in a subclass')
return self.file_exists(name, path) or self.path_exists("%s/%s" % (path, name))
def get_model(self, name, path='', content=True):
"""Get the model of a file or directory with or without content."""
@ -100,7 +152,11 @@ class ContentsManager(LoggingConfigurable):
raise NotImplementedError('must be implemented in a subclass')
def update(self, model, name, path=''):
"""Update the file or directory and return the model with no content."""
"""Update the file or directory and return the model with no content.
For use in PATCH requests, to enable renaming a file without
re-uploading its contents. Only used for renaming at the moment.
"""
raise NotImplementedError('must be implemented in a subclass')
def delete(self, name, path=''):
@ -126,12 +182,12 @@ class ContentsManager(LoggingConfigurable):
"""delete a checkpoint for a file"""
raise NotImplementedError("must be implemented in a subclass")
def info_string(self):
return "Serving notebooks"
# ContentsManager API part 2: methods that have useable default
# implementations, but can be overridden in subclasses.
def info_string(self):
return "Serving contents"
def get_kernel_path(self, name, path='', model=None):
""" Return the path to start kernel in """
return path
@ -144,7 +200,7 @@ class ContentsManager(LoggingConfigurable):
filename : unicode
The name of a file, including extension
path : unicode
The URL path of the target's directory
The API path of the target's directory
Returns
-------
@ -176,7 +232,15 @@ class ContentsManager(LoggingConfigurable):
model['type'] = 'file'
model['format'] = 'text'
if 'name' not in model:
model['name'] = self.increment_filename('Untitled' + ext, path)
if model['type'] == 'directory':
untitled = self.untitled_directory
elif model['type'] == 'notebook':
untitled = self.untitled_notebook
elif model['type'] == 'file':
untitled = self.untitled_file
else:
raise HTTPError(400, "Unexpected model type: %r" % model['type'])
model['name'] = self.increment_filename(untitled + ext, path)
model['path'] = path
model = self.save(model, model['name'], model['path'])
@ -186,9 +250,16 @@ class ContentsManager(LoggingConfigurable):
"""Copy an existing file and return its new model.
If to_name not specified, increment `from_name-Copy#.ext`.
copy_from can be a full path to a file,
or just a base name. If a base name, `path` is used.
"""
path = path.strip('/')
model = self.get_model(from_name, path)
if '/' in from_name:
from_path, from_name = from_name.rsplit('/', 1)
else:
from_path = path
model = self.get_model(from_name, from_path)
if model['type'] == 'directory':
raise HTTPError(400, "Can't copy directories")
if not to_name:
@ -196,6 +267,7 @@ class ContentsManager(LoggingConfigurable):
copy_name = u'{0}-Copy{1}'.format(base, ext)
to_name = self.increment_filename(copy_name, path)
model['name'] = to_name
model['path'] = path
model = self.save(model, to_name, path)
return model
@ -218,7 +290,7 @@ class ContentsManager(LoggingConfigurable):
self.notary.mark_cells(nb, True)
self.save(model, name, path)
def check_and_sign(self, nb, name, path=''):
def check_and_sign(self, nb, name='', path=''):
"""Check for trusted cells, and sign the notebook.
Called as a part of saving notebooks.
@ -226,18 +298,18 @@ class ContentsManager(LoggingConfigurable):
Parameters
----------
nb : dict
The notebook structure
The notebook object (in nbformat.current format)
name : string
The filename of the notebook
The filename of the notebook (for logging)
path : string
The notebook's directory
The notebook's directory (for logging)
"""
if self.notary.check_cells(nb):
self.notary.sign(nb)
else:
self.log.warn("Saving untrusted notebook %s/%s", path, name)
def mark_trusted_cells(self, nb, name, path=''):
def mark_trusted_cells(self, nb, name='', path=''):
"""Mark cells as trusted if the notebook signature matches.
Called as a part of loading notebooks.
@ -245,11 +317,11 @@ class ContentsManager(LoggingConfigurable):
Parameters
----------
nb : dict
The notebook structure
The notebook object (in nbformat.current format)
name : string
The filename of the notebook
The filename of the notebook (for logging)
path : string
The notebook's directory
The notebook's directory (for logging)
"""
trusted = self.notary.check_signature(nb)
if not trusted:

@ -22,8 +22,6 @@ from IPython.utils import py3compat
from IPython.utils.data import uniq_stable
# TODO: Remove this after we create the contents web service and directories are
# no longer listed by the notebook web service.
def notebooks_only(dir_model):
return [nb for nb in dir_model['content'] if nb['type']=='notebook']
@ -279,9 +277,9 @@ class APITest(NotebookTestBase):
def test_create_untitled_txt(self):
resp = self.api.create_untitled(path='foo/bar', ext='.txt')
self._check_created(resp, 'Untitled0.txt', 'foo/bar', type='file')
self._check_created(resp, 'untitled0.txt', 'foo/bar', type='file')
resp = self.api.read(path='foo/bar', name='Untitled0.txt')
resp = self.api.read(path='foo/bar', name='untitled0.txt')
model = resp.json()
self.assertEqual(model['type'], 'file')
self.assertEqual(model['format'], 'text')
@ -363,6 +361,10 @@ class APITest(NotebookTestBase):
resp = self.api.copy(u'ç d.ipynb', u'cøpy.ipynb', path=u'å b')
self._check_created(resp, u'cøpy.ipynb', u'å b')
def test_copy_path(self):
resp = self.api.copy(u'foo/a.ipynb', u'cøpyfoo.ipynb', path=u'å b')
self._check_created(resp, u'cøpyfoo.ipynb', u'å b')
def test_copy_dir_400(self):
# can't copy directories
with assert_http_error(400):

@ -19,7 +19,7 @@ define([
// base_url: string
// notebook_path: string
notebooklist.NotebookList.call(this, selector, $.extend({
element_name: 'running'},
element_name: 'running'},
options));
};
@ -28,13 +28,20 @@ define([
KernelList.prototype.sessions_loaded = function (d) {
this.sessions = d;
this.clear_list();
var item;
for (var path in d) {
item = this.new_notebook_item(-1);
this.add_link('', path, item);
this.add_shutdown_button(item, this.sessions[path]);
var item, path_name;
for (path_name in d) {
if (!d.hasOwnProperty(path_name)) {
// nothing is safe in javascript
continue;
}
item = this.new_item(-1);
this.add_link({
name: path_name,
path: '',
type: 'notebook',
}, item);
this.add_shutdown_button(item, this.sessions[path_name]);
}
$('#running_list_header').toggle($.isEmptyObject(d));
};

@ -80,7 +80,7 @@ define([
var name_and_ext = utils.splitext(f.name);
var file_ext = name_and_ext[1];
if (file_ext === '.ipynb') {
var item = that.new_notebook_item(0);
var item = that.new_item(0);
item.addClass('new-file');
that.add_name_input(f.name, item);
// Store the notebook item in the reader so we can use it later
@ -236,7 +236,7 @@ define([
var icon = NotebookList.icons[model.type];
var uri_prefix = NotebookList.uri_prefixes[model.type];
item.find(".item_icon").addClass(icon).addClass('icon-fixed-width');
item.find("a.item_link")
var link = item.find("a.item_link")
.attr('href',
utils.url_join_encode(
this.base_url,
@ -245,6 +245,11 @@ define([
name
)
);
// directory nav doesn't open new tabs
// files, notebooks do
if (model.type !== "directory") {
link.attr('target','_blank');
}
var path_name = utils.url_path_join(path, name);
if (model.type == 'file') {
this.add_delete_button(item);
@ -362,7 +367,10 @@ define([
var nbdata = item.data('nbdata');
var content_type = 'application/json';
var model = {
path: path,
name: nbname,
content : JSON.parse(nbdata),
type : 'notebook'
};
var settings = {
processData : false,
@ -372,7 +380,7 @@ define([
data : JSON.stringify(model),
headers : {'Content-Type': content_type},
success : function (data, status, xhr) {
that.add_link(path, nbname, item);
that.add_link(model, item);
that.add_delete_button(item);
},
error : utils.log_ajax_error,

Loading…
Cancel
Save