Rev 2587: (robertc) Make read() errors on directories raise ReadError not IO errors. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Jul 5 01:34:02 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2587
revision-id: pqm at pqm.ubuntu.com-20070705003400-d5p03g67nxpohfox
parent: pqm at pqm.ubuntu.com-20070704204059-a1x9jomep52m9arn
parent: robertc at robertcollins.net-20070705000057-u0gjqy7oeabqn8dp
committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2007-07-05 01:34:00 +0100
message:
  (robertc) Make read() errors on directories raise ReadError not IO errors.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/smart/vfs.py            vfs.py-20061108095550-gunadhxmzkdjfeek-2
  bzrlib/tests/test_errors.py    test_errors.py-20060210110251-41aba2deddf936a8
  bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
  bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
  bzrlib/transport/ftp.py        ftp.py-20051116161804-58dc9506548c2a53
  bzrlib/transport/local.py      local_transport.py-20050711165921-9b1f142bfe480c24
  bzrlib/transport/memory.py     memory.py-20051016101338-cd008dbdf69f04fc
  bzrlib/transport/remote.py     ssh.py-20060608202016-c25gvf1ob7ypbus6-1
  bzrlib/transport/sftp.py       sftp.py-20051019050329-ab48ce71b7e32dfe
    ------------------------------------------------------------
    revno: 2052.6.5
    merged: robertc at robertcollins.net-20070705000057-u0gjqy7oeabqn8dp
    parent: robertc at robertcollins.net-20070704234240-v5z830d3eev730si
    parent: pqm at pqm.ubuntu.com-20070704204059-a1x9jomep52m9arn
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: integration
    timestamp: Thu 2007-07-05 10:00:57 +1000
    message:
      Merge with bzr.dev.
    ------------------------------------------------------------
    revno: 2052.6.4
    merged: robertc at robertcollins.net-20070704234240-v5z830d3eev730si
    parent: robertc at robertcollins.net-20070704080912-h81ahk975cf52kg0
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: transport-get-file
    timestamp: Thu 2007-07-05 09:42:40 +1000
    message:
      Tidier SFTP code (feedback from Aaron).
    ------------------------------------------------------------
    revno: 2052.6.3
    merged: robertc at robertcollins.net-20070704080912-h81ahk975cf52kg0
    parent: robertc at robertcollins.net-20070704080813-wzebx0r88fvwj5rq
    parent: pqm at pqm.ubuntu.com-20070704041011-o8aw2m812hzhz8yr
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: transport-get-file
    timestamp: Wed 2007-07-04 18:09:12 +1000
    message:
      Merge bzr.dev.
    ------------------------------------------------------------
    revno: 2052.6.2
    merged: robertc at robertcollins.net-20070704080813-wzebx0r88fvwj5rq
    parent: robertc at robertcollins.net-20060929062403-7e8b779181d8766c
    parent: pqm at pqm.ubuntu.com-20070703052458-wh36exfav0xnj9nf
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: transport-get-file
    timestamp: Wed 2007-07-04 18:08:13 +1000
    message:
      Merge bzr.dev.
    ------------------------------------------------------------
    revno: 2052.6.1
    merged: robertc at robertcollins.net-20060929062403-7e8b779181d8766c
    parent: pqm at pqm.ubuntu.com-20060928161539-3fc7d0f247fcfc5b
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: transport-get-file
    timestamp: Fri 2006-09-29 16:24:03 +1000
    message:
      ``Transport.get`` has had its interface made more clear for ease of use.
      Retrieval of a directory must now fail with either 'PathError' at open
      time, or raise 'ReadError' on a read. (Robert Collins)
=== modified file 'NEWS'
--- a/NEWS	2007-07-04 13:14:50 +0000
+++ b/NEWS	2007-07-05 00:00:57 +0000
@@ -135,6 +135,10 @@
       allows a nicer UI when hooks are running as the current hook can
       be displayed. (Robert Collins)
 
+    * ``Transport.get`` has had its interface made more clear for ease of use.
+      Retrieval of a directory must now fail with either 'PathError' at open
+      time, or raise 'ReadError' on a read. (Robert Collins)
+
     * New method ``_maybe_expand_globs`` on the ``Command`` class for 
       dealing with unexpanded glob lists - e.g. on the win32 platform. This
       was moved from ``bzrlib.add._prepare_file_list``. (Robert Collins)

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2007-06-28 16:50:06 +0000
+++ b/bzrlib/errors.py	2007-07-04 08:08:13 +0000
@@ -349,6 +349,10 @@
 
 # XXX: Should be unified with TransportError; they seem to represent the
 # same thing
+# RBC 20060929: I think that unifiying with TransportError would be a mistake
+# - this is finer than a TransportError - and more useful as such. It 
+# differentiates between 'transport has failed' and 'operation on a transport
+# has failed.'
 class PathError(BzrError):
     
     _fmt = "Generic path error: %(path)r%(extra)s)"
@@ -457,6 +461,11 @@
         PathError.__init__(self, url, extra=extra)
 
 
