Răsfoiți Sursa

Document "after" command hooks running in case of error and make sure that happens in case of "before" hook error (#790).

Dan Helfman 3 luni în urmă
părinte
comite
624a7de622

+ 3 - 0
NEWS

@@ -2,6 +2,9 @@
  * #790, #821: Deprecate all "before_*", "after_*" and "on_error" command hooks in favor of more
  * #790, #821: Deprecate all "before_*", "after_*" and "on_error" command hooks in favor of more
    flexible "commands:". See the documentation for more information:
    flexible "commands:". See the documentation for more information:
    https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/
    https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/
+ * #790: BREAKING: Run a configured "after" command hook even if an error occurs first. This
+   allows you to perform cleanup steps that correspond to "before" preparation commands—even when
+   something goes wrong.
  * #836: Add a custom command option for the SQLite hook.
  * #836: Add a custom command option for the SQLite hook.
  * #1010: When using Borg 2, don't pass the "--stats" flag to "borg prune".
  * #1010: When using Borg 2, don't pass the "--stats" flag to "borg prune".
  * #1020: Document a database use case involving a temporary database client container:
  * #1020: Document a database use case involving a temporary database client container:

+ 4 - 0
borgmatic/hooks/command.py

@@ -193,6 +193,10 @@ class Before_after_hooks:
             if considered_soft_failure(error):
             if considered_soft_failure(error):
                 return
                 return
 
 
+            # Trigger the after hook manually, since raising here will prevent it from being run
+            # otherwise.
+            self.__exit__(None, None, None)
+
             raise ValueError(f'Error running before {self.before_after} hook: {error}')
             raise ValueError(f'Error running before {self.before_after} hook: {error}')
 
 
     def __exit__(self, exception_type, exception, traceback):
     def __exit__(self, exception_type, exception, traceback):

+ 5 - 0
docs/how-to/add-preparation-and-cleanup-steps-to-backups.md

@@ -66,6 +66,11 @@ Each command in the `commands:` list has the following options:
  * `when`: Only trigger the hook when borgmatic is run with particular actions (`create`, `prune`, etc.) listed here. Defaults to running for all actions.
  * `when`: Only trigger the hook when borgmatic is run with particular actions (`create`, `prune`, etc.) listed here. Defaults to running for all actions.
  * `run`: List of one or more shell commands or scripts to run when this command hook is triggered.
  * `run`: List of one or more shell commands or scripts to run when this command hook is triggered.
 
 
+An `after` command hook runs even if an error occurs in the corresponding `before` hook or between
+those two hooks. This allows you to perform cleanup steps that correspond to `before` preparation
+commands—even when something goes wrong. This is a departure from the way that the deprecated
+`after_*` hooks worked.
+
 There's also another command hook that works a little differently:
 There's also another command hook that works a little differently:
 
 
 ```yaml
 ```yaml

+ 3 - 3
tests/unit/hooks/test_command.py

@@ -439,7 +439,7 @@ def test_before_after_hooks_calls_command_hooks():
         pass
         pass
 
 
 
 
-def test_before_after_hooks_with_before_error_raises_and_skips_after_hook():
+def test_before_after_hooks_with_before_error_runs_after_hook_and_raises():
     commands = [
     commands = [
         {'before': 'repository', 'run': ['foo', 'bar']},
         {'before': 'repository', 'run': ['foo', 'bar']},
         {'after': 'repository', 'run': ['baz']},
         {'after': 'repository', 'run': ['baz']},
@@ -455,8 +455,8 @@ def test_before_after_hooks_with_before_error_raises_and_skips_after_hook():
         after='action',
         after='action',
         hook_name='myhook',
         hook_name='myhook',
         action_names=['create'],
         action_names=['create'],
-    ).never()
-    flexmock(module).should_receive('execute_hooks').and_raise(OSError)
+    ).and_return(flexmock()).once()
+    flexmock(module).should_receive('execute_hooks').and_raise(OSError).and_return(None)
     flexmock(module).should_receive('considered_soft_failure').and_return(False)
     flexmock(module).should_receive('considered_soft_failure').and_return(False)
 
 
     with pytest.raises(ValueError):
     with pytest.raises(ValueError):