[Zodb-checkins] SVN: ZODB/trunk/src/Z - updated the blob todo list

[Christian Theune]

Log message for revision 76513:
   - updated the blob todo list
   - some cleanups
   - re-introduced a way to ask for a rename() operation but fall back to copy
     if rename doesn't work
   - using the rename_or_copy for blobs where reasonable
   - fixed a test that was trying to test a failing consumeBlob but where the
     wrong exception was tested for
  

Changed:
  U   ZODB/trunk/src/ZEO/ClientStorage.py
  U   ZODB/trunk/src/ZODB/Connection.py
  U   ZODB/trunk/src/ZODB/blob.py
  U   ZODB/trunk/src/ZODB/tests/blob_consume.txt
  U   ZODB/trunk/src/ZODB/utils.py

-=-
Modified: ZODB/trunk/src/ZEO/ClientStorage.py
===================================================================
--- ZODB/trunk/src/ZEO/ClientStorage.py	2007-06-08 16:39:04 UTC (rev 76512)
+++ ZODB/trunk/src/ZEO/ClientStorage.py	2007-06-08 17:09:51 UTC (rev 76513)
@@ -920,14 +920,14 @@
             i = 0
             while 1:
                 try:
-                    os.rename(filename, target + str(i))
+                    utils.rename_or_copy(filename, target + str(i))
                 except OSError:
                     i += 1
                 else:
                     break
             target += str(i)
         else:
