Merge pull request #70 from bcdev/forman-69-rollback_fix

Fixed rollback

Author: forman

CHANGES.md

@@ -1,3 +1,12 @@
+## Version 0.5.1 (2024-02-23)
+
+* Fixed rollback for situations where writing to Zarr fails shortly after the
+  Zarr directory has been created. [#69]
+  
+  In this case the error message was
+  ```TypeError: Transaction._delete_dir() missing 1 required positional argument: 'target_path'```. 
+
+
 ## Version 0.5.0 (2024-02-19)
 
 ### Enhancements

tests/fsutil/test_transaction.py

@@ -18,6 +18,28 @@ class TransactionTest(unittest.TestCase):
     def setUp(self):
         clear_memory_fs()
 
+    def test_rollback_after_create_dir(self):
+        """See https://github.com/bcdev/zappend/issues/69"""
+        test_root = FileObj("memory://test")
+        test_root.mkdir()
+        target_dir = test_root / "target.zarr"
+        try:
+            with Transaction(target_dir, FileObj("memory://temp")) as rollback_cb:
+                try:
+                    # create target directory
+                    target_dir.mkdir()
+                    # and then suddenly fail
+                    raise OSError("core meltdown")
+                finally:
+                    # assert target directory created
+                    self.assertTrue(target_dir.exists())
+                    # notify this
+                    rollback_cb("delete_dir", "", None)
+        except OSError:
+            # assert that it correctly rolled back
+            # creating of target directory
+            self.assertFalse(target_dir.exists())
+
     def test_transaction_success(self):
         self._run_transaction_test(fail=False, rollback=True)
 
@@ -79,14 +101,14 @@ def create_test_folder(rollback_cb: Callable):
                 if rollback:
                     rollback_data = rollback_file.read(mode="rt")
                     rollback_records = [
-                        line.split()[:2] for line in rollback_data.split("\n")
+                        line.split(";")[:2] for line in rollback_data.split("\n")
                     ]
                     self.assertEqual(
                         [
                             ["replace_file", "file-1.txt"],
                             ["delete_file", "file-2.txt"],
                             ["delete_dir", "folder"],
-                            [],
+                            [""],
                         ],
                         rollback_records,
                     )

zappend/__init__.py

@@ -2,4 +2,4 @@
 # Permissions are hereby granted under the terms of the MIT License:
 # https://opensource.org/licenses/MIT.
 
-__version__ = "0.5.0"
+__version__ = "0.5.1"

zappend/fsutil/transaction.py

@@ -23,6 +23,7 @@
 
 LOCK_EXT = ".lock"
 ROLLBACK_FILE = "__rollback__.txt"
+ROLLBACK_CSV_SEP = ";"
 
 ROLLBACK_ACTIONS = "delete_dir", "delete_file", "replace_file"
 
@@ -141,8 +142,10 @@ def __exit__(self, exc_type, exc_val, exc_tb):
             rollback_txt = self._rollback_file.read(mode="r")
             rollback_records = [
                 record
-                for record in [line.split() for line in rollback_txt.split("\n")]
-                if record
+                for record in [
+                    line.split(ROLLBACK_CSV_SEP) for line in rollback_txt.split("\n")
+                ]
+                if record and record != [""]
             ]
 
             if rollback_records:
@@ -209,13 +212,14 @@ def _add_rollback_action(
             return
 
         assert hasattr(self, "_" + action)
+
         if data is not None:
             backup_id = str(uuid.uuid4())
             backup_file = self._rollback_dir.for_path(backup_id)
             backup_file.write(data)
-            rollback_entry = f"{action} {path} {backup_id}"
+            rollback_entry = ROLLBACK_CSV_SEP.join((action, path, backup_id))
         else:
-            rollback_entry = f"{action} {path}"
+            rollback_entry = ROLLBACK_CSV_SEP.join((action, path))
 
         logger.debug(f"Recording rollback record: {rollback_entry!r}")
         self._rollback_file.write(rollback_entry + "\n", mode="a")