Skip to content

Commit

Permalink
Fix for hooks executing when using --dry-run (#160).
Browse files Browse the repository at this point in the history
  • Loading branch information
witten committed May 7, 2019
1 parent 1c88dda commit a291477
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 12 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 7 additions & 3 deletions borgmatic/commands/borgmatic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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


Expand Down
20 changes: 15 additions & 5 deletions borgmatic/commands/hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,30 @@
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)
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from setuptools import setup, find_packages


VERSION = '1.3.1'
VERSION = '1.3.2'


setup(
Expand Down
13 changes: 10 additions & 3 deletions tests/unit/borg/test_hook.py → tests/unit/commands/test_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,23 @@ 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():
subprocess = flexmock(module.subprocess)
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)

0 comments on commit a291477

Please sign in to comment.