Move hooks into directory, so there can be one source file per type of hook (#225).

borgmatic/commands/borgmatic.py

@@ -8,7 +8,6 @@ from subprocess import CalledProcessError
 import colorama
 import pkg_resources
 
-from borgmatic import hook
 from borgmatic.borg import check as borg_check
 from borgmatic.borg import create as borg_create
 from borgmatic.borg import environment as borg_environment
@@ -19,6 +18,7 @@ from borgmatic.borg import list as borg_list
 from borgmatic.borg import prune as borg_prune
 from borgmatic.commands.arguments import parse_arguments
 from borgmatic.config import checks, collect, convert, validate
+from borgmatic.hooks import command, healthchecks
 from borgmatic.logger import configure_logging, should_do_markup
 from borgmatic.signals import configure_signals
 from borgmatic.verbosity import verbosity_to_log_level
@@ -53,14 +53,14 @@ def run_configuration(config_filename, config, arguments):
 
     if 'create' in arguments:
         try:
-            hook.execute_hook(
+            command.execute_hook(
                 hooks.get('before_backup'),
                 hooks.get('umask'),
                 config_filename,
                 'pre-backup',
                 global_arguments.dry_run,
             )
-            hook.ping_healthchecks(
+            healthchecks.ping_healthchecks(
                 hooks.get('healthchecks'), config_filename, global_arguments.dry_run, 'start'
             )
         except (OSError, CalledProcessError) as error:
@@ -91,14 +91,14 @@ def run_configuration(config_filename, config, arguments):
 
     if 'create' in arguments and not encountered_error:
         try:
-            hook.execute_hook(
+            command.execute_hook(
                 hooks.get('after_backup'),
                 hooks.get('umask'),
                 config_filename,
                 'post-backup',
                 global_arguments.dry_run,
             )
-            hook.ping_healthchecks(
+            healthchecks.ping_healthchecks(
                 hooks.get('healthchecks'), config_filename, global_arguments.dry_run
             )
         except (OSError, CalledProcessError) as error:
@@ -109,7 +109,7 @@ def run_configuration(config_filename, config, arguments):
 
     if encountered_error:
         try:
-            hook.execute_hook(
+            command.execute_hook(
                 hooks.get('on_error'),
                 hooks.get('umask'),
                 config_filename,
@@ -119,7 +119,7 @@ def run_configuration(config_filename, config, arguments):
                 error=encountered_error,
                 output=getattr(encountered_error, 'output', ''),
             )
-            hook.ping_healthchecks(
+            healthchecks.ping_healthchecks(
                 hooks.get('healthchecks'), config_filename, global_arguments.dry_run, 'fail'
             )
         except (OSError, CalledProcessError) as error:
@@ -339,7 +339,7 @@ def collect_configuration_run_summary_logs(configs, arguments):
         try:
             for config_filename, config in configs.items():
                 hooks = config.get('hooks', {})
-                hook.execute_hook(
+                command.execute_hook(
                     hooks.get('before_everything'),
                     hooks.get('umask'),
                     config_filename,
@@ -379,7 +379,7 @@ def collect_configuration_run_summary_logs(configs, arguments):
         try:
             for config_filename, config in configs.items():
                 hooks = config.get('hooks', {})
-                hook.execute_hook(
+                command.execute_hook(
                     hooks.get('after_everything'),
                     hooks.get('umask'),
                     config_filename,

borgmatic/hook.py → borgmatic/hooks/command.py

@@ -1,8 +1,6 @@
 import logging
 import os
 
-import requests
-
 from borgmatic import execute
 
 logger = logging.getLogger(__name__)
@@ -71,34 +69,3 @@ def execute_hook(commands, umask, config_filename, description, dry_run, **conte
     finally:
         if original_umask:
             os.umask(original_umask)
-
-
-def ping_healthchecks(ping_url_or_uuid, config_filename, dry_run, append=None):
-    '''
-    Ping the given healthchecks.io URL or UUID, appending the append string if any. Use the given
-    configuration filename in any log entries. If this is a dry run, then don't actually ping
-    anything.
-    '''
-    if not ping_url_or_uuid:
-        logger.debug('{}: No healthchecks hook set'.format(config_filename))
-        return
-
-    ping_url = (
-        ping_url_or_uuid
-        if ping_url_or_uuid.startswith('http')
-        else 'https://hc-ping.com/{}'.format(ping_url_or_uuid)
-    )
-    dry_run_label = ' (dry run; not actually pinging)' if dry_run else ''
-
-    if append:
-        ping_url = '{}/{}'.format(ping_url, append)
-
-    logger.info(
-        '{}: Pinging healthchecks.io{}{}'.format(
-            config_filename, ' ' + append if append else '', dry_run_label
-        )
-    )
-    logger.debug('{}: Using healthchecks.io ping URL {}'.format(config_filename, ping_url))
-
-    logging.getLogger('urllib3').setLevel(logging.ERROR)
-    requests.get(ping_url)

borgmatic/hooks/healthchecks.py

@@ -0,0 +1,36 @@
+import logging
+
+import requests
+
+logger = logging.getLogger(__name__)
+
+
+def ping_healthchecks(ping_url_or_uuid, config_filename, dry_run, append=None):
+    '''
+    Ping the given healthchecks.io URL or UUID, appending the append string if any. Use the given
+    configuration filename in any log entries. If this is a dry run, then don't actually ping
+    anything.
+    '''
+    if not ping_url_or_uuid:
+        logger.debug('{}: No healthchecks hook set'.format(config_filename))
+        return
+
+    ping_url = (
+        ping_url_or_uuid
+        if ping_url_or_uuid.startswith('http')
+        else 'https://hc-ping.com/{}'.format(ping_url_or_uuid)
+    )
+    dry_run_label = ' (dry run; not actually pinging)' if dry_run else ''
+
+    if append:
+        ping_url = '{}/{}'.format(ping_url, append)
+
+    logger.info(
+        '{}: Pinging healthchecks.io{}{}'.format(
+            config_filename, ' ' + append if append else '', dry_run_label
+        )
+    )
+    logger.debug('{}: Using healthchecks.io ping URL {}'.format(config_filename, ping_url))
+
+    logging.getLogger('urllib3').setLevel(logging.ERROR)
+    requests.get(ping_url)

tests/unit/commands/test_borgmatic.py

@@ -22,7 +22,7 @@ def test_run_configuration_runs_actions_for_each_repository():
 
 def test_run_configuration_executes_hooks_for_create_action():
     flexmock(module.borg_environment).should_receive('initialize')
-    flexmock(module.hook).should_receive('execute_hook').twice()
+    flexmock(module.command).should_receive('execute_hook').twice()
     flexmock(module).should_receive('run_actions').and_return([])
     config = {'location': {'repositories': ['foo']}}
     arguments = {'global': flexmock(dry_run=False), 'create': flexmock()}
@@ -32,7 +32,7 @@ def test_run_configuration_executes_hooks_for_create_action():
 
 def test_run_configuration_logs_actions_error():
     flexmock(module.borg_environment).should_receive('initialize')
-    flexmock(module.hook).should_receive('execute_hook')
+    flexmock(module.command).should_receive('execute_hook')
     expected_results = [flexmock()]
     flexmock(module).should_receive('make_error_log_records').and_return(expected_results)
     flexmock(module).should_receive('run_actions').and_raise(OSError)
@@ -46,7 +46,7 @@ def test_run_configuration_logs_actions_error():
 
 def test_run_configuration_logs_pre_hook_error():
     flexmock(module.borg_environment).should_receive('initialize')
-    flexmock(module.hook).should_receive('execute_hook').and_raise(OSError).and_return(None)
+    flexmock(module.command).should_receive('execute_hook').and_raise(OSError).and_return(None)
     expected_results = [flexmock()]
     flexmock(module).should_receive('make_error_log_records').and_return(expected_results)
     flexmock(module).should_receive('run_actions').never()
@@ -60,7 +60,7 @@ def test_run_configuration_logs_pre_hook_error():
 
 def test_run_configuration_logs_post_hook_error():
     flexmock(module.borg_environment).should_receive('initialize')
-    flexmock(module.hook).should_receive('execute_hook').and_return(None).and_raise(
+    flexmock(module.command).should_receive('execute_hook').and_return(None).and_raise(
         OSError
     ).and_return(None)
     expected_results = [flexmock()]
@@ -76,7 +76,7 @@ def test_run_configuration_logs_post_hook_error():
 
 def test_run_configuration_logs_on_error_hook_error():
     flexmock(module.borg_environment).should_receive('initialize')
-    flexmock(module.hook).should_receive('execute_hook').and_raise(OSError)
+    flexmock(module.command).should_receive('execute_hook').and_raise(OSError)
     expected_results = [flexmock(), flexmock()]
     flexmock(module).should_receive('make_error_log_records').and_return(
         expected_results[:1]
@@ -148,7 +148,7 @@ def test_make_error_log_records_generates_nothing_for_other_error():
 
 
 def test_collect_configuration_run_summary_logs_info_for_success():
-    flexmock(module.hook).should_receive('execute_hook').never()
+    flexmock(module.command).should_receive('execute_hook').never()
     flexmock(module).should_receive('run_configuration').and_return([])
     arguments = {}
 
@@ -208,7 +208,7 @@ def test_collect_configuration_run_summary_logs_missing_configs_error():
 
 
 def test_collect_configuration_run_summary_logs_pre_hook_error():
-    flexmock(module.hook).should_receive('execute_hook').and_raise(ValueError)
+    flexmock(module.command).should_receive('execute_hook').and_raise(ValueError)
     expected_logs = (flexmock(),)
     flexmock(module).should_receive('make_error_log_records').and_return(expected_logs)
     arguments = {'create': flexmock(), 'global': flexmock(dry_run=False)}
@@ -221,7 +221,7 @@ def test_collect_configuration_run_summary_logs_pre_hook_error():
 
 
 def test_collect_configuration_run_summary_logs_post_hook_error():
-    flexmock(module.hook).should_receive('execute_hook').and_return(None).and_raise(ValueError)
+    flexmock(module.command).should_receive('execute_hook').and_return(None).and_raise(ValueError)
     flexmock(module).should_receive('run_configuration').and_return([])
     expected_logs = (flexmock(),)
     flexmock(module).should_receive('make_error_log_records').and_return(expected_logs)

tests/unit/test_hook.py → tests/unit/hooks/test_command.py

@@ -2,7 +2,7 @@ import logging
 
 from flexmock import flexmock
 
-from borgmatic import hook as module
+from borgmatic.hooks import command as module
 
 
 def test_interpolate_context_passes_through_command_without_variable():
@@ -79,33 +79,3 @@ def test_execute_hook_on_error_logs_as_error():
     ).once()
 
     module.execute_hook([':'], None, 'config.yaml', 'on-error', dry_run=False)
-
-
-def test_ping_healthchecks_hits_ping_url():
-    ping_url = 'https://example.com'
-    flexmock(module.requests).should_receive('get').with_args(ping_url)
-
-    module.ping_healthchecks(ping_url, 'config.yaml', dry_run=False)
-
-
-def test_ping_healthchecks_without_ping_url_does_not_raise():
-    flexmock(module.requests).should_receive('get').never()
-
-    module.ping_healthchecks(ping_url_or_uuid=None, config_filename='config.yaml', dry_run=False)
-
-
-def test_ping_healthchecks_with_ping_uuid_hits_corresponding_url():
-    ping_uuid = 'abcd-efgh-ijkl-mnop'
-    flexmock(module.requests).should_receive('get').with_args(
-        'https://hc-ping.com/{}'.format(ping_uuid)
-    )
-
-    module.ping_healthchecks(ping_uuid, 'config.yaml', dry_run=False)
-
-
-def test_ping_healthchecks_hits_ping_url_with_append():
-    ping_url = 'https://example.com'
-    append = 'failed-so-hard'
-    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, append))
-
-    module.ping_healthchecks(ping_url, 'config.yaml', dry_run=False, append=append)

tests/unit/hooks/test_healthchecks.py

@@ -0,0 +1,33 @@
+from flexmock import flexmock
+
+from borgmatic.hooks import healthchecks as module
+
+
+def test_ping_healthchecks_hits_ping_url():
+    ping_url = 'https://example.com'
+    flexmock(module.requests).should_receive('get').with_args(ping_url)
+
+    module.ping_healthchecks(ping_url, 'config.yaml', dry_run=False)
+
+
+def test_ping_healthchecks_without_ping_url_does_not_raise():
+    flexmock(module.requests).should_receive('get').never()
+
+    module.ping_healthchecks(ping_url_or_uuid=None, config_filename='config.yaml', dry_run=False)
+
+
+def test_ping_healthchecks_with_ping_uuid_hits_corresponding_url():
+    ping_uuid = 'abcd-efgh-ijkl-mnop'
+    flexmock(module.requests).should_receive('get').with_args(
+        'https://hc-ping.com/{}'.format(ping_uuid)
+    )
+
+    module.ping_healthchecks(ping_uuid, 'config.yaml', dry_run=False)
+
+
+def test_ping_healthchecks_hits_ping_url_with_append():
+    ping_url = 'https://example.com'
+    append = 'failed-so-hard'
+    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, append))
+
+    module.ping_healthchecks(ping_url, 'config.yaml', dry_run=False, append=append)