From: Brian Warner Date: Mon, 27 Oct 2008 20:15:25 +0000 (-0700) Subject: dirnode lookup: use distinct NoSuchChildError instead of the generic KeyError when... X-Git-Url: https://git.rkrishnan.org/listings/reliability?a=commitdiff_plain;h=fca158e83aab54ad350293d6a367dbde54b5a399;p=tahoe-lafs%2Ftahoe-lafs.git dirnode lookup: use distinct NoSuchChildError instead of the generic KeyError when a child can't be found --- diff --git a/src/allmydata/dirnode.py b/src/allmydata/dirnode.py index e87acf33..3271f611 100644 --- a/src/allmydata/dirnode.py +++ b/src/allmydata/dirnode.py @@ -8,7 +8,7 @@ from allmydata.mutable.common import NotMutableError from allmydata.mutable.node import MutableFileNode from allmydata.interfaces import IMutableFileNode, IDirectoryNode,\ IURI, IFileNode, IMutableFileURI, IFilesystemNode, \ - ExistingChildError, ICheckable, IDeepCheckable + ExistingChildError, NoSuchChildError, ICheckable, IDeepCheckable from allmydata.checker_results import DeepCheckResults, \ DeepCheckAndRepairResults from allmydata.monitor import Monitor @@ -28,7 +28,7 @@ class Deleter: children = self.node._unpack_contents(old_contents) if self.name not in children: if self.must_exist: - raise KeyError(self.name) + raise NoSuchChildError(self.name) self.old_child = None return None self.old_child, metadata = children[self.name] @@ -44,6 +44,8 @@ class MetadataSetter: def modify(self, old_contents): children = self.node._unpack_contents(old_contents) + if self.name not in children: + raise NoSuchChildError(self.name) children[self.name] = (children[self.name][0], self.metadata) new_contents = self.node._pack_contents(children) return new_contents @@ -246,13 +248,13 @@ class NewDirectoryNode: def _get(self, children, name): child = children.get(name) if child is None: - raise KeyError(name) + raise NoSuchChildError(name) return child[0] def _get_with_metadata(self, children, name): child = children.get(name) if child is None: - raise KeyError(name) + raise NoSuchChildError(name) return child def get(self, name): diff --git a/src/allmydata/ftpd.py b/src/allmydata/ftpd.py index 413d3d6c..07ae38fe 100644 --- a/src/allmydata/ftpd.py +++ b/src/allmydata/ftpd.py @@ -9,7 +9,8 @@ from twisted.protocols import ftp from twisted.cred import error, portal, checkers, credentials from twisted.web.client import getPage -from allmydata.interfaces import IDirectoryNode, ExistingChildError +from allmydata.interfaces import IDirectoryNode, ExistingChildError, \ + NoSuchChildError from allmydata.immutable.download import ConsumerAdapter from allmydata.immutable.upload import FileHandle from allmydata.util import base32 @@ -86,7 +87,7 @@ class Handler: return defer.succeed(node) d = node.get(path[0]) def _maybe_create(f): - f.trap(KeyError) + f.trap(NoSuchChildError) return node.create_empty_directory(path[0]) d.addErrback(_maybe_create) d.addCallback(self._get_or_create_directories, path[1:]) @@ -159,7 +160,7 @@ class Handler: return d def _convert_error(self, f): - if f.check(KeyError): + if f.check(NoSuchChildError): childname = f.value.args[0].encode("utf-8") msg = "'%s' doesn't exist" % childname raise ftp.FileNotFoundError(msg) diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index 8623d676..f6becf7b 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -653,6 +653,9 @@ class ExistingChildError(Exception): """A directory node was asked to add or replace a child that already exists, and overwrite= was set to False.""" +class NoSuchChildError(Exception): + """A directory node was asked to fetch a child which does not exist.""" + class IDirectoryNode(IMutableFilesystemNode): """I represent a name-to-child mapping, holding the tahoe equivalent of a directory. All child names are unicode strings, and all children are some @@ -691,13 +694,15 @@ class IDirectoryNode(IMutableFilesystemNode): def get(name): """I return a Deferred that fires with a specific named child node, either an IFileNode or an IDirectoryNode. The child name must be a - unicode string.""" + unicode string. I raise NoSuchChildError if I do not have a child by + that name.""" def get_metadata_for(name): """I return a Deferred that fires with the metadata dictionary for a specific named child node. This metadata is stored in the *edge*, not in the child, so it is attached to the parent dirnode rather than the - child dir-or-file-node. The child name must be a unicode string.""" + child dir-or-file-node. The child name must be a unicode string. I + raise NoSuchChildError if I do not have a child by that name.""" def set_metadata_for(name, metadata): """I replace any existing metadata for the named child with the new @@ -705,14 +710,15 @@ class IDirectoryNode(IMutableFilesystemNode): stored in the *edge*, not in the child, so it is attached to the parent dirnode rather than the child dir-or-file-node. I return a Deferred (that fires with this dirnode) when the operation is - complete.""" + complete. I raise NoSuchChildError if I do not have a child by that + name.""" def get_child_at_path(path): """Transform a child path into an IDirectoryNode or IFileNode. I perform a recursive series of 'get' operations to find the named descendant node. I return a Deferred that fires with the node, or - errbacks with IndexError if the node could not be found. + errbacks with NoSuchChildError if the node could not be found. The path can be either a single string (slash-separated) or a list of path-name elements. All elements must be unicode strings. @@ -792,7 +798,8 @@ class IDirectoryNode(IMutableFilesystemNode): def delete(name): """I remove the child at the specific name. I return a Deferred that fires when the operation finishes. The child name must be a unicode - string.""" + string. I raise NoSuchChildError if I do not have a child by that + name.""" def create_empty_directory(name, overwrite=True): """I create and attach an empty directory at the given name. The @@ -805,7 +812,8 @@ class IDirectoryNode(IMutableFilesystemNode): is referenced by name. On the new parent, the child will live under 'new_child_name', which defaults to 'current_child_name'. TODO: what should we do about metadata? I return a Deferred that fires when the - operation finishes. The child name must be a unicode string.""" + operation finishes. The child name must be a unicode string. I raise + NoSuchChildError if I do not have a child by that name.""" def build_manifest(): """Return a Monitor. The Monitor's results will be a list of (path, diff --git a/src/allmydata/test/test_dirnode.py b/src/allmydata/test/test_dirnode.py index 3b7693b0..9fa9fd28 100644 --- a/src/allmydata/test/test_dirnode.py +++ b/src/allmydata/test/test_dirnode.py @@ -7,7 +7,8 @@ from allmydata import uri, dirnode from allmydata.immutable import upload from allmydata.interfaces import IURI, IClient, IMutableFileNode, \ INewDirectoryURI, IReadonlyNewDirectoryURI, IFileNode, \ - ExistingChildError, IDeepCheckResults, IDeepCheckAndRepairResults + ExistingChildError, NoSuchChildError, \ + IDeepCheckResults, IDeepCheckAndRepairResults from allmydata.util import hashutil, testutil from allmydata.monitor import Monitor from allmydata.test.common import make_chk_file_uri, make_mutable_file_uri, \ @@ -104,7 +105,7 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin): self.failUnless(u"child" in children) d.addCallback(_check1) d.addCallback(lambda res: - self.shouldFail(KeyError, "get bogus", None, + self.shouldFail(NoSuchChildError, "get bogus", None, dn.get, u"bogus")) def _corrupt(res): filenode = dn._node @@ -381,8 +382,8 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin): ["ctime", "mtime"])) d.addCallback(lambda res: - self.shouldFail(KeyError, "gcamap-no", - "'nope'", + self.shouldFail(NoSuchChildError, "gcamap-no", + "nope", n.get_child_and_metadata_at_path, u"subdir/nope")) d.addCallback(lambda res: diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py index 2080b381..42837841 100644 --- a/src/allmydata/test/test_system.py +++ b/src/allmydata/test/test_system.py @@ -15,7 +15,7 @@ from allmydata.util import log, base32 from allmydata.scripts import runner from allmydata.interfaces import IDirectoryNode, IFileNode, IFileURI, \ ICheckerResults, ICheckAndRepairResults, IDeepCheckResults, \ - IDeepCheckAndRepairResults + IDeepCheckAndRepairResults, NoSuchChildError from allmydata.monitor import Monitor, OperationCancelledError from allmydata.mutable.common import NotMutableError from allmydata.mutable import layout as mutable_layout @@ -900,7 +900,7 @@ class SystemTest(SystemTestMixin, unittest.TestCase): d1.addCallback(lambda res: self.shouldFail2(NotMutableError, "set_uri(nope)", None, dirnode.set_uri, u"hopeless", self.uri)) - d1.addCallback(lambda res: self.shouldFail2(KeyError, "get(missing)", "'missing'", dirnode.get, u"missing")) + d1.addCallback(lambda res: self.shouldFail2(NoSuchChildError, "get(missing)", "missing", dirnode.get, u"missing")) personal = self._personal_node d1.addCallback(lambda res: self.shouldFail2(NotMutableError, "mv from readonly", None, dirnode.move_child_to, u"mydata992", personal, u"nope")) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 0612b87c..207dc53c 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -14,7 +14,7 @@ from foolscap.eventual import fireEventually from allmydata.util import base32 from allmydata.uri import from_string_dirnode from allmydata.interfaces import IDirectoryNode, IFileNode, IMutableFileNode, \ - ExistingChildError + ExistingChildError, NoSuchChildError from allmydata.monitor import Monitor from allmydata.web.common import text_plain, WebError, \ IClient, IOpHandleTable, NeedOperationHandleError, \ @@ -72,7 +72,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): t = get_arg(req, "t", "").strip() if isinstance(node_or_failure, Failure): f = node_or_failure - f.trap(KeyError) + f.trap(NoSuchChildError) # No child by this name. What should we do about it? if DEBUG: print "no child", name if DEBUG: print "postpath", req.postpath @@ -235,7 +235,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): return defer.succeed(node) d = node.get(path[0]) def _maybe_create(f): - f.trap(KeyError) + f.trap(NoSuchChildError) return node.create_empty_directory(path[0]) d.addErrback(_maybe_create) d.addCallback(self._get_or_create_directories, path[1:]) @@ -268,7 +268,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): def _maybe_got_node(node_or_failure): if isinstance(node_or_failure, Failure): f = node_or_failure - f.trap(KeyError) + f.trap(NoSuchChildError) # create a placeholder which will see POST t=upload return PlaceHolderNodeHandler(self.node, name) else: