Refactor ping monitors (Healthchecks, Cronitor, Cronhub) to share a common invocation and function signature.

NEWS

@@ -1,3 +1,6 @@
+1.4.10.dev0
+ *
+
 1.4.9
  * #228: Database dump hooks for MySQL/MariaDB, so you can easily dump your databases before backups
    run.

borgmatic/commands/borgmatic.py

@@ -18,7 +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, cronhub, cronitor, dispatch, dump, healthchecks
+from borgmatic.hooks import command, dispatch, dump, monitor
 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,13 @@ def run_configuration(config_filename, config, arguments):
 
     if 'create' in arguments:
         try:
-            healthchecks.ping_healthchecks(
+            dispatch.call_hooks(
-                hooks.get('healthchecks'), config_filename, global_arguments.dry_run, 'start'
+                'ping_monitor',
-            )
+                hooks,
-            cronitor.ping_cronitor(
+                config_filename,
-                hooks.get('cronitor'), config_filename, global_arguments.dry_run, 'run'
+                monitor.MONITOR_HOOK_NAMES,
-            )
+                monitor.State.START,
-            cronhub.ping_cronhub(
+                global_arguments.dry_run,
-                hooks.get('cronhub'), config_filename, global_arguments.dry_run, 'start'
             )
             command.execute_hook(
                 hooks.get('before_backup'),
@@ -119,14 +118,13 @@ def run_configuration(config_filename, config, arguments):
                 'post-backup',
                 global_arguments.dry_run,
             )
-            healthchecks.ping_healthchecks(
+            dispatch.call_hooks(
-                hooks.get('healthchecks'), config_filename, global_arguments.dry_run
+                'ping_monitor',
-            )
+                hooks,
-            cronitor.ping_cronitor(
+                config_filename,
-                hooks.get('cronitor'), config_filename, global_arguments.dry_run, 'complete'
+                monitor.MONITOR_HOOK_NAMES,
-            )
+                monitor.State.FINISH,
-            cronhub.ping_cronhub(
+                global_arguments.dry_run,
-                hooks.get('cronhub'), config_filename, global_arguments.dry_run, 'finish'
             )
         except (OSError, CalledProcessError) as error:
             encountered_error = error
@@ -146,14 +144,13 @@ def run_configuration(config_filename, config, arguments):
                 error=encountered_error,
                 output=getattr(encountered_error, 'output', ''),
             )
-            healthchecks.ping_healthchecks(
+            dispatch.call_hooks(
-                hooks.get('healthchecks'), config_filename, global_arguments.dry_run, 'fail'
+                'ping_monitor',
-            )
+                hooks,
-            cronitor.ping_cronitor(
+                config_filename,
-                hooks.get('cronitor'), config_filename, global_arguments.dry_run, 'fail'
+                monitor.MONITOR_HOOK_NAMES,
-            )
+                monitor.State.FAIL,
-            cronhub.ping_cronhub(
+                global_arguments.dry_run,
-                hooks.get('cronhub'), config_filename, global_arguments.dry_run, 'fail'
             )
         except (OSError, CalledProcessError) as error:
             yield from make_error_log_records(

borgmatic/hooks/cronhub.py

@@ -2,23 +2,29 @@ import logging
 
 import requests
 
+from borgmatic.hooks import monitor
+
 logger = logging.getLogger(__name__)
 
+MONITOR_STATE_TO_CRONHUB = {
+    monitor.State.START: 'start',
+    monitor.State.FINISH: 'finish',
+    monitor.State.FAIL: 'fail',
+}
 
-def ping_cronhub(ping_url, config_filename, dry_run, state):
+
+def ping_monitor(ping_url, config_filename, state, dry_run):
     '''
-    Ping the given Cronhub URL, substituting in the state string. Use the given configuration
+    Ping the given Cronhub URL, modified with the monitor.State. 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:
-        logger.debug('{}: No Cronhub hook set'.format(config_filename))
-        return
-
     dry_run_label = ' (dry run; not actually pinging)' if dry_run else ''
-    formatted_state = '/{}/'.format(state)
+    formatted_state = '/{}/'.format(MONITOR_STATE_TO_CRONHUB[state])
     ping_url = ping_url.replace('/start/', formatted_state).replace('/ping/', formatted_state)
 
-    logger.info('{}: Pinging Cronhub {}{}'.format(config_filename, state, dry_run_label))
+    logger.info(
+        '{}: Pinging Cronhub {}{}'.format(config_filename, state.name.lower(), dry_run_label)
+    )
     logger.debug('{}: Using Cronhub ping URL {}'.format(config_filename, ping_url))
 
     if not dry_run:

borgmatic/hooks/cronitor.py

@@ -2,22 +2,28 @@ import logging
 
 import requests
 
+from borgmatic.hooks import monitor
+
 logger = logging.getLogger(__name__)
 
+MONITOR_STATE_TO_CRONITOR = {
+    monitor.State.START: 'run',
+    monitor.State.FINISH: 'complete',
+    monitor.State.FAIL: 'fail',
+}
 
-def ping_cronitor(ping_url, config_filename, dry_run, append):
-    '''
-    Ping the given Cronitor URL, appending the append string. 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:
-        logger.debug('{}: No Cronitor hook set'.format(config_filename))
-        return
 
+def ping_monitor(ping_url, config_filename, state, dry_run):
+    '''
+    Ping the given Cronitor URL, modified with the monitor.State. Use the given configuration
+    filename in any log entries. If this is a dry run, then don't actually ping anything.
+    '''
     dry_run_label = ' (dry run; not actually pinging)' if dry_run else ''
-    ping_url = '{}/{}'.format(ping_url, append)
+    ping_url = '{}/{}'.format(ping_url, MONITOR_STATE_TO_CRONITOR[state])
 
-    logger.info('{}: Pinging Cronitor {}{}'.format(config_filename, append, dry_run_label))
+    logger.info(
+        '{}: Pinging Cronitor {}{}'.format(config_filename, state.name.lower(), dry_run_label)
+    )
     logger.debug('{}: Using Cronitor ping URL {}'.format(config_filename, ping_url))
 
     if not dry_run:

borgmatic/hooks/dispatch.py

@@ -1,10 +1,16 @@
 import logging
 
-from borgmatic.hooks import mysql, postgresql
+from borgmatic.hooks import cronhub, cronitor, healthchecks, mysql, postgresql
 
 logger = logging.getLogger(__name__)
 
-HOOK_NAME_TO_MODULE = {'postgresql_databases': postgresql, 'mysql_databases': mysql}
+HOOK_NAME_TO_MODULE = {
+    'healthchecks': healthchecks,
+    'cronitor': cronitor,
+    'cronhub': cronhub,
+    'postgresql_databases': postgresql,
+    'mysql_databases': mysql,
+}
 
 
 def call_hook(function_name, hooks, log_prefix, hook_name, *args, **kwargs):

borgmatic/hooks/dump.py

@@ -109,7 +109,6 @@ def get_per_hook_database_configurations(hooks, names, dump_patterns):
     Raise ValueError if one of the database names cannot be matched to a database in borgmatic's
     database configuration.
     '''
-    # TODO: Need to filter names by database type? Maybe take a database --type argument to disambiguate.
     hook_databases = {
         hook_name: list(
             get_database_configurations(

borgmatic/hooks/healthchecks.py

@@ -2,19 +2,23 @@ import logging
 
 import requests
 
+from borgmatic.hooks import monitor
+
 logger = logging.getLogger(__name__)
 
+MONITOR_STATE_TO_HEALTHCHECKS = {
+    monitor.State.START: 'start',
+    monitor.State.FINISH: None,  # Healthchecks doesn't append to the URL for the finished state.
+    monitor.State.FAIL: 'fail',
+}
 
-def ping_healthchecks(ping_url_or_uuid, config_filename, dry_run, append=None):
+
+def ping_monitor(ping_url_or_uuid, config_filename, state, dry_run):
     '''
-    Ping the given Healthchecks URL or UUID, appending the append string if any. Use the given
+    Ping the given Healthchecks URL or UUID, modified with the monitor.State. 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')
@@ -22,13 +26,12 @@ def ping_healthchecks(ping_url_or_uuid, config_filename, dry_run, append=None):
     )
     dry_run_label = ' (dry run; not actually pinging)' if dry_run else ''
 
-    if append:
+    healthchecks_state = MONITOR_STATE_TO_HEALTHCHECKS.get(state)
-        ping_url = '{}/{}'.format(ping_url, append)
+    if healthchecks_state:
+        ping_url = '{}/{}'.format(ping_url, healthchecks_state)
 
     logger.info(
-        '{}: Pinging Healthchecks{}{}'.format(
+        '{}: Pinging Healthchecks {}{}'.format(config_filename, state.name.lower(), dry_run_label)
-            config_filename, ' ' + append if append else '', dry_run_label
-        )
     )
     logger.debug('{}: Using Healthchecks ping URL {}'.format(config_filename, ping_url))
 

borgmatic/hooks/monitor.py

@@ -0,0 +1,9 @@
+from enum import Enum
+
+MONITOR_HOOK_NAMES = ('healthchecks', 'cronitor', 'cronhub')
+
+
+class State(Enum):
+    START = 1
+    FINISH = 2
+    FAIL = 3

setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-VERSION = '1.4.9'
+VERSION = '1.4.10.dev0'
 
 
 setup(

tests/unit/commands/test_borgmatic.py

@@ -23,10 +23,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.command).should_receive('execute_hook').twice()
-    flexmock(module.dispatch).should_receive('call_hooks').twice()
+    flexmock(module.dispatch).should_receive('call_hooks').at_least().twice()
-    flexmock(module.healthchecks).should_receive('ping_healthchecks').twice()
-    flexmock(module.cronitor).should_receive('ping_cronitor').twice()
-    flexmock(module.cronhub).should_receive('ping_cronhub').twice()
     flexmock(module).should_receive('run_actions').and_return([])
     config = {'location': {'repositories': ['foo']}}
     arguments = {'global': flexmock(dry_run=False), 'create': flexmock()}
@@ -38,9 +35,6 @@ def test_run_configuration_logs_actions_error():
     flexmock(module.borg_environment).should_receive('initialize')
     flexmock(module.command).should_receive('execute_hook')
     flexmock(module.dispatch).should_receive('call_hooks')
-    flexmock(module.healthchecks).should_receive('ping_healthchecks')
-    flexmock(module.cronitor).should_receive('ping_cronitor')
-    flexmock(module.cronhub).should_receive('ping_cronhub')
     expected_results = [flexmock()]
     flexmock(module).should_receive('make_error_log_records').and_return(expected_results)
     flexmock(module).should_receive('run_actions').and_raise(OSError)
@@ -72,9 +66,6 @@ def test_run_configuration_logs_post_hook_error():
         OSError
     ).and_return(None)
     flexmock(module.dispatch).should_receive('call_hooks')
-    flexmock(module.healthchecks).should_receive('ping_healthchecks')
-    flexmock(module.cronitor).should_receive('ping_cronitor')
-    flexmock(module.cronhub).should_receive('ping_cronhub')
     expected_results = [flexmock()]
     flexmock(module).should_receive('make_error_log_records').and_return(expected_results)
     flexmock(module).should_receive('run_actions').and_return([])

tests/unit/hooks/test_cronhub.py

@@ -3,30 +3,36 @@ from flexmock import flexmock
 from borgmatic.hooks import cronhub as module
 
 
-def test_ping_cronhub_hits_ping_url_with_start_state():
+def test_ping_monitor_rewrites_ping_url_for_start_state():
     ping_url = 'https://example.com/start/abcdef'
-    state = 'bork'
+    flexmock(module.requests).should_receive('get').with_args('https://example.com/start/abcdef')
-    flexmock(module.requests).should_receive('get').with_args('https://example.com/bork/abcdef')
 
-    module.ping_cronhub(ping_url, 'config.yaml', dry_run=False, state=state)
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=False)
 
 
-def test_ping_cronhub_hits_ping_url_with_ping_state():
+def test_ping_monitor_rewrites_ping_url_and_state_for_start_state():
     ping_url = 'https://example.com/ping/abcdef'