-            os.rename(filename, target)
+            utils.rename_or_copy(filename, target)
         # Now tell the server where we put it
         self._server.storeBlobShared(
             oid, serial, data,

Modified: ZODB/trunk/src/ZODB/Connection.py
===================================================================
--- ZODB/trunk/src/ZODB/Connection.py	2007-06-08 16:39:04 UTC (rev 76512)
+++ ZODB/trunk/src/ZODB/Connection.py	2007-06-08 17:09:51 UTC (rev 76513)
@@ -1242,7 +1242,7 @@
     def storeBlob(self, oid, serial, data, blobfilename, version,
                   transaction):
         serial = self.store(oid, serial, data, version, transaction)
-        assert isinstance(serial, str) # XXX in theory serials could be 
+        assert isinstance(serial, str) # XXX in theory serials could be
                                        # something else
 
         targetpath = self._getBlobPath(oid)
@@ -1250,7 +1250,7 @@
             os.makedirs(targetpath, 0700)
 
         targetname = self._getCleanFilename(oid, serial)
-        os.rename(blobfilename, targetname)
+        utils.rename_or_copy(blobfilename, targetname)
 
     def loadBlob(self, oid, serial):
         """Return the filename where the blob file can be found.

Modified: ZODB/trunk/src/ZODB/blob.py
===================================================================
--- ZODB/trunk/src/ZODB/blob.py	2007-06-08 16:39:04 UTC (rev 76512)
+++ ZODB/trunk/src/ZODB/blob.py	2007-06-08 17:09:51 UTC (rev 76513)
@@ -16,7 +16,6 @@
 
 import base64
 import logging
-import logging
 import os
 import shutil
 import sys
@@ -195,7 +194,7 @@
             os.unlink(target)
 
         try:
-            os.rename(filename, target)
+            utils.rename_or_copy(filename, target)
         except:
             # Recover from the failed consumption: First remove the file, it
             # might exist and mark the pointer to the uncommitted file.
@@ -254,7 +253,7 @@
     def __init__(self, name, mode, blob):
         super(BlobFile, self).__init__(name, mode+'b')
         self.blob = blob
-            
+
     def close(self):
         self.blob.closed(self)
         file.close(self)
@@ -411,7 +410,7 @@
                     os.makedirs(targetpath, 0700)
 
                 targetname = self.fshelper.getBlobFilename(oid, serial)
-                os.rename(blobfilename, targetname)
+                utils.rename_or_copy(blobfilename, targetname)
 
                 # XXX if oid already in there, something is really hosed.
                 # The underlying storage should have complained anyway
@@ -611,9 +610,6 @@
 #       cache. (Idea: LRU principle via mstat access time and a
 #       size-based threshold) currently).
 # 
-#       Make blobs able to efficiently consume existing files from the
-#       filesystem
-# 
 # Savepoint support
 # =================
 # 

Modified: ZODB/trunk/src/ZODB/tests/blob_consume.txt
===================================================================
--- ZODB/trunk/src/ZODB/tests/blob_consume.txt	2007-06-08 16:39:04 UTC (rev 76512)
+++ ZODB/trunk/src/ZODB/tests/blob_consume.txt	2007-06-08 17:09:51 UTC (rev 76513)
@@ -64,29 +64,53 @@
 There are some edge cases what happens when the link() operation
 fails. We simulate this in different states:
 
-Case 1: We don't have uncommitted data, but the link operation fails. The
-exception will be re-raised and the target file will not exist::
+Case 1: We don't have uncommitted data, but the link operation fails. We fall
+back to try a copy/remove operation that is successfull::
 
     >>> open('to_import', 'wb').write('Some data.')
 
-    >>> def failing_rename(self, filename):
-    ...   raise Exception("I can't link.")
+    >>> def failing_rename(f1, f2):
+    ...   import exceptions
+    ...   if f1 == 'to_import':
+    ...       raise exceptions.OSError("I can't link.")
+    ...   os_rename(f1, f2)
 
     >>> blob = Blob()
     >>> os_rename = os.rename
     >>> os.rename = failing_rename
     >>> blob.consumeFile('to_import')
+
+The blob did not have data before, so it shouldn't have data now::
+
+    >>> blob.open('r').read()
+    'Some data.'
+
+Case 2: We don't have uncommitted data and both the link operation and the
+copy fail. The exception will be re-raised and the target file will not
+exist::
+
+    >>> blob = Blob()
+    >>> import ZODB.utils
+    >>> utils_cp = ZODB.utils.cp
+
+    >>> def failing_copy(f1, f2):
+    ...     import exceptions
+    ...     raise exceptions.OSError("I can't copy.")
+
+    >>> ZODB.utils.cp = failing_copy
+    >>> open('to_import', 'wb').write('Some data.')
+    >>> blob.consumeFile('to_import')
     Traceback (most recent call last):
-    Exception: I can't link.
+    OSError: I can't copy.
 
 The blob did not have data before, so it shouldn't have data now::
 
     >>> blob.open('r').read()
     ''
 
-Case 2: We thave uncommitted data, but the link operation fails. The
-exception will be re-raised and the target file will exist with the previous
-uncomitted data::
+Case 3: We have uncommitted data, but the link and the copy operations fail.
+The exception will be re-raised and the target file will exist with the
+previous uncomitted data::
 
     >>> blob = Blob()
     >>> blob_writing = blob.open('w')
@@ -95,7 +119,7 @@
 
     >>> blob.consumeFile('to_import')
     Traceback (most recent call last):
-    Exception: I can't link.
+    OSError: I can't copy.
 
 The blob did existed before and had uncommitted data, this shouldn't have
 changed::
@@ -104,3 +128,4 @@
     'Uncommitted data'
 
     >>> os.rename = os_rename
+    >>> ZODB.utils.cp = utils_cp

Modified: ZODB/trunk/src/ZODB/utils.py
===================================================================
--- ZODB/trunk/src/ZODB/utils.py	2007-06-08 16:39:04 UTC (rev 76512)
+++ ZODB/trunk/src/ZODB/utils.py	2007-06-08 17:09:51 UTC (rev 76513)
@@ -12,6 +12,7 @@
 #
 ##############################################################################
 
+import exceptions
 import sys
 import time
 import struct
@@ -84,6 +85,7 @@
 
 U64 = u64
 
+
 def cp(f1, f2, length=None):
     """Copy all data from one file to another.
     
@@ -112,6 +114,22 @@
         write(data)
         length -= len(data)
 
+
+def rename_or_copy(f1, f2):
+    """Try to rename f1 to f2, fallback to copy.
+
+    Under certain conditions a rename might not work, e.g. because the target
+    directory is on a different partition. In this case we try to copy the
+    data and remove the old file afterwards.
+
+    """
+    try:
+        os.rename(f1, f2)
+    except exceptions.OSError:
+        cp(open(f1, 'rb'), open(f2, 'wb'))
+        os.unlink(f1)
+
+
 def newTimeStamp(old=None,
                  TimeStamp=TimeStamp,
                  time=time.time, gmtime=time.gmtime):