+class ReadError(PathError):
+    
+    _fmt = """Error reading from %(path)r."""
+
+
 class ShortReadvError(PathError):
 
     _fmt = ("readv() read %(actual)s bytes rather than %(length)s bytes"

=== modified file 'bzrlib/smart/vfs.py'
--- a/bzrlib/smart/vfs.py	2007-04-24 12:20:09 +0000
+++ b/bzrlib/smart/vfs.py	2007-07-04 08:08:13 +0000
@@ -70,7 +70,11 @@
 class GetRequest(VfsRequest):
 
     def do(self, relpath):
-        backing_bytes = self._backing_transport.get_bytes(relpath)
+        try:
+            backing_bytes = self._backing_transport.get_bytes(relpath)
+        except errors.ReadError:
+            # cannot read the file
+            return request.FailedSmartServerResponse(('ReadError', ))
         return request.SuccessfulSmartServerResponse(('ok',), backing_bytes)
 
 

=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py	2007-06-28 16:50:06 +0000
+++ b/bzrlib/tests/test_errors.py	2007-07-04 08:08:13 +0000
@@ -166,6 +166,13 @@
                              repo.bzrdir.root_transport.base,
                              str(error))
 
+    def test_read_error(self):
+        # a unicode path to check that %r is being used.
+        path = u'a path'
+        error = errors.ReadError(path)
+        self.assertEqualDiff("Error reading from u'a path'.", str(error))
+
+
     def test_bzrnewerror_is_deprecated(self):
         class DeprecatedError(errors.BzrNewError):
             pass
@@ -205,9 +212,9 @@
             str(error))
 
     def test_transport_not_possible(self):
-        e = errors.TransportNotPossible('readonly', 'original error')
-        self.assertEqual('Transport operation not possible:'
-                         ' readonly original error', str(e))
+        error = errors.TransportNotPossible('readonly', 'original error')
+        self.assertEqualDiff('Transport operation not possible:'
+                         ' readonly original error', str(error))
 
     def assertSocketConnectionError(self, expected, *args, **kwargs):
         """Check the formatting of a SocketConnectionError exception"""
@@ -360,4 +367,3 @@
         e = ErrorWithBadFormat(not_thing='x')
         self.assertStartsWith(
             str(e), 'Unprintable exception ErrorWithBadFormat')
-

=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py	2007-06-09 15:34:07 +0000
+++ b/bzrlib/tests/test_transport.py	2007-07-04 08:09:12 +0000
@@ -32,6 +32,9 @@
                            NoSuchFile,
                            PathNotChild,
                            TransportNotPossible,
+                           ConnectionError,
+                           DependencyNotPresent,
+                           ReadError,
                            UnsupportedProtocol,
                            )
 from bzrlib.tests import TestCase, TestCaseInTempDir
@@ -40,6 +43,7 @@
                               _set_protocol_handlers,
                               _get_transport_modules,
                               get_transport,
+                              LateReadError,
                               register_lazy_transport,
                               register_transport_proto,
                               _clear_protocol_handlers,
@@ -117,6 +121,16 @@
         finally:
             _set_protocol_handlers(saved_handlers)
 
+    def test_LateReadError(self):
+        """The LateReadError helper should raise on read()."""
+        a_file = LateReadError('a path')
+        try:
+            a_file.read()
+        except ReadError, error:
+            self.assertEqual('a path', error.path)
+        self.assertRaises(ReadError, a_file.read, 40)
+        a_file.close()
+
     def test__combine_paths(self):
         t = Transport('/')
         self.assertEqual('/home/sarah/project/foo',

=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2007-06-28 04:57:44 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2007-07-04 08:08:13 +0000
@@ -181,6 +181,26 @@
         self.assertListRaises(NoSuchFile, t.get_multi, ['a', 'b', 'c'])
         self.assertListRaises(NoSuchFile, t.get_multi, iter(['a', 'b', 'c']))
 
+    def test_get_directory_read_gives_ReadError(self):
+        """consistent errors for read() on a file returned by get()."""
+        t = self.get_transport()
+        if t.is_readonly():
+            self.build_tree(['a directory/'])
+        else:
+            t.mkdir('a%20directory')
+        # getting the file must either work or fail with a PathError
+        try:
+            a_file = t.get('a%20directory')
+        except (errors.PathError, errors.RedirectRequested):
+            # early failure return immediately.
+            return
+        # having got a file, read() must either work (i.e. http reading a dir listing) or
+        # fail with ReadError
+        try:
+            a_file.read()
+        except errors.ReadError:
+            pass
+
     def test_get_bytes(self):
         t = self.get_transport()
 

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-07-04 09:04:20 +0000
+++ b/bzrlib/transport/__init__.py	2007-07-05 00:00:57 +0000
@@ -214,6 +214,32 @@
                    (other.start, other.length, other.ranges))
 
 