-    state = 'bork'
+    flexmock(module.requests).should_receive('get').with_args('https://example.com/start/abcdef')
-    flexmock(module.requests).should_receive('get').with_args('https://example.com/bork/abcdef')
 
-    module.ping_cronhub(ping_url, 'config.yaml', dry_run=False, state=state)
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=False)
 
 
-def test_ping_cronhub_without_ping_url_does_not_raise():
+def test_ping_monitor_rewrites_ping_url_for_finish_state():
-    flexmock(module.requests).should_receive('get').never()
+    ping_url = 'https://example.com/start/abcdef'
+    flexmock(module.requests).should_receive('get').with_args('https://example.com/finish/abcdef')
 
-    module.ping_cronhub(ping_url=None, config_filename='config.yaml', dry_run=False, state='oops')
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.FINISH, dry_run=False)
 
 
-def test_ping_cronhub_dry_run_does_not_hit_ping_url():
+def test_ping_monitor_rewrites_ping_url_for_fail_state():
+    ping_url = 'https://example.com/start/abcdef'
+    flexmock(module.requests).should_receive('get').with_args('https://example.com/fail/abcdef')
+
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.FAIL, dry_run=False)
+
+
+def test_ping_monitor_dry_run_does_not_hit_ping_url():
     ping_url = 'https://example.com'
     flexmock(module.requests).should_receive('get').never()
 
