Rev 2677: Review feedback. in http://people.ubuntu.com/~robertc/baz2.0/transport

Robert Collins robertc at robertcollins.net
Wed Aug 8 08:16:22 BST 2007


At http://people.ubuntu.com/~robertc/baz2.0/transport

------------------------------------------------------------
revno: 2677
revision-id: robertc at robertcollins.net-20070808071618-4e1jopgxjj6g16ug
parent: robertc at robertcollins.net-20070805081501-ipg5fapwuigozr50
committer: Robert Collins <robertc at robertcollins.net>
branch nick: transport-get-file
timestamp: Wed 2007-08-08 17:16:18 +1000
message:
  Review feedback.
modified:
  bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
  bzrlib/transport/chroot.py     chroot.py-20061011104729-0us9mgm97z378vnt-1
  bzrlib/transport/decorator.py  decorator.py-20060402223305-e913a0f25319ab42
  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
=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2007-08-05 08:15:01 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2007-08-08 07:16:18 +0000
@@ -236,10 +236,10 @@
             return
         handle = t.open_file_stream('foo')
         try:
-            handle('b')
+            handle.write('b')
             self.assertEqual('b', t.get('foo').read())
         finally:
-            t.close_file_stream('foo')
+            handle.close()
 
     def test_get_bytes_with_open_file_stream_sees_all_content(self):
         t = self.get_transport()
@@ -247,11 +247,11 @@
             return
         handle = t.open_file_stream('foo')
         try:
-            handle('b')
+            handle.write('b')
             self.assertEqual('b', t.get_bytes('foo'))
             self.assertEqual('b', t.get('foo').read())
         finally:
-            t.close_file_stream('foo')
+            handle.close()
 
     def test_put(self):
         t = self.get_transport()
@@ -664,7 +664,7 @@
         try:
             self.assertEqual('', t.get_bytes('foo'))
         finally:
-            t.close_file_stream('foo')
+            handle.close()
 
     def test_opening_a_file_stream_can_set_mode(self):
         t = self.get_transport()
@@ -675,7 +675,7 @@
             return
         def check_mode(name, mode, expected):
             handle = t.open_file_stream(name, mode=mode)
-            t.close_file_stream(name)
+            handle.close()
             self.assertTransportMode(t, name, expected)
         check_mode('mode644', 0644, 0644)
         check_mode('mode666', 0666, 0666)
@@ -1588,10 +1588,10 @@
             return
         handle = t.open_file_stream('foo')
         try:
-            handle('bcd')
+            handle.write('bcd')
             self.assertEqual([(0, 'b'), (2, 'd')], list(t.readv('foo', ((0,1), (2,1)))))
         finally:
-            t.close_file_stream('foo')
+            handle.close()
 
     def test_get_smart_medium(self):
         """All transports must either give a smart medium, or know they can't.

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-08-05 05:38:15 +0000
+++ b/bzrlib/transport/__init__.py	2007-08-08 07:16:18 +0000
@@ -257,6 +257,49 @@
         self._fail()
 
 
+class FileStream(object):
+    """Base class for FileStreams."""
+
+    def __init__(self, transport, relpath):
+        """Create a FileStream for relpath on transport."""
+        self.transport = transport
+        self.relpath = relpath
+
+    def _close(self):
+        """A hook point for subclasses that need to take action on close."""
+
+    def close(self):
+        self._close()
+        del _file_streams[self.transport.abspath(self.relpath)]
+
+
+class FileFileStream(FileStream):
+    """A file stream object returned by open_file_stream.
+    
+    This version uses a file like object to perform writes.
+    """
+
+    def __init__(self, transport, relpath, file_handle):
+        FileStream.__init__(self, transport, relpath)
+        self.file_handle = file_handle
+
+    def _close(self):
+        self.file_handle.close()
+
+    def write(self, bytes):
+        self.file_handle.write(bytes)
+
+
+class AppendBasedFileStream(FileStream):
+    """A file stream object returned by open_file_stream.
+    
+    This version uses append on a transport to perform writes.
+    """
+
+    def write(self, bytes):
+        self.transport.append_bytes(self.relpath, bytes)
+
+
 class Transport(object):
     """This class encapsulates methods for retrieving or putting a file
     from/to a storage location.
