From 9b5bfa703e233c05be118084ab728dedfe47556e Mon Sep 17 00:00:00 2001 From: Norman Fomferra Date: Fri, 23 Feb 2024 13:05:35 +0100 Subject: [PATCH] Fixed #69, preparing 0.5.1 --- CHANGES.md | 9 +++++++++ tests/fsutil/test_transaction.py | 26 ++++++++++++++++++++++++-- zappend/__init__.py | 2 +- zappend/fsutil/transaction.py | 12 ++++++++---- 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1b3b569..990ceee 100644 --- a/CHANGES.md +++ b/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 diff --git a/tests/fsutil/test_transaction.py b/tests/fsutil/test_transaction.py index 5c91f8c..375e83e 100644 --- a/tests/fsutil/test_transaction.py +++ b/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, ) diff --git a/zappend/__init__.py b/zappend/__init__.py index 1fb5458..f44f9a6 100644 --- a/zappend/__init__.py +++ b/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" diff --git a/zappend/fsutil/transaction.py b/zappend/fsutil/transaction.py index 34293f0..9474444 100644 --- a/zappend/fsutil/transaction.py +++ b/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")