+class LateReadError(object):
+    """A helper for transports which pretends to be a readable file.
+
+    When read() is called, errors.ReadError is raised.
+    """
+
+    def __init__(self, path):
+        self._path = path
+
+    def close(self):
+        """a no-op - do nothing."""
+
+    def _fail(self):
+        """Raise ReadError."""
+        raise errors.ReadError(self._path)
+
+    def __iter__(self):
+        self._fail()
+
+    def read(self, count=-1):
+        self._fail()
+
+    def readlines(self):
+        self._fail()
+
+
 class Transport(object):
     """This class encapsulates methods for retrieving or putting a file
     from/to a storage location.
@@ -472,6 +498,17 @@
     def get(self, relpath):
         """Get the file at the given relative path.
 
+        This may fail in a number of ways:
+         - HTTP servers may return content for a directory. (unexpected
+           content failure)
+         - FTP servers may indicate NoSuchFile for a directory.
+         - SFTP servers may give a file handle for a directory that will
+           fail on read().
+
+        For correct use of the interface, be sure to catch errors.PathError
+        when calling it and catch errors.ReadError when reading from the
+        returned object.
+
         :param relpath: The relative path to the file
         :rtype: File-like object.
         """

=== modified file 'bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py	2007-07-02 17:35:53 +0000
+++ b/bzrlib/transport/ftp.py	2007-07-04 08:08:13 +0000
@@ -559,7 +559,7 @@
 
 
 class FtpServer(Server):
-    """Common code for SFTP server facilities."""
+    """Common code for FTP server facilities."""
 
     def __init__(self):
         self._root = None

=== modified file 'bzrlib/transport/local.py'
--- a/bzrlib/transport/local.py	2007-04-05 09:35:26 +0000
+++ b/bzrlib/transport/local.py	2007-07-04 08:08:13 +0000
@@ -35,6 +35,7 @@
     symbol_versioning,
     )
 from bzrlib.trace import mutter
+from bzrlib.transport import LateReadError
 """)
 
 from bzrlib.transport import Transport, Server
@@ -141,6 +142,8 @@
             path = self._abspath(relpath)
             return open(path, 'rb')
         except (IOError, OSError),e:
+            if e.errno == errno.EISDIR:
+                return LateReadError(relpath)
             self._translate_error(e, path)
 
     def put_file(self, relpath, f, mode=None):

=== modified file 'bzrlib/transport/memory.py'
--- a/bzrlib/transport/memory.py	2007-04-23 05:03:44 +0000
+++ b/bzrlib/transport/memory.py	2007-07-04 08:08:13 +0000
@@ -29,7 +29,12 @@
 
 from bzrlib.errors import TransportError, NoSuchFile, FileExists, LockError
 from bzrlib.trace import mutter
-from bzrlib.transport import (Transport, register_transport, Server)
+from bzrlib.transport import (
+    LateReadError,
+    register_transport,
+    Server,
+    Transport,
+    )
 import bzrlib.urlutils as urlutils
 
 
@@ -121,7 +126,10 @@
         """See Transport.get()."""
         _abspath = self._abspath(relpath)
         if not _abspath in self._files:
-            raise NoSuchFile(relpath)
+            if _abspath in self._dirs:
+                return LateReadError(relpath)
+            else:
+                raise NoSuchFile(relpath)
         return StringIO(self._files[_abspath][0])
 
     def put_file(self, relpath, f, mode=None):

=== modified file 'bzrlib/transport/remote.py'
--- a/bzrlib/transport/remote.py	2007-06-27 04:33:04 +0000
+++ b/bzrlib/transport/remote.py	2007-07-04 08:08:13 +0000
@@ -389,6 +389,12 @@
                 raise UnicodeEncodeError(encoding, val, start, end, reason)
         elif what == "ReadOnlyError":
             raise errors.TransportNotPossible('readonly transport')
+        elif what == "ReadError":
+            if orig_path is not None:
+                error_path = orig_path
+            else:
+                error_path = resp[1]
+            raise errors.ReadError(error_path)
         else:
             raise errors.SmartProtocolError('unexpected smart server error: %r' % (resp,))
 

=== modified file 'bzrlib/transport/sftp.py'
--- a/bzrlib/transport/sftp.py	2007-04-16 04:24:11 +0000
+++ b/bzrlib/transport/sftp.py	2007-07-04 23:42:40 +0000
@@ -393,7 +393,8 @@
                 f.prefetch()
             return f
         except (IOError, paramiko.SSHException), e:
-            self._translate_io_exception(e, path, ': error retrieving')
+            self._translate_io_exception(e, path, ': error retrieving',
+                failure_exc=errors.ReadError)
 
     def readv(self, relpath, offsets):
         """See Transport.readv()"""
@@ -682,7 +683,7 @@
         """Create a directory at the given path."""
         self._mkdir(self._remote_path(relpath), mode=mode)
 
-    def _translate_io_exception(self, e, path, more_info='', 
+    def _translate_io_exception(self, e, path, more_info='',
                                 failure_exc=PathError):
         """Translate a paramiko or IOError into a friendlier exception.
 




More information about the bazaar-commits mailing list