@@ -318,15 +361,6 @@
         """
         raise NotImplementedError(self.clone)
 
-    def close_file_stream(self, relpath):
-        """Close a file stream at relpath.
-
-        :raises: NoSuchFile if there is no open file stream for relpath.
-        :seealso: open_file_stream.
-        :return: None
-        """
-        raise NotImplementedError(self.close_file_stream)
-
     def ensure_base(self):
         """Ensure that the directory this transport references exists.
 
@@ -858,7 +892,10 @@
         :param relpath: The relative path to the file.
         :param mode: The mode for the newly created file, 
                      None means just use the default
-        :return: A write callback to add data to the file.
+        :return: A FileStream. FileStream objects have two methods, write() and
+            close(). There is no guarantee that data is committed to the file
+            if close() has not been called (even if get() is called on the same
+            path).
         """
         raise NotImplementedError(self.open_file_stream)
 

=== modified file 'bzrlib/transport/chroot.py'
--- a/bzrlib/transport/chroot.py	2007-08-05 05:38:15 +0000
+++ b/bzrlib/transport/chroot.py	2007-08-08 07:16:18 +0000
@@ -95,9 +95,6 @@
     def clone(self, relpath):
         return ChrootTransport(self.server, self.abspath(relpath))
 
-    def close_file_stream(self, relpath):
-        return self._call('close_file_stream', relpath)
-
     def delete(self, relpath):
         return self._call('delete', relpath)
 

=== modified file 'bzrlib/transport/decorator.py'
--- a/bzrlib/transport/decorator.py	2007-08-05 05:38:15 +0000
+++ b/bzrlib/transport/decorator.py	2007-08-08 07:16:18 +0000
@@ -75,10 +75,6 @@
         return self.__class__(
             self._get_url_prefix() + decorated_clone.base, decorated_clone)
 
-    def close_file_stream(self, relpath):
-        """See Transport.close_file_stream."""
-        return self._decorated.close_file_stream(relpath)
-
     def delete(self, relpath):
         """See Transport.delete()."""
         return self._decorated.delete(relpath)

=== modified file 'bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py	2007-08-05 02:57:45 +0000
+++ b/bzrlib/transport/ftp.py	2007-08-08 07:16:18 +0000
@@ -46,6 +46,7 @@
     )
 from bzrlib.trace import mutter, warning
 from bzrlib.transport import (
+    AppendBasedFileStream,
     _file_streams,
     Server,
     ConnectedTransport,
@@ -106,10 +107,6 @@
             self._set_connection(connection, credentials)
         return connection
 
-    def close_file_stream(self, relpath):
-        """See Transport.close_file_stream."""
-        del _file_streams[self.abspath(relpath)]
-
     def _create_connection(self, credentials=None):
         """Create a new connection with the provided credentials.
 
@@ -328,13 +325,12 @@
             self._translate_perm_error(e, abspath,
                 unknown_exc=errors.FileExists)
 
-    def open_file_stream(self, relpath):
+    def open_file_stream(self, relpath, mode=None):
         """See Transport.open_file_stream."""
-        def append_data(bytes):
-            self.append_bytes(relpath, bytes)
-        self.put_bytes(relpath, "")
-        _file_streams[self.abspath(relpath)] = append_data
-        return append_data
+        self.put_bytes(relpath, "", mode)
+        result = AppendBasedFileStream(self, relpath)
+        _file_streams[self.abspath(relpath)] = result
+        return result
 
     def recommended_page_size(self):
         """See Transport.recommended_page_size().

=== modified file 'bzrlib/transport/local.py'
--- a/bzrlib/transport/local.py	2007-08-05 05:53:53 +0000
+++ b/bzrlib/transport/local.py	2007-08-08 07:16:18 +0000
@@ -85,11 +85,6 @@
                 abspath = self.base
             return LocalTransport(abspath)
 
-    def close_file_stream(self, relpath):
-        """See Transport.close_file_stream."""
-        handle = transport._file_streams.pop(self.abspath(relpath))
-        handle.close()
-
     def _abspath(self, relative_reference):
         """Return a path for use in os calls.
 
@@ -316,7 +311,7 @@
         self.put_bytes_non_atomic(relpath, "", mode=mode)
         handle = open(self._abspath(relpath), 'wb')
         transport._file_streams[self.abspath(relpath)] = handle
