From a291477c19bc8bbf674206e6fb955b466ccd3e6b Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 7 May 2019 16:06:31 -0700 Subject: [PATCH] Fix for hooks executing when using --dry-run (#160). --- NEWS | 3 +++ borgmatic/commands/borgmatic.py | 10 +++++++--- borgmatic/commands/hook.py | 20 +++++++++++++++----- setup.py | 2 +- tests/unit/{borg => commands}/test_hook.py | 13 ++++++++++--- 5 files changed, 36 insertions(+), 12 deletions(-) rename tests/unit/{borg => commands}/test_hook.py (67%) diff --git a/NEWS b/NEWS index 09a6315..44658e2 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.3.2 + * #160: Fix for hooks executing when using --dry-run. Now hooks are skipped during a dry run. + 1.3.1 * #155: Fix for invalid JSON output when using multiple borgmatic configuration files. * #157: Fix for seemingly random filename ordering when running through a directory of diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 4ab5a75..31e810e 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -272,7 +272,9 @@ def run_configuration(config_filename, config, args): # pragma: no cover borg_environment.initialize(storage) if args.create: - hook.execute_hook(hooks.get('before_backup'), config_filename, 'pre-backup') + hook.execute_hook( + hooks.get('before_backup'), config_filename, 'pre-backup', args.dry_run + ) for repository_path in location['repositories']: yield from run_actions( @@ -287,9 +289,11 @@ def run_configuration(config_filename, config, args): # pragma: no cover ) if args.create: - hook.execute_hook(hooks.get('after_backup'), config_filename, 'post-backup') + hook.execute_hook( + hooks.get('after_backup'), config_filename, 'post-backup', args.dry_run + ) except (OSError, CalledProcessError): - hook.execute_hook(hooks.get('on_error'), config_filename, 'on-error') + hook.execute_hook(hooks.get('on_error'), config_filename, 'on-error', args.dry_run) raise diff --git a/borgmatic/commands/hook.py b/borgmatic/commands/hook.py index 7546947..77cbcc9 100644 --- a/borgmatic/commands/hook.py +++ b/borgmatic/commands/hook.py @@ -5,20 +5,30 @@ import subprocess logger = logging.getLogger(__name__) -def execute_hook(commands, config_filename, description): +def execute_hook(commands, config_filename, description, dry_run): + ''' + + Given a list of hook commands to execute, a config filename, a hook description, and whether + this is a dry run, run the given commands. Or, don't run them if this is a dry run. + ''' if not commands: logger.debug('{}: No commands to run for {} hook'.format(config_filename, description)) return + dry_run_label = ' (dry run; not actually running hooks)' if dry_run else '' + if len(commands) == 1: - logger.info('{}: Running command for {} hook'.format(config_filename, description)) + logger.info( + '{}: Running command for {} hook{}'.format(config_filename, description, dry_run_label) + ) else: logger.info( - '{}: Running {} commands for {} hook'.format( - config_filename, len(commands), description + '{}: Running {} commands for {} hook{}'.format( + config_filename, len(commands), description, dry_run_label ) ) for command in commands: logger.debug('{}: Hook command: {}'.format(config_filename, command)) - subprocess.check_call(command, shell=True) + if not dry_run: + subprocess.check_call(command, shell=True) diff --git a/setup.py b/setup.py index 134e91f..31b1866 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,7 @@ from setuptools import setup, find_packages -VERSION = '1.3.1' +VERSION = '1.3.2' setup( diff --git a/tests/unit/borg/test_hook.py b/tests/unit/commands/test_hook.py similarity index 67% rename from tests/unit/borg/test_hook.py rename to tests/unit/commands/test_hook.py index f87edda..d54c078 100644 --- a/tests/unit/borg/test_hook.py +++ b/tests/unit/commands/test_hook.py @@ -7,7 +7,7 @@ def test_execute_hook_invokes_each_command(): subprocess = flexmock(module.subprocess) subprocess.should_receive('check_call').with_args(':', shell=True).once() - module.execute_hook([':'], 'config.yaml', 'pre-backup') + module.execute_hook([':'], 'config.yaml', 'pre-backup', dry_run=False) def test_execute_hook_with_multiple_commands_invokes_each_command(): @@ -15,8 +15,15 @@ def test_execute_hook_with_multiple_commands_invokes_each_command(): subprocess.should_receive('check_call').with_args(':', shell=True).once() subprocess.should_receive('check_call').with_args('true', shell=True).once() - module.execute_hook([':', 'true'], 'config.yaml', 'pre-backup') + module.execute_hook([':', 'true'], 'config.yaml', 'pre-backup', dry_run=False) + + +def test_execute_hook_with_dry_run_skips_commands(): + subprocess = flexmock(module.subprocess) + subprocess.should_receive('check_call').never() + + module.execute_hook([':', 'true'], 'config.yaml', 'pre-backup', dry_run=True) def test_execute_hook_with_empty_commands_does_not_raise(): - module.execute_hook([], 'config.yaml', 'post-backup') + module.execute_hook([], 'config.yaml', 'post-backup', dry_run=False)