-    module.ping_cronhub(ping_url, 'config.yaml', dry_run=True, state='yay')
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=True)

tests/unit/hooks/test_cronitor.py

@@ -3,22 +3,29 @@ from flexmock import flexmock
 from borgmatic.hooks import cronitor as module
 
 
-def test_ping_cronitor_hits_ping_url():
+def test_ping_monitor_hits_ping_url_for_start_state():
     ping_url = 'https://example.com'
-    append = 'failed-so-hard'
+    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, 'run'))
-    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, append))
 
-    module.ping_cronitor(ping_url, 'config.yaml', dry_run=False, append=append)
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=False)
 
 
-def test_ping_cronitor_without_ping_url_does_not_raise():
+def test_ping_monitor_hits_ping_url_for_finish_state():
-    flexmock(module.requests).should_receive('get').never()
+    ping_url = 'https://example.com'
+    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, 'complete'))
 
-    module.ping_cronitor(ping_url=None, config_filename='config.yaml', dry_run=False, append='oops')
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.FINISH, dry_run=False)
 
 
-def test_ping_cronitor_dry_run_does_not_hit_ping_url():
+def test_ping_monitor_hits_ping_url_for_fail_state():
+    ping_url = 'https://example.com'
+    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, 'fail'))
+
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.FAIL, dry_run=False)
+
+
+def test_ping_monitor_dry_run_does_not_hit_ping_url():
     ping_url = 'https://example.com'
     flexmock(module.requests).should_receive('get').never()
 