-        return handle.write
+        return transport.FileFileStream(self, relpath, handle)
 
     def _get_append_file(self, relpath, mode=None):
         """Call os.open() for the given relpath"""

=== modified file 'bzrlib/transport/memory.py'
--- a/bzrlib/transport/memory.py	2007-08-05 02:57:45 +0000
+++ b/bzrlib/transport/memory.py	2007-08-08 07:16:18 +0000
@@ -36,6 +36,7 @@
     )
 from bzrlib.trace import mutter
 from bzrlib.transport import (
+    AppendBasedFileStream,
     _file_streams,
     LateReadError,
     register_transport,
@@ -90,10 +91,6 @@
         result._locks = self._locks
         return result
 
-    def close_file_stream(self, relpath):
-        """See Transport.close_file_stream."""
-        del _file_streams[self.abspath(relpath)]
-
     def abspath(self, relpath):
         """See Transport.abspath()."""
         # while a little slow, this is sufficiently fast to not matter in our
@@ -170,13 +167,12 @@
             raise FileExists(relpath)
         self._dirs[_abspath]=mode
 
-    def open_file_stream(self, relpath):
+    def open_file_stream(self, relpath, mode=None):
         """See Transport.open_file_stream."""
-        def append_data(bytes):
-            self.append_bytes(relpath, bytes)
-        self.put_bytes(relpath, "")
-        _file_streams[self.abspath(relpath)] = append_data
-        return append_data
+        self.put_bytes(relpath, "", mode)
+        result = AppendBasedFileStream(self, relpath)
+        _file_streams[self.abspath(relpath)] = result
+        return result
 
     def listable(self):
         """See Transport.listable."""

=== modified file 'bzrlib/transport/remote.py'
--- a/bzrlib/transport/remote.py	2007-08-05 02:57:45 +0000
+++ b/bzrlib/transport/remote.py	2007-08-08 07:16:18 +0000
@@ -127,10 +127,6 @@
         # No credentials
         return None, None
 
-    def close_file_stream(self, relpath):
-        """See Transport.close_file_stream."""
-        del transport._file_streams[self.abspath(relpath)]
-
     def is_readonly(self):
         """Smart server transport can do read/write file operations."""
         resp = self._call2('Transport.is_readonly')
@@ -217,13 +213,12 @@
             self._serialise_optional_mode(mode))
         self._translate_error(resp)
 
-    def open_file_stream(self, relpath):
+    def open_file_stream(self, relpath, mode=None):
         """See Transport.open_file_stream."""
-        def append_data(bytes):
-            self.append_bytes(relpath, bytes)
-        self.put_bytes(relpath, "")
-        transport._file_streams[self.abspath(relpath)] = append_data
-        return append_data
+        self.put_bytes(relpath, "", mode)
+        result = transport.AppendBasedFileStream(self, relpath)
+        transport._file_streams[self.abspath(relpath)] = result
+        return result
 
     def put_bytes(self, relpath, upload_contents, mode=None):
         # FIXME: upload_file is probably not safe for non-ascii characters -

=== modified file 'bzrlib/transport/sftp.py'
--- a/bzrlib/transport/sftp.py	2007-08-05 05:38:15 +0000
+++ b/bzrlib/transport/sftp.py	2007-08-08 07:16:18 +0000
@@ -53,6 +53,7 @@
         )
 from bzrlib.trace import mutter, warning
 from bzrlib.transport import (
+    FileFileStream,
     _file_streams,
     local,
     register_urlparse_netloc_protocol,
@@ -157,11 +158,6 @@
         super(SFTPTransport, self).__init__(base,
                                             _from_transport=_from_transport)
 
-    def close_file_stream(self, relpath):
-        """See Transport.close_file_stream."""
-        handle = _file_streams.pop(self.abspath(relpath))
-        handle.close()
-
     def _remote_path(self, relpath):
         """Return the path to be passed along the sftp protocol for relpath.
         
@@ -559,7 +555,7 @@
             self._translate_io_exception(e, abspath,
                                          ': unable to open')
         _file_streams[self.abspath(relpath)] = handle
-        return handle.write
+        return FileFileStream(self, relpath, handle)
 
     def _translate_io_exception(self, e, path, more_info='',
                                 failure_exc=PathError):



More information about the bazaar-commits mailing list