Minor cleanups and addition of comments, plus fleshing out of TODO.txt.

branches/ctheune-blobsupport/src/ZODB/Blobs/BlobStorage.py

@@ -32,6 +32,7 @@ class BlobStorage(ProxyBase):
     implements(IBlobStorage)
 
     __slots__ = ('base_directory', 'dirty_oids')
+    # XXX CM: what is the purpose of specifying __slots__ here?
 
     def __new__(self, base_directory, storage):
         return ProxyBase.__new__(self, storage)
@@ -42,7 +43,8 @@ class BlobStorage(ProxyBase):
         self.base_directory = base_directory
         self.dirty_oids = []
      
-    def storeBlob(self, oid, oldserial, data, blobfilename, version, transaction):
+    def storeBlob(self, oid, oldserial, data, blobfilename, version,
+                  transaction):
         """Stores data that has a BLOB attached."""
         serial = self.store(oid, oldserial, data, version, transaction)
         assert isinstance(serial, str) # XXX in theory serials could be 
@@ -55,19 +57,7 @@ class BlobStorage(ProxyBase):
                 os.makedirs(targetpath, 0700)
                               
             targetname = self._getCleanFilename(oid, serial)
-            try:
-                os.rename(blobfilename, targetname)
-            except OSError:
-                # XXX CM: I don't think this is a good idea; maybe just fail
-                # here instead of doing a brute force copy?  This is awfully
-                # expensive and people won't know it's happening without
-                # at least a warning.
-                target = file(targetname, "wb")
-                source = file(blobfilename, "rb")
-                utils.cp(blobfile, target)
-                target.close()
-                source.close()
-                os.unlink(blobfilename)
+            utils.best_rename(blobfilename, targetname)
 
             # XXX if oid already in there, something is really hosed.
             # The underlying storage should have complained anyway

branches/ctheune-blobsupport/src/ZODB/Blobs/TODO.txt

@@ -2,4 +2,24 @@
 Tests
 -----
 
+- Test packing.
+
+- Test import/export.
+
+- Test conflict behavior.
+
+- Test shared client usage of blob cache dir.
+
+- More ZEO tests.
+
+Features
+--------
+
+- Ensure we detect and play a failed txn involving blobs forward or
+  backward at startup.
+
 - Importing backward compatible ZEXP files (no \0BLOBSTART) used
+
+- More options for blob directory structures (e.g. dirstorage's
+  bushy/chunky/lawn/flat).
+

branches/ctheune-blobsupport/src/ZODB/Blobs/interfaces.py

@@ -29,6 +29,3 @@ class IBlobStorage(Interface):
         Raises POSKeyError if the blobfile cannot be found.
         """
 
-    def getBlobDirectory():
-        """
-        """

branches/ctheune-blobsupport/src/ZODB/Connection.py

@@ -351,6 +351,9 @@ class Connection(ExportImport, object):
                                             obj._p_blob_uncommitted,
                                             self._version, transaction)
                 obj._p_invalidate()
+                # XXX CM: do we invalidate the object here in order to
+                # ensure that that the next attribute access of its
+                # name unghostify it?
             else:
                 s = self._storage.store(oid, serial, p, self._version,
                                         transaction)

branches/ctheune-blobsupport/src/ZODB/utils.py

@@ -329,14 +329,22 @@ def mktemp():
     return filename
 
 def best_rename(sourcename, targetname):
+    """ Try to rename via os.rename, but if we can't (for instance, if the
+    source and target are on separate partitions/volumes), fall back to copying
+    the file and unlinking the original. """
     try:
         os.rename(sourcename, targetname)
     except OSError:
-        # XXX This creates a race condition for un-locked return above
+        # XXX CM: I don't think this is a good idea; maybe just fail
+        # here instead of doing a brute force copy?  This is awfully
+        # expensive and people won't know it's happening without
+        # at least a warning.  It also increases the possibility of a race
+        # condition: both the source and target filenames exist at the
+        # same time.
         source = open(sourcename, "rb")
         target = open(targetname, "wb")
         while True:
-            chunk = source.read(4096)
+            chunk = source.read(1<<16)
             if not chunk:
                 break
             target.write(chunk)