-    module.ping_cronitor(ping_url, 'config.yaml', dry_run=True, append='yay')
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=True)

tests/unit/hooks/test_healthchecks.py

@@ -3,38 +3,38 @@ from flexmock import flexmock
 from borgmatic.hooks import healthchecks as module
 
 
-def test_ping_healthchecks_hits_ping_url():
+def test_ping_monitor_hits_ping_url_for_start_state():
+    ping_url = 'https://example.com'
+    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, 'start'))
+
+    module.ping_monitor(ping_url, 'config.yaml', state=module.monitor.State.START, dry_run=False)
+
+
+def test_ping_monitor_hits_ping_url_for_finish_state():
     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)
+    module.ping_monitor(ping_url, 'config.yaml', state=module.monitor.State.FINISH, dry_run=False)
 
 
-def test_ping_healthchecks_without_ping_url_does_not_raise():
+def test_ping_monitor_hits_ping_url_for_fail_state():
-    flexmock(module.requests).should_receive('get').never()
+    ping_url = 'https://example.com'
+    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, 'fail'))
 
-    module.ping_healthchecks(ping_url_or_uuid=None, config_filename='config.yaml', dry_run=False)
+    module.ping_monitor(ping_url, 'config.yaml', state=module.monitor.State.FAIL, dry_run=False)
 
 
-def test_ping_healthchecks_with_ping_uuid_hits_corresponding_url():
+def test_ping_monitor_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)
+    module.ping_monitor(ping_uuid, 'config.yaml', state=module.monitor.State.FINISH, dry_run=False)
 
 
-def test_ping_healthchecks_hits_ping_url_with_append():
+def test_ping_monitor_dry_run_does_not_hit_ping_url():
-    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)
-
-
-def test_ping_healthchecks_dry_run_does_not_hit_ping_url():
     ping_url = 'https://example.com'
     flexmock(module.requests).should_receive('get').never()
 
-    module.ping_healthchecks(ping_url, 'config.yaml', dry_run=True)
+    module.ping_monitor(ping_url, 'config.yaml', state=module.monitor.State.START, dry_run=True)