Fix for "borgmatic restore" showing success and incorrectly extracting archive files, even when no databases are configured to restore (#246).

.gitea/issue_template.md

@@ -28,4 +28,8 @@ Use `sudo borg --version`
 
 Use `python3 --version`
 
+**Database version (if applicable):** [version here]
+
+Use `psql --version` or `mysql --version` on client and server.
+
 **operating system and version:** [OS here]

AUTHORS

@@ -10,3 +10,5 @@ newtonne: Read encryption password from external file
 Robin `ypid` Schneider: Support additional options of Borg and add validate-borgmatic-config command
 Scott Squires: Custom archive names
 Thomas LÉVEIL: Support for a keep_minutely prune option. Support for the --json option
+
+And many others! See the output of "git log".

NEWS

@@ -1,3 +1,26 @@
+1.4.10
+ * #246: Fix for "borgmatic restore" showing success and incorrectly extracting archive files, even
+   when no databases are configured to restore. As this can overwrite files from the archive and
+   lead to data loss, please upgrade to get the fix before using "borgmatic restore".
+ * Reopen the file given by "--log-file" flag if an external program rotates the log file while
+   borgmatic is running.
+
+1.4.9
+ * #228: Database dump hooks for MySQL/MariaDB, so you can easily dump your databases before backups
+   run.
+ * #243: Fix repository does not exist error with "borgmatic extract" when repository is remote.
+
+1.4.8
+ * Monitor backups with Cronhub hook integration. See the documentation for more information:
+   https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#cronhub-hook
+ * Fix Healthchecks/Cronitor hooks to skip actions when the borgmatic "--dry-run" flag is used.
+
+1.4.7
+ * #238: In documentation, clarify when Healthchecks/Cronitor hooks fire in relation to other hooks.
+ * #239: Upgrade your borgmatic configuration to get new options and comments via
+   "generate-borgmatic-config --source". See the documentation for more information:
+   https://torsion.org/borgmatic/docs/how-to/upgrade/#upgrading-your-configuration
+
 1.4.6
  * Verbosity level "-1" for even quieter output: Errors only (#236).
 

borgmatic/borg/extract.py

@@ -62,6 +62,7 @@ def extract_archive(
     remote_path=None,
     destination_path=None,
     progress=False,
+    error_on_warnings=True,
 ):
     '''
     Given a dry-run flag, a local or remote repository path, an archive name, zero or more paths to
@@ -82,7 +83,7 @@ def extract_archive(
         + (('--debug', '--list', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ())
         + (('--dry-run',) if dry_run else ())
         + (('--progress',) if progress else ())
-        + ('::'.join((os.path.abspath(repository), archive)),)
+        + ('::'.join((repository if ':' in repository else os.path.abspath(repository), archive)),)
         + (tuple(paths) if paths else ())
     )
 
@@ -90,10 +91,12 @@ def extract_archive(
     # the terminal directly.
     if progress:
         execute_command_without_capture(
-            full_command, working_directory=destination_path, error_on_warnings=True
+            full_command, working_directory=destination_path, error_on_warnings=error_on_warnings
         )
         return
 
-    # Error on warnings, as Borg only gives a warning if the restore paths don't exist in the
-    # archive!
-    execute_command(full_command, working_directory=destination_path, error_on_warnings=True)
+    # Error on warnings by default, as Borg only gives a warning if the restore paths don't exist in
+    # the archive!
+    execute_command(
+        full_command, working_directory=destination_path, error_on_warnings=error_on_warnings
+    )

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, cronitor, healthchecks, postgresql
+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,11 +53,13 @@ def run_configuration(config_filename, config, arguments):
 
     if 'create' in arguments:
         try:
-            healthchecks.ping_healthchecks(
-                hooks.get('healthchecks'), config_filename, global_arguments.dry_run, 'start'
-            )
-            cronitor.ping_cronitor(
-                hooks.get('cronitor'), config_filename, global_arguments.dry_run, 'run'
+            dispatch.call_hooks(
+                'ping_monitor',
+                hooks,
+                config_filename,
+                monitor.MONITOR_HOOK_NAMES,
+                monitor.State.START,
+                global_arguments.dry_run,
             )
             command.execute_hook(
                 hooks.get('before_backup'),
@@ -66,8 +68,12 @@ def run_configuration(config_filename, config, arguments):
                 'pre-backup',
                 global_arguments.dry_run,
             )
-            postgresql.dump_databases(
-                hooks.get('postgresql_databases'), config_filename, global_arguments.dry_run
+            dispatch.call_hooks(
+                'dump_databases',
+                hooks,
+                config_filename,
+                dump.DATABASE_HOOK_NAMES,
+                global_arguments.dry_run,
             )
         except (OSError, CalledProcessError) as error:
             encountered_error = error
@@ -98,8 +104,12 @@ def run_configuration(config_filename, config, arguments):
 
     if 'create' in arguments and not encountered_error:
         try:
-            postgresql.remove_database_dumps(
-                hooks.get('postgresql_databases'), config_filename, global_arguments.dry_run
+            dispatch.call_hooks(
+                'remove_database_dumps',
+                hooks,
+                config_filename,
+                dump.DATABASE_HOOK_NAMES,
+                global_arguments.dry_run,
             )
             command.execute_hook(
                 hooks.get('after_backup'),
@@ -108,11 +118,13 @@ def run_configuration(config_filename, config, arguments):
                 'post-backup',
                 global_arguments.dry_run,
             )
-            healthchecks.ping_healthchecks(
-                hooks.get('healthchecks'), config_filename, global_arguments.dry_run
-            )
-            cronitor.ping_cronitor(
-                hooks.get('cronitor'), config_filename, global_arguments.dry_run, 'complete'
+            dispatch.call_hooks(
+                'ping_monitor',
+                hooks,
+                config_filename,
+                monitor.MONITOR_HOOK_NAMES,
+                monitor.State.FINISH,
+                global_arguments.dry_run,
             )
         except (OSError, CalledProcessError) as error:
             encountered_error = error
@@ -132,11 +144,13 @@ def run_configuration(config_filename, config, arguments):
                 error=encountered_error,
                 output=getattr(encountered_error, 'output', ''),
             )
-            healthchecks.ping_healthchecks(
-                hooks.get('healthchecks'), config_filename, global_arguments.dry_run, 'fail'
-            )
-            cronitor.ping_cronitor(
-                hooks.get('cronitor'), config_filename, global_arguments.dry_run, 'fail'
+            dispatch.call_hooks(
+                'ping_monitor',
+                hooks,
+                config_filename,
+                monitor.MONITOR_HOOK_NAMES,
+                monitor.State.FAIL,
+                global_arguments.dry_run,
             )
         except (OSError, CalledProcessError) as error:
             yield from make_error_log_records(
@@ -245,31 +259,51 @@ def run_actions(
                 restore_names = []
 
             # Extract dumps for the named databases from the archive.
-            dump_patterns = postgresql.make_database_dump_patterns(restore_names)
+            dump_patterns = dispatch.call_hooks(
+                'make_database_dump_patterns',
+                hooks,
+                repository,
+                dump.DATABASE_HOOK_NAMES,
+                restore_names,
+            )
+
             borg_extract.extract_archive(
                 global_arguments.dry_run,
                 repository,
                 arguments['restore'].archive,
-                postgresql.convert_glob_patterns_to_borg_patterns(dump_patterns),
+                dump.convert_glob_patterns_to_borg_patterns(
+                    dump.flatten_dump_patterns(dump_patterns, restore_names)
+                ),
                 location,
                 storage,
                 local_path=local_path,
                 remote_path=remote_path,
                 destination_path='/',
                 progress=arguments['restore'].progress,
+                # We don't want glob patterns that don't match to error.
+                error_on_warnings=False,
             )
 
-            # Map the restore names to the corresponding database configurations.
-            databases = list(
-                postgresql.get_database_configurations(
-                    hooks.get('postgresql_databases'),
-                    restore_names or postgresql.get_database_names_from_dumps(dump_patterns),
-                )
+            # Map the restore names or detected dumps to the corresponding database configurations.
+            restore_databases = dump.get_per_hook_database_configurations(
+                hooks, restore_names, dump_patterns
             )
 
             # Finally, restore the databases and cleanup the dumps.
-            postgresql.restore_database_dumps(databases, repository, global_arguments.dry_run)
-            postgresql.remove_database_dumps(databases, repository, global_arguments.dry_run)
+            dispatch.call_hooks(
+                'restore_database_dumps',
+                restore_databases,
+                repository,
+                dump.DATABASE_HOOK_NAMES,
+                global_arguments.dry_run,
+            )
+            dispatch.call_hooks(
+                'remove_database_dumps',
+                restore_databases,
+                repository,
+                dump.DATABASE_HOOK_NAMES,
+                global_arguments.dry_run,
+            )
     if 'list' in arguments:
         if arguments['list'].repository is None or repository == arguments['list'].repository:
             logger.info('{}: Listing archives'.format(repository))

borgmatic/commands/generate_config.py

@@ -12,12 +12,18 @@ def parse_arguments(*arguments):
     them as an ArgumentParser instance.
     '''
     parser = ArgumentParser(description='Generate a sample borgmatic YAML configuration file.')
+    parser.add_argument(
+        '-s',
+        '--source',
+        dest='source_filename',
+        help='Optional YAML configuration file to merge into the generated configuration, useful for upgrading your configuration',
+    )
     parser.add_argument(
         '-d',
         '--destination',
         dest='destination_filename',
         default=DEFAULT_DESTINATION_CONFIG_FILENAME,
-        help='Destination YAML configuration filename. Default: {}'.format(
+        help='Destination YAML configuration file. Default: {}'.format(
             DEFAULT_DESTINATION_CONFIG_FILENAME
         ),
     )
@@ -30,11 +36,21 @@ def main():  # pragma: no cover
         args = parse_arguments(*sys.argv[1:])
 
         generate.generate_sample_configuration(
-            args.destination_filename, validate.schema_filename()
+            args.source_filename, args.destination_filename, validate.schema_filename()
         )
 
         print('Generated a sample configuration file at {}.'.format(args.destination_filename))
         print()
+        if args.source_filename:
+            print(
+                'Merged in the contents of configuration file at {}.'.format(args.source_filename)
+            )
+            print('To review the changes made, run:')
+            print()
+            print(
+                '    diff --unified {} {}'.format(args.source_filename, args.destination_filename)
+            )
+            print()
         print('Please edit the file to suit your needs. The values are representative.')
         print('All fields are optional except where indicated.')
         print()

borgmatic/config/generate.py

@@ -1,9 +1,12 @@
+import collections
 import io
 import os
 import re
 
 from ruamel import yaml
 
+from borgmatic.config import load
+
 INDENT = 4
 SEQUENCE_INDENT = 2
 
@@ -40,8 +43,8 @@ def _schema_to_sample_configuration(schema, level=0, parent_is_sequence=False):
     elif 'map' in schema:
         config = yaml.comments.CommentedMap(
             [
-                (section_name, _schema_to_sample_configuration(section_schema, level + 1))
-                for section_name, section_schema in schema['map'].items()
+                (field_name, _schema_to_sample_configuration(sub_schema, level + 1))
+                for field_name, sub_schema in schema['map'].items()
             ]
         )
         indent = (level * INDENT) + (SEQUENCE_INDENT if parent_is_sequence else 0)
@@ -68,37 +71,32 @@ def _comment_out_line(line):
     return '# '.join((indent_spaces, line[count_indent_spaces:]))
 
 
-REQUIRED_KEYS = {'source_directories', 'repositories', 'keep_daily'}
-REQUIRED_SECTION_NAMES = {'location', 'retention'}
-
-
 def _comment_out_optional_configuration(rendered_config):
     '''
-    Post-process a rendered configuration string to comment out optional key/values. The idea is
-    that this prevents the user from having to comment out a bunch of configuration they don't care
-    about to get to a minimal viable configuration file.
+    Post-process a rendered configuration string to comment out optional key/values, as determined
+    by a sentinel in the comment before each key.
 
-    Ideally ruamel.yaml would support this during configuration generation, but it's not terribly
-    easy to accomplish that way.
+    The idea is that the pre-commented configuration prevents the user from having to comment out a
+    bunch of configuration they don't care about to get to a minimal viable configuration file.
+
+    Ideally ruamel.yaml would support commenting out keys during configuration generation, but it's
+    not terribly easy to accomplish that way.
     '''
     lines = []
-    required = False
+    optional = False
 
     for line in rendered_config.split('\n'):
-        key = line.strip().split(':')[0]
-
-        if key in REQUIRED_SECTION_NAMES:
-            lines.append(line)
+        # Upon encountering an optional configuration option, commenting out lines until the next
+        # blank line.
+        if line.strip().startswith('# {}'.format(COMMENTED_OUT_SENTINEL)):
+            optional = True
             continue
 
-        # Upon encountering a required configuration option, skip commenting out lines until the
-        # next blank line.
-        if key in REQUIRED_KEYS:
-            required = True
-        elif not key:
-            required = False
+        # Hit a blank line, so reset commenting.
+        if not line.strip():
+            optional = False
 
-        lines.append(_comment_out_line(line) if not required else line)
+        lines.append(_comment_out_line(line) if optional else line)
 
     return '\n'.join(lines)
 
@@ -168,6 +166,11 @@ def add_comments_to_configuration_sequence(config, schema, indent=0):
         return
 
 
+REQUIRED_SECTION_NAMES = {'location', 'retention'}
+REQUIRED_KEYS = {'source_directories', 'repositories', 'keep_daily'}
+COMMENTED_OUT_SENTINEL = 'COMMENT_OUT'
+
+
 def add_comments_to_configuration_map(config, schema, indent=0, skip_first=False):
     '''
     Using descriptions from a schema as a source, add those descriptions as comments to the given
@@ -178,10 +181,20 @@ def add_comments_to_configuration_map(config, schema, indent=0, skip_first=False
             continue
 
         field_schema = schema['map'].get(field_name, {})
-        description = field_schema.get('desc')
+        description = field_schema.get('desc', '').strip()
+
+        # If this is an optional key, add an indicator to the comment flagging it to be commented
+        # out from the sample configuration. This sentinel is consumed by downstream processing that
+        # does the actual commenting out.
+        if field_name not in REQUIRED_SECTION_NAMES and field_name not in REQUIRED_KEYS:
+            description = (
+                '\n'.join((description, COMMENTED_OUT_SENTINEL))
+                if description
+                else COMMENTED_OUT_SENTINEL
+            )
 
         # No description to use? Skip it.
-        if not field_schema or not description:
+        if not field_schema or not description:  # pragma: no cover
             continue
 
         config.yaml_set_comment_before_after_key(key=field_name, before=description, indent=indent)
@@ -190,14 +203,88 @@ def add_comments_to_configuration_map(config, schema, indent=0, skip_first=False
             _insert_newline_before_comment(config, field_name)
 
 
-def generate_sample_configuration(config_filename, schema_filename):
+RUAMEL_YAML_COMMENTS_INDEX = 1
+
+
+def remove_commented_out_sentinel(config, field_name):
     '''
-    Given a target config filename and the path to a schema filename in pykwalify YAML schema
-    format, write out a sample configuration file based on that schema.
+    Given a configuration CommentedMap and a top-level field name in it, remove any "commented out"
+    sentinel found at the end of its YAML comments. This prevents the given field name from getting
+    commented out by downstream processing that consumes the sentinel.
+    '''
+    try:
+        last_comment_value = config.ca.items[field_name][RUAMEL_YAML_COMMENTS_INDEX][-1].value
+    except KeyError:
+        return
+
+    if last_comment_value == '# {}\n'.format(COMMENTED_OUT_SENTINEL):
+        config.ca.items[field_name][RUAMEL_YAML_COMMENTS_INDEX].pop()
+
+
+def merge_source_configuration_into_destination(destination_config, source_config):
+    '''
+    Deep merge the given source configuration dict into the destination configuration CommentedMap,
+    favoring values from the source when there are collisions.
+
+    The purpose of this is to upgrade configuration files from old versions of borgmatic by adding
+    new
+    configuration keys and comments.
+    '''
+    if not source_config:
+        return destination_config
+    if not destination_config or not isinstance(source_config, collections.abc.Mapping):
+        return source_config
+
+    for field_name, source_value in source_config.items():
+        # Since this key/value is from the source configuration, leave it uncommented and remove any
+        # sentinel that would cause it to get commented out.
+        remove_commented_out_sentinel(destination_config, field_name)
+
+        # This is a mapping. Recurse for this key/value.
+        if isinstance(source_value, collections.abc.Mapping):
+            destination_config[field_name] = merge_source_configuration_into_destination(
+                destination_config[field_name], source_value
+            )
+            continue
+
+        # This is a sequence. Recurse for each item in it.
+        if isinstance(source_value, collections.abc.Sequence) and not isinstance(source_value, str):
+            destination_value = destination_config[field_name]
+            destination_config[field_name] = yaml.comments.CommentedSeq(
+                [
+                    merge_source_configuration_into_destination(
+                        destination_value[index] if index < len(destination_value) else None,
+                        source_item,
+                    )
+                    for index, source_item in enumerate(source_value)
+                ]
+            )
+            continue
+
+        # This is some sort of scalar. Simply set it into the destination.
+        destination_config[field_name] = source_config[field_name]
+
+    return destination_config
+
+
+def generate_sample_configuration(source_filename, destination_filename, schema_filename):
+    '''
+    Given an optional source configuration filename, and a required destination configuration
+    filename, and the path to a schema filename in pykwalify YAML schema format, write out a
+    sample configuration file based on that schema. If a source filename is provided, merge the
+    parsed contents of that configuration into the generated configuration.
     '''
     schema = yaml.round_trip_load(open(schema_filename))
-    config = _schema_to_sample_configuration(schema)
+    source_config = None
+
+    if source_filename:
+        source_config = load.load_configuration(source_filename)
+
+    destination_config = merge_source_configuration_into_destination(
+        _schema_to_sample_configuration(schema), source_config
+    )
 
     write_configuration(
-        config_filename, _comment_out_optional_configuration(_render_configuration(config))
+        destination_filename,
+        _comment_out_optional_configuration(_render_configuration(destination_config)),
     )

borgmatic/config/schema.yaml

@@ -434,20 +434,82 @@ map:
                     directories at runtime, backed up, and then removed afterwards. Requires
                     pg_dump/pg_dumpall/pg_restore commands. See
                     https://www.postgresql.org/docs/current/app-pgdump.html for details.
+            mysql_databases:
+                seq:
+                    - map:
+                        name:
+                            required: true
+                            type: str
+                            desc: |
+                                Database name (required if using this hook). Or "all" to dump all
+                                databases on the host.
+                            example: users
+                        hostname:
+                            type: str
+                            desc: |
+                                Database hostname to connect to. Defaults to connecting via local
+                                Unix socket.
+                            example: database.example.org
+                        port:
+                            type: int
+                            desc: Port to connect to. Defaults to 3306.
+                            example: 3307
+                        username:
+                            type: str
+                            desc: |
+                                Username with which to connect to the database. Defaults to the
+                                username of the current user.
+                            example: dbuser
+                        password:
+                            type: str
+                            desc: |
+                                Password with which to connect to the database. Omitting a password
+                                will only work if MySQL is configured to trust the configured
+                                username without a password.
+                            example: trustsome1
+                        options:
+                            type: str
+                            desc: |
+                                Additional mysqldump options to pass directly to the dump command,
+                                without performing any validation on them. See
+                                https://dev.mysql.com/doc/refman/8.0/en/mysqldump.html or
+                                https://mariadb.com/kb/en/library/mysqldump/ for details.
+                            example: --skip-comments
+                desc: |
+                    List of one or more MySQL/MariaDB databases to dump before creating a backup,
+                    run once per configuration file. The database dumps are added to your source
+                    directories at runtime, backed up, and then removed afterwards. Requires
+                    mysqldump/mysql commands (from either MySQL or MariaDB). See
+                    https://dev.mysql.com/doc/refman/8.0/en/mysqldump.html or
+                    https://mariadb.com/kb/en/library/mysqldump/ for details.
             healthchecks:
                 type: str
                 desc: |
                     Healthchecks ping URL or UUID to notify when a backup begins, ends, or errors.
                     Create an account at https://healthchecks.io if you'd like to use this service.
+                    See
+                    https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#healthchecks-hook
+                    for details.
                 example:
                     https://hc-ping.com/your-uuid-here
             cronitor:
                 type: str
                 desc: |
                     Cronitor ping URL to notify when a backup begins, ends, or errors. Create an
-                    account at https://cronitor.io if you'd like to use this service.
+                    account at https://cronitor.io if you'd like to use this service. See
+                    https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#cronitor-hook
+                    for details.
                 example:
                     https://cronitor.link/d3x0c1
+            cronhub:
+                type: str
+                desc: |
+                    Cronhub ping URL to notify when a backup begins, ends, or errors. Create an
+                    account at https://cronhub.io if you'd like to use this service. See
+                    https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#cronhub-hook for
+                    details.
+                example:
+                    https://cronhub.io/start/1f5e3410-254c-11e8-b61d-55875966d031
             before_everything:
                 seq:
                     - type: str

borgmatic/execute.py

@@ -20,21 +20,17 @@ def exit_code_indicates_error(command, exit_code, error_on_warnings=False):
     return bool(exit_code != 0)
 
 
-def execute_and_log_output(
-    full_command, output_log_level, shell, environment, working_directory, error_on_warnings
-):
+def log_output(command, process, output_buffer, output_log_level, error_on_warnings):
+    '''
+    Given a command already executed, its process opened by subprocess.Popen(), and the process'
+    relevant output buffer (stderr or stdout), log its output with the requested log level.
+    Additionally, raise a CalledProcessException if the process exits with an error (or a warning,
+    if error on warnings is True).
+    '''
     last_lines = []
-    process = subprocess.Popen(
-        full_command,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.STDOUT,
-        shell=shell,
-        env=environment,
-        cwd=working_directory,
-    )
 
     while process.poll() is None:
-        line = process.stdout.readline().rstrip().decode()
+        line = output_buffer.readline().rstrip().decode()
         if not line:
             continue
 
@@ -46,26 +42,26 @@ def execute_and_log_output(
 
         logger.log(output_log_level, line)
 
-    remaining_output = process.stdout.read().rstrip().decode()
+    remaining_output = output_buffer.read().rstrip().decode()
     if remaining_output:  # pragma: no cover
         logger.log(output_log_level, remaining_output)
 
     exit_code = process.poll()
 
-    if exit_code_indicates_error(full_command, exit_code, error_on_warnings):
+    if exit_code_indicates_error(command, exit_code, error_on_warnings):
         # If an error occurs, include its output in the raised exception so that we don't
         # inadvertently hide error output.
         if len(last_lines) == ERROR_OUTPUT_MAX_LINE_COUNT:
             last_lines.insert(0, '...')
 
-        raise subprocess.CalledProcessError(
-            exit_code, ' '.join(full_command), '\n'.join(last_lines)
-        )
+        raise subprocess.CalledProcessError(exit_code, ' '.join(command), '\n'.join(last_lines))
 
 
 def execute_command(
     full_command,
     output_log_level=logging.INFO,
+    output_file=None,
+    input_file=None,
     shell=False,
     extra_environment=None,
     working_directory=None,
@@ -73,14 +69,21 @@ def execute_command(
 ):
     '''
     Execute the given command (a sequence of command/argument strings) and log its output at the
-    given log level. If output log level is None, instead capture and return the output. If
-    shell is True, execute the command within a shell. If an extra environment dict is given, then
-    use it to augment the current environment, and pass the result into the command. If a working
-    directory is given, use that as the present working directory when running the command.
+    given log level. If output log level is None, instead capture and return the output. If an
+    open output file object is given, then write stdout to the file and only log stderr (but only
+    if an output log level is set). If an open input file object is given, then read stdin from the
+    file. If shell is True, execute the command within a shell. If an extra environment dict is
+    given, then use it to augment the current environment, and pass the result into the command. If
+    a working directory is given, use that as the present working directory when running the
+    command.
 
     Raise subprocesses.CalledProcessError if an error occurs while running the command.
     '''
-    logger.debug(' '.join(full_command))
+    logger.debug(
+        ' '.join(full_command)
+        + (' < {}'.format(input_file.name) if input_file else '')
+        + (' > {}'.format(output_file.name) if output_file else '')
+    )
     environment = {**os.environ, **extra_environment} if extra_environment else None
 
     if output_log_level is None:
@@ -89,13 +92,21 @@ def execute_command(
         )
         return output.decode() if output is not None else None
     else:
-        execute_and_log_output(
+        process = subprocess.Popen(
             full_command,
-            output_log_level,
+            stdin=input_file,
+            stdout=output_file or subprocess.PIPE,
+            stderr=subprocess.PIPE if output_file else subprocess.STDOUT,
             shell=shell,
-            environment=environment,
-            working_directory=working_directory,
-            error_on_warnings=error_on_warnings,
+            env=environment,
+            cwd=working_directory,
+        )
+        log_output(
+            full_command,
+            process,
+            process.stderr if output_file else process.stdout,
+            output_log_level,
+            error_on_warnings,
         )
 
 

borgmatic/hooks/cronhub.py

@@ -0,0 +1,32 @@
+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_monitor(ping_url, config_filename, state, dry_run):
+    '''
+    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.
+    '''
+    dry_run_label = ' (dry run; not actually pinging)' if dry_run else ''
+    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.name.lower(), dry_run_label)
+    )
+    logger.debug('{}: Using Cronhub ping URL {}'.format(config_filename, ping_url))
+
+    if not dry_run:
+        logging.getLogger('urllib3').setLevel(logging.ERROR)
+        requests.get(ping_url)

borgmatic/hooks/cronitor.py

@@ -2,23 +2,30 @@ 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))
 
-    logging.getLogger('urllib3').setLevel(logging.ERROR)
-    requests.get(ping_url)
+    if not dry_run:
+        logging.getLogger('urllib3').setLevel(logging.ERROR)
+        requests.get(ping_url)

borgmatic/hooks/dispatch.py

@@ -0,0 +1,61 @@
+import logging
+
+from borgmatic.hooks import cronhub, cronitor, healthchecks, mysql, postgresql
+
+logger = logging.getLogger(__name__)
+
+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):
+    '''
+    Given the hooks configuration dict and a prefix to use in log entries, call the requested
+    function of the Python module corresponding to the given hook name. Supply that call with the
+    configuration for this hook, the log prefix, and any given args and kwargs. Return any return
+    value.
+
+    If the hook name is not present in the hooks configuration, then bail without calling anything.
+
+    Raise ValueError if the hook name is unknown.
+    Raise AttributeError if the function name is not found in the module.
+    Raise anything else that the called function raises.
+    '''
+    config = hooks.get(hook_name)
+    if not config:
+        logger.debug('{}: No {} hook configured.'.format(log_prefix, hook_name))
+        return
+
+    try:
+        module = HOOK_NAME_TO_MODULE[hook_name]
+    except KeyError:
+        raise ValueError('Unknown hook name: {}'.format(hook_name))
+
+    logger.debug('{}: Calling {} hook function {}'.format(log_prefix, hook_name, function_name))
+    return getattr(module, function_name)(config, log_prefix, *args, **kwargs)
+
+
+def call_hooks(function_name, hooks, log_prefix, hook_names, *args, **kwargs):
+    '''
+    Given the hooks configuration dict and a prefix to use in log entries, call the requested
+    function of the Python module corresponding to each given hook name. Supply each call with the
+    configuration for that hook, the log prefix, and any given args and kwargs. Collect any return
+    values into a dict from hook name to return value.
+
+    If the hook name is not present in the hooks configuration, then don't call the function for it,
+    and omit it from the return values.
+
+    Raise ValueError if the hook name is unknown.
+    Raise AttributeError if the function name is not found in the module.
+    Raise anything else that a called function raises. An error stops calls to subsequent functions.
+    '''
+    return {
+        hook_name: call_hook(function_name, hooks, log_prefix, hook_name, *args, **kwargs)
+        for hook_name in hook_names
+        if hook_name in hooks
+    }

borgmatic/hooks/dump.py

@@ -0,0 +1,162 @@
+import glob
+import logging
+import os
+
+logger = logging.getLogger(__name__)
+
+DATABASE_HOOK_NAMES = ('postgresql_databases', 'mysql_databases')
+
+
+def make_database_dump_filename(dump_path, name, hostname=None):
+    '''
+    Based on the given dump directory path, database name, and hostname, return a filename to use
+    for the database dump. The hostname defaults to localhost.
+
+    Raise ValueError if the database name is invalid.
+    '''
+    if os.path.sep in name:
+        raise ValueError('Invalid database name {}'.format(name))
+
+    return os.path.join(os.path.expanduser(dump_path), hostname or 'localhost', name)
+
+
+def flatten_dump_patterns(dump_patterns, names):
+    '''
+    Given a dict from a database hook name to glob patterns matching the dumps for the named
+    databases, flatten out all the glob patterns into a single sequence, and return it.
+
+    Raise ValueError if there are no resulting glob patterns, which indicates that databases are not
+    configured in borgmatic's configuration.
+    '''
+    flattened = [pattern for patterns in dump_patterns.values() for pattern in patterns]
+
+    if not flattened:
+        raise ValueError(
+            'Cannot restore database(s) {} missing from borgmatic\'s configuration'.format(
+                ', '.join(names) or '"all"'
+            )
+        )
+
+    return flattened
+
+
+def remove_database_dumps(dump_path, databases, database_type_name, log_prefix, dry_run):
+    '''
+    Remove the database dumps for the given databases in the dump directory path. The databases are
+    supplied as a sequence of dicts, one dict describing each database as per the configuration
+    schema. Use the name of the database type and the log prefix in any log entries. If this is a
+    dry run, then don't actually remove anything.
+    '''
+    if not databases:
+        logger.debug('{}: No {} databases configured'.format(log_prefix, database_type_name))
+        return
+
+    dry_run_label = ' (dry run; not actually removing anything)' if dry_run else ''
+
+    logger.info(
+        '{}: Removing {} database dumps{}'.format(log_prefix, database_type_name, dry_run_label)
+    )
+
+    for database in databases:
+        dump_filename = make_database_dump_filename(
+            dump_path, database['name'], database.get('hostname')
+        )
+
+        logger.debug(
+            '{}: Removing {} database dump {} from {}{}'.format(
+                log_prefix, database_type_name, database['name'], dump_filename, dry_run_label
+            )
+        )
+        if dry_run:
+            continue
+
+        os.remove(dump_filename)
+        dump_path = os.path.dirname(dump_filename)
+
+        if len(os.listdir(dump_path)) == 0:
+            os.rmdir(dump_path)
+
+
+def convert_glob_patterns_to_borg_patterns(patterns):
+    '''
+    Convert a sequence of shell glob patterns like "/etc/*" to the corresponding Borg archive
+    patterns like "sh:etc/*".
+    '''
+    return ['sh:{}'.format(pattern.lstrip(os.path.sep)) for pattern in patterns]
+
+
+def get_database_names_from_dumps(patterns):
+    '''
+    Given a sequence of database dump patterns, find the corresponding database dumps on disk and
+    return the database names from their filenames.
+    '''
+    return [os.path.basename(dump_path) for pattern in patterns for dump_path in glob.glob(pattern)]
+
+
+def get_database_configurations(databases, names):
+    '''
+    Given the full database configuration dicts as per the configuration schema, and a sequence of
+    database names, filter down and yield the configuration for just the named databases.
+    Additionally, if a database configuration is named "all", project out that configuration for
+    each named database.
+    '''
+    named_databases = {database['name']: database for database in databases}
+
+    for name in names:
+        database = named_databases.get(name)
+        if database:
+            yield database
+            continue
+
+        if 'all' in named_databases:
+            yield {**named_databases['all'], **{'name': name}}
+            continue
+
+
+def get_per_hook_database_configurations(hooks, names, dump_patterns):
+    '''
+    Given the hooks configuration dict as per the configuration schema, a sequence of database
+    names to restore, and a dict from database hook name to glob patterns for matching dumps,
+    filter down the configuration for just the named databases.
+
+    If there are no named databases given, then find the corresponding database dumps on disk and
+    use the database names from their filenames. Additionally, if a database configuration is named
+    "all", project out that configuration for each named database.
+
+    Return the results as a dict from database hook name to a sequence of database configuration
+    dicts for that database type.
+
+    Raise ValueError if one of the database names cannot be matched to a database in borgmatic's
+    database configuration.
+    '''
+    hook_databases = {
+        hook_name: list(
+            get_database_configurations(
+                hooks.get(hook_name),
+                names or get_database_names_from_dumps(dump_patterns[hook_name]),
+            )
+        )
+        for hook_name in DATABASE_HOOK_NAMES
+        if hook_name in hooks
+    }
+
+    if not names or 'all' in names:
+        if not any(hook_databases.values()):
+            raise ValueError(
+                'Cannot restore database "all", as there are no database dumps in the archive'
+            )
+
+        return hook_databases
+
+    found_names = {
+        database['name'] for databases in hook_databases.values() for database in databases
+    }
+    missing_names = sorted(set(names) - found_names)
+    if missing_names:
+        raise ValueError(
+            'Cannot restore database(s) {} missing from borgmatic\'s configuration'.format(
+                ', '.join(missing_names)
+            )
+        )
+
+    return hook_databases

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,15 +26,15 @@ 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:
-        ping_url = '{}/{}'.format(ping_url, append)
+    healthchecks_state = MONITOR_STATE_TO_HEALTHCHECKS.get(state)
+    if healthchecks_state:
+        ping_url = '{}/{}'.format(ping_url, healthchecks_state)
 
     logger.info(
-        '{}: Pinging Healthchecks{}{}'.format(
-            config_filename, ' ' + append if append else '', dry_run_label
-        )
+        '{}: Pinging Healthchecks {}{}'.format(config_filename, state.name.lower(), dry_run_label)
     )
     logger.debug('{}: Using Healthchecks ping URL {}'.format(config_filename, ping_url))
 
-    logging.getLogger('urllib3').setLevel(logging.ERROR)
-    requests.get(ping_url)
+    if not dry_run:
+        logging.getLogger('urllib3').setLevel(logging.ERROR)
+        requests.get(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

borgmatic/hooks/mysql.py

@@ -0,0 +1,94 @@
+import logging
+import os
+
+from borgmatic.execute import execute_command
+from borgmatic.hooks import dump
+
+DUMP_PATH = '~/.borgmatic/mysql_databases'
+logger = logging.getLogger(__name__)
+
+
+def dump_databases(databases, log_prefix, dry_run):
+    '''
+    Dump the given MySQL/MariaDB databases to disk. The databases are supplied as a sequence of
+    dicts, one dict describing each database as per the configuration schema. Use the given log
+    prefix in any log entries. If this is a dry run, then don't actually dump anything.
+    '''
+    dry_run_label = ' (dry run; not actually dumping anything)' if dry_run else ''
+
+    logger.info('{}: Dumping MySQL databases{}'.format(log_prefix, dry_run_label))
+
+    for database in databases:
+        name = database['name']
+        dump_filename = dump.make_database_dump_filename(DUMP_PATH, name, database.get('hostname'))
+        command = (
+            ('mysqldump', '--add-drop-database')
+            + (('--host', database['hostname']) if 'hostname' in database else ())
+            + (('--port', str(database['port'])) if 'port' in database else ())
+            + (('--protocol', 'tcp') if 'hostname' in database or 'port' in database else ())
+            + (('--user', database['username']) if 'username' in database else ())
+            + (tuple(database['options'].split(' ')) if 'options' in database else ())
+            + (('--all-databases',) if name == 'all' else ('--databases', name))
+        )
+        extra_environment = {'MYSQL_PWD': database['password']} if 'password' in database else None
+
+        logger.debug(
+            '{}: Dumping MySQL database {} to {}{}'.format(
+                log_prefix, name, dump_filename, dry_run_label
+            )
+        )
+        if not dry_run:
+            os.makedirs(os.path.dirname(dump_filename), mode=0o700, exist_ok=True)
+            execute_command(
+                command, output_file=open(dump_filename, 'w'), extra_environment=extra_environment
+            )
+
+
+def remove_database_dumps(databases, log_prefix, dry_run):  # pragma: no cover
+    '''
+    Remove the database dumps for the given databases. The databases are supplied as a sequence of
+    dicts, one dict describing each database as per the configuration schema. Use the log prefix in
+    any log entries. If this is a dry run, then don't actually remove anything.
+    '''
+    dump.remove_database_dumps(DUMP_PATH, databases, 'MySQL', log_prefix, dry_run)
+
+
+def make_database_dump_patterns(databases, log_prefix, names):
+    '''
+    Given a sequence of configurations dicts, a prefix to log with, and a sequence of database
+    names to match, return the corresponding glob patterns to match the database dumps in an
+    archive. An empty sequence of names indicates that the patterns should match all dumps.
+    '''
+    return [
+        dump.make_database_dump_filename(DUMP_PATH, name, hostname='*') for name in (names or ['*'])
+    ]
+
+
+def restore_database_dumps(databases, log_prefix, dry_run):
+    '''
+    Restore the given MySQL/MariaDB databases from disk. The databases are supplied as a sequence of
+    dicts, one dict describing each database as per the configuration schema. Use the given log
+    prefix in any log entries. If this is a dry run, then don't actually restore anything.
+    '''
+    dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
+
+    for database in databases:
+        dump_filename = dump.make_database_dump_filename(
+            DUMP_PATH, database['name'], database.get('hostname')
+        )
+        restore_command = (
+            ('mysql', '--batch')
+            + (('--host', database['hostname']) if 'hostname' in database else ())
+            + (('--port', str(database['port'])) if 'port' in database else ())
+            + (('--protocol', 'tcp') if 'hostname' in database or 'port' in database else ())
+            + (('--user', database['username']) if 'username' in database else ())
+        )
+        extra_environment = {'MYSQL_PWD': database['password']} if 'password' in database else None
+
+        logger.debug(
+            '{}: Restoring MySQL database {}{}'.format(log_prefix, database['name'], dry_run_label)
+        )
+        if not dry_run:
+            execute_command(
+                restore_command, input_file=open(dump_filename), extra_environment=extra_environment
+            )

borgmatic/hooks/postgresql.py

@@ -1,42 +1,26 @@
-import glob
 import logging
 import os
 
 from borgmatic.execute import execute_command
+from borgmatic.hooks import dump
 
 DUMP_PATH = '~/.borgmatic/postgresql_databases'
 logger = logging.getLogger(__name__)
 
 
-def make_database_dump_filename(name, hostname=None):
-    '''
-    Based on the given database name and hostname, return a filename to use for the database dump.
-
-    Raise ValueError if the database name is invalid.
-    '''
-    if os.path.sep in name:
-        raise ValueError('Invalid database name {}'.format(name))
-
-    return os.path.join(os.path.expanduser(DUMP_PATH), hostname or 'localhost', name)
-
-
 def dump_databases(databases, log_prefix, dry_run):
     '''
     Dump the given PostgreSQL databases to disk. The databases are supplied as a sequence of dicts,
     one dict describing each database as per the configuration schema. Use the given log prefix in
     any log entries. If this is a dry run, then don't actually dump anything.
     '''
-    if not databases:
-        logger.debug('{}: No PostgreSQL databases configured'.format(log_prefix))
-        return
-
     dry_run_label = ' (dry run; not actually dumping anything)' if dry_run else ''
 
     logger.info('{}: Dumping PostgreSQL databases{}'.format(log_prefix, dry_run_label))
 
     for database in databases:
         name = database['name']
-        dump_filename = make_database_dump_filename(name, database.get('hostname'))
+        dump_filename = dump.make_database_dump_filename(DUMP_PATH, name, database.get('hostname'))
         all_databases = bool(name == 'all')
         command = (
             ('pg_dumpall' if all_databases else 'pg_dump', '--no-password', '--clean')
@@ -50,96 +34,34 @@ def dump_databases(databases, log_prefix, dry_run):
         )
         extra_environment = {'PGPASSWORD': database['password']} if 'password' in database else None
 
-        logger.debug('{}: Dumping PostgreSQL database {}{}'.format(log_prefix, name, dry_run_label))
+        logger.debug(
+            '{}: Dumping PostgreSQL database {} to {}{}'.format(
+                log_prefix, name, dump_filename, dry_run_label
+            )
+        )
         if not dry_run:
             os.makedirs(os.path.dirname(dump_filename), mode=0o700, exist_ok=True)
             execute_command(command, extra_environment=extra_environment)
 
 
-def remove_database_dumps(databases, log_prefix, dry_run):
+def remove_database_dumps(databases, log_prefix, dry_run):  # pragma: no cover
     '''
     Remove the database dumps for the given databases. The databases are supplied as a sequence of
     dicts, one dict describing each database as per the configuration schema. Use the log prefix in
     any log entries. If this is a dry run, then don't actually remove anything.
     '''
-    if not databases:
-        logger.debug('{}: No PostgreSQL databases configured'.format(log_prefix))
-        return
-
-    dry_run_label = ' (dry run; not actually removing anything)' if dry_run else ''
-
-    logger.info('{}: Removing PostgreSQL database dumps{}'.format(log_prefix, dry_run_label))
-
-    for database in databases:
-        dump_filename = make_database_dump_filename(database['name'], database.get('hostname'))
-
-        logger.debug(
-            '{}: Removing PostgreSQL database dump {} from {}{}'.format(
-                log_prefix, database['name'], dump_filename, dry_run_label
-            )
-        )
-        if dry_run:
-            continue
-
-        os.remove(dump_filename)
-        dump_path = os.path.dirname(dump_filename)
-
-        if len(os.listdir(dump_path)) == 0:
-            os.rmdir(dump_path)
+    dump.remove_database_dumps(DUMP_PATH, databases, 'PostgreSQL', log_prefix, dry_run)
 
 
-def make_database_dump_patterns(names):
+def make_database_dump_patterns(databases, log_prefix, names):
     '''
-    Given a sequence of database names, return the corresponding glob patterns to match the database
-    dumps in an archive. An empty sequence of names indicates that the patterns should match all
-    dumps.
+    Given a sequence of configurations dicts, a prefix to log with, and a sequence of database
+    names to match, return the corresponding glob patterns to match the database dumps in an
+    archive. An empty sequence of names indicates that the patterns should match all dumps.
     '''
-    return [make_database_dump_filename(name, hostname='*') for name in (names or ['*'])]
-
-
-def convert_glob_patterns_to_borg_patterns(patterns):
-    '''
-    Convert a sequence of shell glob patterns like "/etc/*" to the corresponding Borg archive
-    patterns like "sh:etc/*".
-    '''
-    return ['sh:{}'.format(pattern.lstrip(os.path.sep)) for pattern in patterns]
-
-
-def get_database_names_from_dumps(patterns):
-    '''
-    Given a sequence of database dump patterns, find the corresponding database dumps on disk and
-    return the database names from their filenames.
-    '''
-    return [os.path.basename(dump_path) for pattern in patterns for dump_path in glob.glob(pattern)]
-
-
-def get_database_configurations(databases, names):
-    '''
-    Given the full database configuration dicts as per the configuration schema, and a sequence of
-    database names, filter down and yield the configuration for just the named databases.
-    Additionally, if a database configuration is named "all", project out that configuration for
-    each named database.
-
-    Raise ValueError if one of the database names cannot be matched to a database in borgmatic's
-    database configuration.
-    '''
-    named_databases = {database['name']: database for database in databases}
-
-    for name in names:
-        database = named_databases.get(name)
-        if database:
-            yield database
-            continue
-
-        if 'all' in named_databases:
-            yield {**named_databases['all'], **{'name': name}}
-            continue
-
-        raise ValueError(
-            'Cannot restore database "{}", as it is not defined in borgmatic\'s configuration'.format(
-                name
-            )
-        )
+    return [
+        dump.make_database_dump_filename(DUMP_PATH, name, hostname='*') for name in (names or ['*'])
+    ]
 
 
 def restore_database_dumps(databases, log_prefix, dry_run):
@@ -148,14 +70,12 @@ def restore_database_dumps(databases, log_prefix, dry_run):
     dicts, one dict describing each database as per the configuration schema. Use the given log
     prefix in any log entries. If this is a dry run, then don't actually restore anything.
     '''
-    if not databases:
-        logger.debug('{}: No PostgreSQL databases configured'.format(log_prefix))
-        return
-
     dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
 
     for database in databases:
-        dump_filename = make_database_dump_filename(database['name'], database.get('hostname'))
+        dump_filename = dump.make_database_dump_filename(
+            DUMP_PATH, database['name'], database.get('hostname')
+        )
         restore_command = (
             ('pg_restore', '--no-password', '--clean', '--if-exists', '--exit-on-error')
             + (('--host', database['hostname']) if 'hostname' in database else ())

borgmatic/logger.py

@@ -104,7 +104,7 @@ def configure_logging(
         syslog_handler.setLevel(syslog_log_level)
         handlers = (console_handler, syslog_handler)
     elif log_file:
-        file_handler = logging.FileHandler(log_file)
+        file_handler = logging.handlers.WatchedFileHandler(log_file)
         file_handler.setFormatter(logging.Formatter('[%(asctime)s] %(levelname)s: %(message)s'))
         file_handler.setLevel(log_file_log_level)
         handlers = (console_handler, file_handler)

docs/how-to/backup-your-databases.md

@@ -11,31 +11,40 @@ consistent snapshot that is more suited for backups.
 
 Fortunately, borgmatic includes built-in support for creating database dumps
 prior to running backups. For example, here is everything you need to dump and
-backup a couple of local PostgreSQL databases:
+backup a couple of local PostgreSQL databases and a MySQL/MariaDB database:
 
 ```yaml
 hooks:
     postgresql_databases:
         - name: users
         - name: orders
+    mysql_databases:
+        - name: posts
 ```
 
 Prior to each backup, borgmatic dumps each configured database to a file
 (located in `~/.borgmatic/`) and includes it in the backup. After the backup
 completes, borgmatic removes the database dump files to recover disk space.
 
-Here's a more involved example that connects to a remote database:
+Here's a more involved example that connects to remote databases:
 
 ```yaml
 hooks:
     postgresql_databases:
         - name: users
-          hostname: database.example.org
+          hostname: database1.example.org
           port: 5433
-          username: dbuser
+          username: postgres
           password: trustsome1
           format: tar
           options: "--role=someone"
+    mysql_databases:
+        - name: posts
+          hostname: database2.example.org
+          port: 3307
+          username: root
+          password: trustsome1
+          options: "--skip-comments"
 ```
 
 If you want to dump all databases on a host, use `all` for the database name:
@@ -44,10 +53,12 @@ If you want to dump all databases on a host, use `all` for the database name:
 hooks:
     postgresql_databases:
         - name: all
+    mysql_databases:
+        - name: all
 ```
 
 Note that you may need to use a `username` of the `postgres` superuser for
-this to work.
+this to work with PostgreSQL.
 
 
 ### Configuration backups
@@ -61,9 +72,9 @@ bring back any missing configuration files in order to restore a database.
 
 ## Supported databases
 
-As of now, borgmatic only supports PostgreSQL databases directly. But see
-below about general-purpose preparation and cleanup hooks as a work-around
-with other database systems. Also, please [file a
+As of now, borgmatic supports PostgreSQL and MySQL/MariaDB databases
+directly. But see below about general-purpose preparation and cleanup hooks as
+a work-around with other database systems. Also, please [file a
 ticket](https://torsion.org/borgmatic/#issues) for additional database systems
 that you'd like supported.
 
@@ -148,7 +159,8 @@ databases that share the exact same name on different hosts.
 If you prefer to restore a database without the help of borgmatic, first
 [extract](https://torsion.org/borgmatic/docs/how-to/extract-a-backup/) an
 archive containing a database dump, and then manually restore the dump file
-found within the extracted `~/.borgmatic/` path (e.g. with `pg_restore`).
+found within the extracted `~/.borgmatic/` path (e.g. with `pg_restore` or
+`mysql` commands).
 
 
 ## Preparation and cleanup hooks

docs/how-to/inspect-your-backups.md

@@ -70,6 +70,21 @@ Or to increase syslog logging to include debug spew:
 borgmatic --syslog-verbosity 2
 ```
 
+### Rate limiting
+
+If you are using rsyslog or systemd's journal, be aware that by default they
+both throttle the rate at which logging occurs. So you may need to change
+either [the global rate
+limit](https://www.rootusers.com/how-to-change-log-rate-limiting-in-linux/) or
+[the per-service rate
+limit](https://www.freedesktop.org/software/systemd/man/journald.conf.html#RateLimitIntervalSec=)
+if you're finding that borgmatic logs are missing.
+
+Note that the [sample borgmatic systemd service
+file](https://torsion.org/borgmatic/docs/how-to/set-up-backups/#systemd)
+already has this rate limit disabled for systemd's journal.
+
+
 ### Logging to file
 
 If you don't want to use syslog, and you'd rather borgmatic log to a plain
@@ -84,20 +99,6 @@ the log file so it doesn't grow too large. Also, there is a
 `--log-file-verbosity` flag to customize the log file's log level.
 
 
-### systemd journal
-
-If your local syslog daemon is systemd's journal, be aware that journald by
-default throttles the rate at which a particular program can log. So you may
-need to [change the journald rate
-limit](https://www.freedesktop.org/software/systemd/man/journald.conf.html#RateLimitIntervalSec=)
-in `/etc/systemd/journald.conf` if you're finding that borgmatic journald logs
-are missing.
-
-Note that the [sample borgmatic systemd service
-file](https://torsion.org/borgmatic/docs/how-to/set-up-backups/#systemd)
-already has this rate limit disabled.
-
-
 ## Related documentation
 
  * [Set up backups with borgmatic](https://torsion.org/borgmatic/docs/how-to/set-up-backups/)

docs/how-to/monitor-your-backups.md

@@ -27,14 +27,15 @@ See [error
 hooks](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#error-hooks)
 below for how to configure this.
 4. **borgmatic monitoring hooks**: This feature integrates with monitoring
-services like [Healthchecks](https://healthchecks.io/) and
-[Cronitor](https://cronitor.io), and pings these services whenever borgmatic
-runs. That way, you'll receive an alert when something goes wrong or the
-service doesn't hear from borgmatic for a configured interval. See
+   services like [Healthchecks](https://healthchecks.io/),
+[Cronitor](https://cronitor.io), and [Cronhub](https://cronhub.io), and pings
+these services whenever borgmatic runs. That way, you'll receive an alert when
+something goes wrong or the service doesn't hear from borgmatic for a
+configured interval. See
 [Healthchecks
-hook](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#healthchecks-hook)
-and [Cronitor
-hook](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#cronitor-hook)
+hook](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#healthchecks-hook), [Cronitor
+hook](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#cronitor-hook), and [Cronhub
+hook](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#cronhub-hook)
 below for how to configure this.
 3. **Third-party monitoring software**: You can use traditional monitoring
 software to consume borgmatic JSON output and track when the last
@@ -111,9 +112,15 @@ hooks:
     healthchecks: https://hc-ping.com/addffa72-da17-40ae-be9c-ff591afb942a
 ```
 
-With this hook in place, borgmatic will ping your Healthchecks project when a
-backup begins, ends, or errors. Then you can configure Healthchecks to notify
-you by a [variety of
+With this hook in place, borgmatic pings your Healthchecks project when a
+backup begins, ends, or errors. Specifically, before the <a
+href="https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/">`before_backup`
+hooks</a> run, borgmatic lets Healthchecks know that a backup has started.
+Then, if the backup completes successfully, borgmatic notifies Healthchecks of
+the success after the `after_backup` hooks run. And if an error occurs during
+the backup, borgmatic notifies Healthchecks after the `on_error` hooks run.
+
+You can configure Healthchecks to notify you by a [variety of
 mechanisms](https://healthchecks.io/#welcome-integrations) when backups fail
 or it doesn't hear from borgmatic for a certain period of time.
 
@@ -132,11 +139,48 @@ hooks:
     cronitor: https://cronitor.link/d3x0c1
 ```
 
-With this hook in place, borgmatic will ping your Cronitor monitor when a
-backup begins, ends, or errors. Then you can configure Cronitor to notify you
-by a [variety of
-mechanisms](https://cronitor.io/docs/cron-job-notifications) when backups
-fail or it doesn't hear from borgmatic for a certain period of time.
+With this hook in place, borgmatic pings your Cronitor monitor when a backup
+begins, ends, or errors. Specifically, before the <a
+href="https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/">`before_backup`
+hooks</a> run, borgmatic lets Cronitor know that a backup has started. Then,
+if the backup completes successfully, borgmatic notifies Cronitor of the
+success after the `after_backup` hooks run. And if an error occurs during the
+backup, borgmatic notifies Cronitor after the `on_error` hooks run.
+
+You can configure Cronitor to notify you by a [variety of
+mechanisms](https://cronitor.io/docs/cron-job-notifications) when backups fail
+or it doesn't hear from borgmatic for a certain period of time.
+
+
+## Cronhub hook
+
+[Cronhub](https://cronhub.io/) provides "instant alerts when any of your
+background jobs fail silently or run longer than expected", and borgmatic has
+built-in integration with it. Once you create a Cronhub account and monitor on
+their site, all you need to do is configure borgmatic with the unique "Ping
+URL" for your monitor. Here's an example:
+
+
+```yaml
+hooks:
+    cronhub: https://cronhub.io/start/1f5e3410-254c-11e8-b61d-55875966d031
+```
+
+With this hook in place, borgmatic pings your Cronhub monitor when a backup
+begins, ends, or errors. Specifically, before the <a
+href="https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/">`before_backup`
+hooks</a> run, borgmatic lets Cronhub know that a backup has started. Then,
+if the backup completes successfully, borgmatic notifies Cronhub of the
+success after the `after_backup` hooks run. And if an error occurs during the
+backup, borgmatic notifies Cronhub after the `on_error` hooks run.
+
+Note that even though you configure borgmatic with the "start" variant of the
+ping URL, borgmatic substitutes the correct state into the URL when pinging
+Cronhub ("start", "finish", or "fail").
+
+You can configure Cronhub to notify you by a [variety of
+mechanisms](https://docs.cronhub.io/integrations.html) when backups fail
+or it doesn't hear from borgmatic for a certain period of time.
 
 
 ## Scripting borgmatic

docs/how-to/set-up-backups.md

@@ -19,8 +19,14 @@ sudo pip3 install --user --upgrade borgmatic
 
 This is a [recommended user site
 installation](https://packaging.python.org/tutorials/installing-packages/#installing-to-the-user-site).
-You will need to ensure that `/root/.local/bin` is available on your `$PATH` so
-that the borgmatic executable is available.
+You will need to ensure that `/root/.local/bin` is available on your `$PATH`
+so
+that the borgmatic executable is available. For instance, adding this to
+root's `~/.profile` or `~/.bash_profile` may do the trick:
+
+```bash
+export PATH="$PATH:~/.local/bin"
+```
 
 Note that your pip binary may have a different name than "pip3". Make sure
 you're using Python 3, as borgmatic does not support Python 2.
@@ -63,7 +69,7 @@ sudo generate-borgmatic-config
 ```
 
 If that command is not found, then it may be installed in a location that's
-not in your system `PATH`. Try looking in `/usr/local/bin/`.
+not in your system `PATH` (see above). Try looking in `~/.local/bin/`.
 
 This generates a sample configuration file at /etc/borgmatic/config.yaml (by
 default). You should edit the file to suit your needs, as the values are
@@ -77,9 +83,12 @@ else borgmatic won't recognize the option. Also be sure to use spaces rather
 than tabs for indentation; YAML does not allow tabs.
 
 You can also get the same sample configuration file from the [configuration
-reference](https://torsion.org/borgmatic/docs/reference/configuration/), the authoritative set of
-all configuration options. This is handy if borgmatic has added new options
-since you originally created your configuration file.
+reference](https://torsion.org/borgmatic/docs/reference/configuration/), the
+authoritative set of all configuration options. This is handy if borgmatic has
+added new options
+since you originally created your configuration file. Also check out how to
+[upgrade your
+configuration](https://torsion.org/borgmatic/docs/how-to/upgrade/#upgrading-your-configuration).
 
 
 ### Encryption
@@ -248,5 +257,6 @@ it.
  * [Deal with very large backups](https://torsion.org/borgmatic/docs/how-to/deal-with-very-large-backups/)
  * [Inspect your backups](https://torsion.org/borgmatic/docs/how-to/inspect-your-backups/)
  * [Monitor your backups](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/)
+ * [Upgrade borgmatic](https://torsion.org/borgmatic/docs/how-to/upgrade/)
  * [borgmatic configuration reference](https://torsion.org/borgmatic/docs/reference/configuration/)
  * [borgmatic command-line reference](https://torsion.org/borgmatic/docs/reference/command-line/)

docs/how-to/upgrade.md

@@ -10,7 +10,46 @@ following:
 sudo pip3 install --user --upgrade borgmatic
 ```
 
-See below about special cases.
+See below about special cases with old versions of borgmatic. Additionally, if
+you installed borgmatic [without using `pip3 install
+--user`](https://torsion.org/borgmatic/docs/how-to/set-up-backups/#other-ways-to-install),
+then your upgrade process may be different.
+
+
+### Upgrading your configuration
+
+The borgmatic configuration file format is almost always backwards-compatible
+from release to release without any changes, but you may still want to update
+your configuration file when you upgrade to take advantage of new
+configuration options. This is completely optional. If you prefer, you can add
+new configuration options manually.
+
+If you do want to upgrade your configuration file to include new options, use
+the `generate-borgmatic-config` script with its optional `--source` flag that
+takes the path to your original configuration file. If provided with this
+path, `generate-borgmatic-config` merges your original configuration into the
+generated configuration file, so you get all the newest options and comments.
+
+Here's an example:
+
+```bash
+generate-borgmatic-config --source config.yaml --destination config-new.yaml
+```
+
+New options start as commented out, so you can edit the file and decide
+whether you want to use each one.
+
+There are a few caveats to this process. First, when generating the new
+configuration file, `generate-borgmatic-config` replaces any comments you've
+written in your original configuration file with the newest generated
+comments. Second, the script adds back any options you had originally deleted,
+although it does so with the options commented out. And finally, any YAML
+includes you've used in the source configuration get flattened out into a
+single generated file.
+
+As a safety measure, `generate-borgmatic-config` refuses to modify
+configuration files in-place. So it's up to you to review the generated file
+and, if desired, replace your original configuration file with it.
 
 
 ### Upgrading from borgmatic 1.0.x

setup.py

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

tests/integration/config/test_generate.py

@@ -47,9 +47,13 @@ def test_comment_out_line_comments_twice_indented_option():
 
 
 def test_comment_out_optional_configuration_comments_optional_config_only():
+    # The "# COMMENT_OUT" comment is a sentinel used to express that the following key is optional.
+    # It's stripped out of the final output.
     flexmock(module)._comment_out_line = lambda line: '# ' + line
     config = '''
+# COMMENT_OUT
 foo:
+    # COMMENT_OUT
     bar:
         - baz
         - quux
@@ -59,6 +63,8 @@ location:
         - one
         - two
 
+    # This comment should be kept.
+    # COMMENT_OUT
     other: thing
     '''
 
@@ -68,12 +74,13 @@ location:
 #     bar:
 #         - baz
 #         - quux
-# 
+
 location:
     repositories:
         - one
         - two
-# 
+
+    # This comment should be kept.
 #     other: thing
     '''
 
@@ -142,12 +149,67 @@ def test_add_comments_to_configuration_map_does_not_raise():
     module.add_comments_to_configuration_map(config, schema)
 
 
+def test_add_comments_to_configuration_map_with_skip_first_does_not_raise():
+    config = module.yaml.comments.CommentedMap([('foo', 33)])
+    schema = {'map': {'foo': {'desc': 'Foo'}}}
+
+    module.add_comments_to_configuration_map(config, schema, skip_first=True)
+
+
+def test_remove_commented_out_sentinel_keeps_other_comments():
+    field_name = 'foo'
+    config = module.yaml.comments.CommentedMap([(field_name, 33)])
+    config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.\nCOMMENT_OUT')
+
+    module.remove_commented_out_sentinel(config, field_name)
+
+    comments = config.ca.items[field_name][module.RUAMEL_YAML_COMMENTS_INDEX]
+    assert len(comments) == 1
+    assert comments[0].value == '# Actual comment.\n'
+
+
+def test_remove_commented_out_sentinel_without_sentinel_keeps_other_comments():
+    field_name = 'foo'
+    config = module.yaml.comments.CommentedMap([(field_name, 33)])
+    config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.')
+
+    module.remove_commented_out_sentinel(config, field_name)
+
+    comments = config.ca.items[field_name][module.RUAMEL_YAML_COMMENTS_INDEX]
+    assert len(comments) == 1
+    assert comments[0].value == '# Actual comment.\n'
+
+
+def test_remove_commented_out_sentinel_on_unknown_field_does_not_raise():
+    field_name = 'foo'
+    config = module.yaml.comments.CommentedMap([(field_name, 33)])
+    config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.')
+
+    module.remove_commented_out_sentinel(config, 'unknown')
+
+
 def test_generate_sample_configuration_does_not_raise():
     builtins = flexmock(sys.modules['builtins'])
     builtins.should_receive('open').with_args('schema.yaml').and_return('')
+    flexmock(module.yaml).should_receive('round_trip_load')
     flexmock(module).should_receive('_schema_to_sample_configuration')
+    flexmock(module).should_receive('merge_source_configuration_into_destination')
     flexmock(module).should_receive('_render_configuration')
     flexmock(module).should_receive('_comment_out_optional_configuration')
     flexmock(module).should_receive('write_configuration')
 
-    module.generate_sample_configuration('config.yaml', 'schema.yaml')
+    module.generate_sample_configuration(None, 'dest.yaml', 'schema.yaml')
+
+
+def test_generate_sample_configuration_with_source_filename_does_not_raise():
+    builtins = flexmock(sys.modules['builtins'])
+    builtins.should_receive('open').with_args('schema.yaml').and_return('')
+    flexmock(module.yaml).should_receive('round_trip_load')
+    flexmock(module.load).should_receive('load_configuration')
+    flexmock(module).should_receive('_schema_to_sample_configuration')
+    flexmock(module).should_receive('merge_source_configuration_into_destination')
+    flexmock(module).should_receive('_render_configuration')
+    flexmock(module).should_receive('_comment_out_optional_configuration')
+    flexmock(module).should_receive('write_configuration')
+
+    module.generate_sample_configuration('source.yaml', 'dest.yaml', 'schema.yaml')

tests/integration/test_execute.py

@@ -7,40 +7,42 @@ from flexmock import flexmock
 from borgmatic import execute as module
 
 
-def test_execute_and_log_output_logs_each_line_separately():
+def test_log_output_logs_each_line_separately():
     flexmock(module.logger).should_receive('log').with_args(logging.INFO, 'hi').once()
     flexmock(module.logger).should_receive('log').with_args(logging.INFO, 'there').once()
     flexmock(module).should_receive('exit_code_indicates_error').and_return(False)
 
-    module.execute_and_log_output(
+    hi_process = subprocess.Popen(['echo', 'hi'], stdout=subprocess.PIPE)
+    module.log_output(
         ['echo', 'hi'],
+        hi_process,
+        hi_process.stdout,
         output_log_level=logging.INFO,
-        shell=False,
-        environment=None,
-        working_directory=None,
         error_on_warnings=False,
     )
-    module.execute_and_log_output(
+
+    there_process = subprocess.Popen(['echo', 'there'], stdout=subprocess.PIPE)
+    module.log_output(
         ['echo', 'there'],
+        there_process,
+        there_process.stdout,
         output_log_level=logging.INFO,
-        shell=False,
-        environment=None,
-        working_directory=None,
         error_on_warnings=False,
     )
 
 
-def test_execute_and_log_output_includes_error_output_in_exception():
+def test_log_output_includes_error_output_in_exception():
     flexmock(module.logger).should_receive('log')
     flexmock(module).should_receive('exit_code_indicates_error').and_return(True)
 
+    process = subprocess.Popen(['grep'], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+
     with pytest.raises(subprocess.CalledProcessError) as error:
-        module.execute_and_log_output(
+        module.log_output(
             ['grep'],
+            process,
+            process.stdout,
             output_log_level=logging.INFO,
-            shell=False,
-            environment=None,
-            working_directory=None,
             error_on_warnings=False,
         )
 
@@ -48,18 +50,19 @@ def test_execute_and_log_output_includes_error_output_in_exception():
     assert error.value.output
 
 
-def test_execute_and_log_output_truncates_long_error_output():
+def test_log_output_truncates_long_error_output():
     flexmock(module).ERROR_OUTPUT_MAX_LINE_COUNT = 0
     flexmock(module.logger).should_receive('log')
     flexmock(module).should_receive('exit_code_indicates_error').and_return(True)
 
+    process = subprocess.Popen(['grep'], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+
     with pytest.raises(subprocess.CalledProcessError) as error:
-        module.execute_and_log_output(
+        module.log_output(
             ['grep'],
+            process,
+            process.stdout,
             output_log_level=logging.INFO,
-            shell=False,
-            environment=None,
-            working_directory=None,
             error_on_warnings=False,
         )
 
@@ -67,15 +70,11 @@ def test_execute_and_log_output_truncates_long_error_output():
     assert error.value.output.startswith('...')
 
 
-def test_execute_and_log_output_with_no_output_logs_nothing():
+def test_log_output_with_no_output_logs_nothing():
     flexmock(module.logger).should_receive('log').never()
     flexmock(module).should_receive('exit_code_indicates_error').and_return(False)
 
-    module.execute_and_log_output(
-        ['true'],
-        output_log_level=logging.INFO,
-        shell=False,
-        environment=None,
-        working_directory=None,
-        error_on_warnings=False,
+    process = subprocess.Popen(['true'], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+    module.log_output(
+        ['true'], process, process.stdout, output_log_level=logging.INFO, error_on_warnings=False
     )

tests/unit/borg/test_extract.py

@@ -236,3 +236,19 @@ def test_extract_archive_calls_borg_with_progress_parameter():
         storage_config={},
         progress=True,
     )
+
+
+def test_extract_archive_skips_abspath_for_remote_repository():
+    flexmock(module.os.path).should_receive('abspath').never()
+    flexmock(module).should_receive('execute_command').with_args(
+        ('borg', 'extract', 'server:repo::archive'), working_directory=None, error_on_warnings=True
+    ).once()
+
+    module.extract_archive(
+        dry_run=False,
+        repository='server:repo',
+        archive='archive',
+        paths=None,
+        location_config={},
+        storage_config={},
+    )

tests/unit/commands/test_borgmatic.py

@@ -23,9 +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.postgresql).should_receive('dump_databases').once()
-    flexmock(module.healthchecks).should_receive('ping_healthchecks').twice()
-    flexmock(module.postgresql).should_receive('remove_database_dumps').once()
+    flexmock(module.dispatch).should_receive('call_hooks').at_least().twice()
     flexmock(module).should_receive('run_actions').and_return([])
     config = {'location': {'repositories': ['foo']}}
     arguments = {'global': flexmock(dry_run=False), 'create': flexmock()}
@@ -36,8 +34,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.command).should_receive('execute_hook')
-    flexmock(module.postgresql).should_receive('dump_databases')
-    flexmock(module.healthchecks).should_receive('ping_healthchecks')
+    flexmock(module.dispatch).should_receive('call_hooks')
     expected_results = [flexmock()]
     flexmock(module).should_receive('make_error_log_records').and_return(expected_results)
     flexmock(module).should_receive('run_actions').and_raise(OSError)
@@ -68,6 +65,7 @@ def test_run_configuration_logs_post_hook_error():
     flexmock(module.command).should_receive('execute_hook').and_return(None).and_raise(
         OSError
     ).and_return(None)
+    flexmock(module.dispatch).should_receive('call_hooks')
     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/config/test_convert.py

@@ -21,6 +21,7 @@ def test_convert_section_generates_integer_value_for_integer_type_in_schema():
 
 def test_convert_legacy_parsed_config_transforms_source_config_to_mapping():
     flexmock(module.yaml.comments).should_receive('CommentedMap').replace_with(OrderedDict)
+    flexmock(module.generate).should_receive('add_comments_to_configuration_map')
     source_config = Parsed_config(
         location=OrderedDict([('source_directories', '/home'), ('repository', 'hostname.borg')]),
         storage=OrderedDict([('encryption_passphrase', 'supersecret')]),
@@ -53,6 +54,7 @@ def test_convert_legacy_parsed_config_transforms_source_config_to_mapping():
 
 def test_convert_legacy_parsed_config_splits_space_separated_values():
     flexmock(module.yaml.comments).should_receive('CommentedMap').replace_with(OrderedDict)
+    flexmock(module.generate).should_receive('add_comments_to_configuration_map')
     source_config = Parsed_config(
         location=OrderedDict(
             [('source_directories', '/home /etc'), ('repository', 'hostname.borg')]

tests/unit/config/test_generate.py

@@ -51,6 +51,7 @@ def test_schema_to_sample_configuration_generates_config_sequence_of_strings_wit
 def test_schema_to_sample_configuration_generates_config_sequence_of_maps_with_examples():
     flexmock(module.yaml.comments).should_receive('CommentedSeq').replace_with(list)
     flexmock(module).should_receive('add_comments_to_configuration_sequence')
+    flexmock(module).should_receive('add_comments_to_configuration_map')
     schema = {
         'seq': [
             {
@@ -71,3 +72,63 @@ def test_schema_to_sample_configuration_with_unsupported_schema_raises():
 
     with pytest.raises(ValueError):
         module._schema_to_sample_configuration(schema)
+
+
+def test_merge_source_configuration_into_destination_inserts_map_fields():
+    destination_config = {'foo': 'dest1', 'bar': 'dest2'}
+    source_config = {'foo': 'source1', 'baz': 'source2'}
+    flexmock(module).should_receive('remove_commented_out_sentinel')
+    flexmock(module).should_receive('yaml.comments.CommentedSeq').replace_with(list)
+
+    module.merge_source_configuration_into_destination(destination_config, source_config)
+
+    assert destination_config == {'foo': 'source1', 'bar': 'dest2', 'baz': 'source2'}
+
+
+def test_merge_source_configuration_into_destination_inserts_nested_map_fields():
+    destination_config = {'foo': {'first': 'dest1', 'second': 'dest2'}, 'bar': 'dest3'}
+    source_config = {'foo': {'first': 'source1'}}
+    flexmock(module).should_receive('remove_commented_out_sentinel')
+    flexmock(module).should_receive('yaml.comments.CommentedSeq').replace_with(list)
+
+    module.merge_source_configuration_into_destination(destination_config, source_config)
+
+    assert destination_config == {'foo': {'first': 'source1', 'second': 'dest2'}, 'bar': 'dest3'}
+
+
+def test_merge_source_configuration_into_destination_inserts_sequence_fields():
+    destination_config = {'foo': ['dest1', 'dest2'], 'bar': ['dest3'], 'baz': ['dest4']}
+    source_config = {'foo': ['source1'], 'bar': ['source2', 'source3']}
+    flexmock(module).should_receive('remove_commented_out_sentinel')
+    flexmock(module).should_receive('yaml.comments.CommentedSeq').replace_with(list)
+
+    module.merge_source_configuration_into_destination(destination_config, source_config)
+
+    assert destination_config == {
+        'foo': ['source1'],
+        'bar': ['source2', 'source3'],
+        'baz': ['dest4'],
+    }
+
+
+def test_merge_source_configuration_into_destination_inserts_sequence_of_maps():
+    destination_config = {'foo': [{'first': 'dest1', 'second': 'dest2'}], 'bar': 'dest3'}
+    source_config = {'foo': [{'first': 'source1'}, {'other': 'source2'}]}
+    flexmock(module).should_receive('remove_commented_out_sentinel')
+    flexmock(module).should_receive('yaml.comments.CommentedSeq').replace_with(list)
+
+    module.merge_source_configuration_into_destination(destination_config, source_config)
+
+    assert destination_config == {
+        'foo': [{'first': 'source1', 'second': 'dest2'}, {'other': 'source2'}],
+        'bar': 'dest3',
+    }
+
+
+def test_merge_source_configuration_into_destination_without_source_does_nothing():
+    original_destination_config = {'foo': 'dest1', 'bar': 'dest2'}
+    destination_config = dict(original_destination_config)
+
+    module.merge_source_configuration_into_destination(destination_config, None)
+
+    assert destination_config == original_destination_config

tests/unit/hooks/test_cronhub.py

@@ -0,0 +1,38 @@
+from flexmock import flexmock
+
+from borgmatic.hooks import cronhub as module
+
+
+def test_ping_monitor_rewrites_ping_url_for_start_state():
+    ping_url = 'https://example.com/start/abcdef'
+    flexmock(module.requests).should_receive('get').with_args('https://example.com/start/abcdef')
+
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=False)
+
+
+def test_ping_monitor_rewrites_ping_url_and_state_for_start_state():
+    ping_url = 'https://example.com/ping/abcdef'
+    flexmock(module.requests).should_receive('get').with_args('https://example.com/start/abcdef')
+
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=False)
+
+
+def test_ping_monitor_rewrites_ping_url_for_finish_state():
+    ping_url = 'https://example.com/start/abcdef'
+    flexmock(module.requests).should_receive('get').with_args('https://example.com/finish/abcdef')
+
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.FINISH, dry_run=False)
+
+
+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_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=True)

tests/unit/hooks/test_cronitor.py

@@ -3,15 +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, append))
+    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, 'run'))
 
-    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():
+    ping_url = 'https://example.com'
+    flexmock(module.requests).should_receive('get').with_args('{}/{}'.format(ping_url, 'complete'))
+
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.FINISH, dry_run=False)
+
+
+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=None, config_filename='config.yaml', dry_run=False, append='oops')
+    module.ping_monitor(ping_url, 'config.yaml', module.monitor.State.START, dry_run=True)

tests/unit/hooks/test_dispatch.py

@@ -0,0 +1,68 @@
+import sys
+
+import pytest
+from flexmock import flexmock
+
+from borgmatic.hooks import dispatch as module
+
+
+def hook_function(config, log_prefix, thing, value):
+    '''
+    This test function gets mocked out below.
+    '''
+    pass
+
+
+def test_call_hook_invokes_module_function_with_arguments_and_returns_value():
+    hooks = {'super_hook': flexmock(), 'other_hook': flexmock()}
+    expected_return_value = flexmock()
+    test_module = sys.modules[__name__]
+    flexmock(module).HOOK_NAME_TO_MODULE = {'super_hook': test_module}
+    flexmock(test_module).should_receive('hook_function').with_args(
+        hooks['super_hook'], 'prefix', 55, value=66
+    ).and_return(expected_return_value).once()
+
+    return_value = module.call_hook('hook_function', hooks, 'prefix', 'super_hook', 55, value=66)
+
+    assert return_value == expected_return_value
+
+
+def test_call_hook_without_hook_config_skips_call():
+    hooks = {'other_hook': flexmock()}
+    test_module = sys.modules[__name__]
+    flexmock(module).HOOK_NAME_TO_MODULE = {'super_hook': test_module}
+    flexmock(test_module).should_receive('hook_function').never()
+
+    module.call_hook('hook_function', hooks, 'prefix', 'super_hook', 55, value=66)
+
+
+def test_call_hook_without_corresponding_module_raises():
+    hooks = {'super_hook': flexmock(), 'other_hook': flexmock()}
+    test_module = sys.modules[__name__]
+    flexmock(module).HOOK_NAME_TO_MODULE = {'other_hook': test_module}
+    flexmock(test_module).should_receive('hook_function').never()
+
+    with pytest.raises(ValueError):
+        module.call_hook('hook_function', hooks, 'prefix', 'super_hook', 55, value=66)
+
+
+def test_call_hooks_calls_each_hook_and_collects_return_values():
+    hooks = {'super_hook': flexmock(), 'other_hook': flexmock()}
+    expected_return_values = {'super_hook': flexmock(), 'other_hook': flexmock()}
+    flexmock(module).should_receive('call_hook').and_return(
+        expected_return_values['super_hook']
+    ).and_return(expected_return_values['other_hook'])
+
+    return_values = module.call_hooks('do_stuff', hooks, 'prefix', ('super_hook', 'other_hook'), 55)
+
+    assert return_values == expected_return_values
+
+
+def test_call_hooks_calls_skips_return_values_for_unconfigured_hooks():
+    hooks = {'super_hook': flexmock()}
+    expected_return_values = {'super_hook': flexmock()}
+    flexmock(module).should_receive('call_hook').and_return(expected_return_values['super_hook'])
+
+    return_values = module.call_hooks('do_stuff', hooks, 'prefix', ('super_hook', 'other_hook'), 55)
+
+    assert return_values == expected_return_values

tests/unit/hooks/test_dump.py

@@ -0,0 +1,169 @@
+import pytest
+from flexmock import flexmock
+
+from borgmatic.hooks import dump as module
+
+
+def test_make_database_dump_filename_uses_name_and_hostname():
+    flexmock(module.os.path).should_receive('expanduser').and_return('databases')
+
+    assert (
+        module.make_database_dump_filename('databases', 'test', 'hostname')
+        == 'databases/hostname/test'
+    )
+
+
+def test_make_database_dump_filename_without_hostname_defaults_to_localhost():
+    flexmock(module.os.path).should_receive('expanduser').and_return('databases')
+
+    assert module.make_database_dump_filename('databases', 'test') == 'databases/localhost/test'
+
+
+def test_make_database_dump_filename_with_invalid_name_raises():
+    flexmock(module.os.path).should_receive('expanduser').and_return('databases')
+
+    with pytest.raises(ValueError):
+        module.make_database_dump_filename('databases', 'invalid/name')
+
+
+def test_flatten_dump_patterns_produces_list_of_all_patterns():
+    dump_patterns = {'postgresql_databases': ['*/glob', 'glob/*'], 'mysql_databases': ['*/*/*']}
+    expected_patterns = dump_patterns['postgresql_databases'] + dump_patterns['mysql_databases']
+
+    assert module.flatten_dump_patterns(dump_patterns, ('bob',)) == expected_patterns
+
+
+def test_flatten_dump_patterns_with_no_patterns_errors():
+    dump_patterns = {'postgresql_databases': [], 'mysql_databases': []}
+
+    with pytest.raises(ValueError):
+        assert module.flatten_dump_patterns(dump_patterns, ('bob',))
+
+
+def test_flatten_dump_patterns_with_no_hooks_errors():
+    dump_patterns = {}
+
+    with pytest.raises(ValueError):
+        assert module.flatten_dump_patterns(dump_patterns, ('bob',))
+
+
+def test_remove_database_dumps_removes_dump_for_each_database():
+    databases = [{'name': 'foo'}, {'name': 'bar'}]
+    flexmock(module).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/foo'
+    ).and_return('databases/localhost/bar')
+    flexmock(module.os).should_receive('listdir').and_return([])
+    flexmock(module.os).should_receive('rmdir')
+
+    for name in ('foo', 'bar'):
+        flexmock(module.os).should_receive('remove').with_args(
+            'databases/localhost/{}'.format(name)
+        ).once()
+
+    module.remove_database_dumps('databases', databases, 'SuperDB', 'test.yaml', dry_run=False)
+
+
+def test_remove_database_dumps_with_dry_run_skips_removal():
+    databases = [{'name': 'foo'}, {'name': 'bar'}]
+    flexmock(module.os).should_receive('rmdir').never()
+    flexmock(module.os).should_receive('remove').never()
+
+    module.remove_database_dumps('databases', databases, 'SuperDB', 'test.yaml', dry_run=True)
+
+
+def test_remove_database_dumps_without_databases_does_not_raise():
+    module.remove_database_dumps('databases', [], 'SuperDB', 'test.yaml', dry_run=False)
+
+
+def test_convert_glob_patterns_to_borg_patterns_removes_leading_slash():
+    assert module.convert_glob_patterns_to_borg_patterns(('/etc/foo/bar',)) == ['sh:etc/foo/bar']
+
+
+def test_get_database_names_from_dumps_gets_names_from_filenames_matching_globs():
+    flexmock(module.glob).should_receive('glob').and_return(
+        ('databases/localhost/foo',)
+    ).and_return(('databases/localhost/bar',)).and_return(())
+
+    assert module.get_database_names_from_dumps(
+        ('databases/*/foo', 'databases/*/bar', 'databases/*/baz')
+    ) == ['foo', 'bar']
+
+
+def test_get_database_configurations_only_produces_named_databases():
+    databases = [
+        {'name': 'foo', 'hostname': 'example.org'},
+        {'name': 'bar', 'hostname': 'example.com'},
+        {'name': 'baz', 'hostname': 'example.org'},
+    ]
+
+    assert list(module.get_database_configurations(databases, ('foo', 'baz'))) == [
+        {'name': 'foo', 'hostname': 'example.org'},
+        {'name': 'baz', 'hostname': 'example.org'},
+    ]
+
+
+def test_get_database_configurations_matches_all_database():
+    databases = [
+        {'name': 'foo', 'hostname': 'example.org'},
+        {'name': 'all', 'hostname': 'example.com'},
+    ]
+
+    assert list(module.get_database_configurations(databases, ('foo', 'bar', 'baz'))) == [
+        {'name': 'foo', 'hostname': 'example.org'},
+        {'name': 'bar', 'hostname': 'example.com'},
+        {'name': 'baz', 'hostname': 'example.com'},
+    ]
+
+
+def test_get_per_hook_database_configurations_partitions_by_hook():
+    hooks = {'postgresql_databases': [flexmock()]}
+    names = ('foo', 'bar')
+    dump_patterns = flexmock()
+    expected_config = {'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}]}
+    flexmock(module).should_receive('get_database_configurations').with_args(
+        hooks['postgresql_databases'], names
+    ).and_return(expected_config['postgresql_databases'])
+
+    config = module.get_per_hook_database_configurations(hooks, names, dump_patterns)
+
+    assert config == expected_config
+
+
+def test_get_per_hook_database_configurations_defaults_to_detected_database_names():
+    hooks = {'postgresql_databases': [flexmock()]}
+    names = ()
+    detected_names = flexmock()
+    dump_patterns = {'postgresql_databases': [flexmock()]}
+    expected_config = {'postgresql_databases': [flexmock()]}
+    flexmock(module).should_receive('get_database_names_from_dumps').and_return(detected_names)
+    flexmock(module).should_receive('get_database_configurations').with_args(
+        hooks['postgresql_databases'], detected_names
+    ).and_return(expected_config['postgresql_databases'])
+
+    config = module.get_per_hook_database_configurations(hooks, names, dump_patterns)
+
+    assert config == expected_config
+
+
+def test_get_per_hook_database_configurations_with_unknown_database_name_raises():
+    hooks = {'postgresql_databases': [flexmock()]}
+    names = ('foo', 'bar')
+    dump_patterns = flexmock()
+    flexmock(module).should_receive('get_database_configurations').with_args(
+        hooks['postgresql_databases'], names
+    ).and_return([])
+
+    with pytest.raises(ValueError):
+        module.get_per_hook_database_configurations(hooks, names, dump_patterns)
+
+
+def test_get_per_hook_database_configurations_with_all_and_no_archive_dumps_raises():
+    hooks = {'postgresql_databases': [flexmock()]}
+    names = ('foo', 'all')
+    dump_patterns = flexmock()
+    flexmock(module).should_receive('get_database_configurations').with_args(
+        hooks['postgresql_databases'], names
+    ).and_return([])
+
+    with pytest.raises(ValueError):
+        module.get_per_hook_database_configurations(hooks, names, dump_patterns)

tests/unit/hooks/test_healthchecks.py

@@ -3,31 +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():
-    flexmock(module.requests).should_receive('get').never()
+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_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))
+    flexmock(module.requests).should_receive('get').never()
 
-    module.ping_healthchecks(ping_url, 'config.yaml', dry_run=False, append=append)
+    module.ping_monitor(ping_url, 'config.yaml', state=module.monitor.State.START, dry_run=True)

tests/unit/hooks/test_mysql.py

@@ -0,0 +1,205 @@
+import sys
+
+from flexmock import flexmock
+
+from borgmatic.hooks import mysql as module
+
+
+def test_dump_databases_runs_mysqldump_for_each_database():
+    databases = [{'name': 'foo'}, {'name': 'bar'}]
+    output_file = flexmock()
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/foo'
+    ).and_return('databases/localhost/bar')
+    flexmock(module.os).should_receive('makedirs')
+    flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file)
+
+    for name in ('foo', 'bar'):
+        flexmock(module).should_receive('execute_command').with_args(
+            ('mysqldump', '--add-drop-database', '--databases', name),
+            output_file=output_file,
+            extra_environment=None,
+        ).once()
+
+    module.dump_databases(databases, 'test.yaml', dry_run=False)
+
+
+def test_dump_databases_with_dry_run_skips_mysqldump():
+    databases = [{'name': 'foo'}, {'name': 'bar'}]
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/foo'
+    ).and_return('databases/localhost/bar')
+    flexmock(module.os).should_receive('makedirs').never()
+    flexmock(module).should_receive('execute_command').never()
+
+    module.dump_databases(databases, 'test.yaml', dry_run=True)
+
+
+def test_dump_databases_runs_mysqldump_with_hostname_and_port():
+    databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
+    output_file = flexmock()
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/database.example.org/foo'
+    )
+    flexmock(module.os).should_receive('makedirs')
+    flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file)
+
+    flexmock(module).should_receive('execute_command').with_args(
+        (
+            'mysqldump',
+            '--add-drop-database',
+            '--host',
+            'database.example.org',
+            '--port',
+            '5433',
+            '--protocol',
+            'tcp',
+            '--databases',
+            'foo',
+        ),
+        output_file=output_file,
+        extra_environment=None,
+    ).once()
+
+    module.dump_databases(databases, 'test.yaml', dry_run=False)
+
+
+def test_dump_databases_runs_mysqldump_with_username_and_password():
+    databases = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}]
+    output_file = flexmock()
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/foo'
+    )
+    flexmock(module.os).should_receive('makedirs')
+    flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file)
+
+    flexmock(module).should_receive('execute_command').with_args(
+        ('mysqldump', '--add-drop-database', '--user', 'root', '--databases', 'foo'),
+        output_file=output_file,
+        extra_environment={'MYSQL_PWD': 'trustsome1'},
+    ).once()
+
+    module.dump_databases(databases, 'test.yaml', dry_run=False)
+
+
+def test_dump_databases_runs_mysqldump_with_options():
+    databases = [{'name': 'foo', 'options': '--stuff=such'}]
+    output_file = flexmock()
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/foo'
+    )
+    flexmock(module.os).should_receive('makedirs')
+    flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file)
+
+    flexmock(module).should_receive('execute_command').with_args(
+        ('mysqldump', '--add-drop-database', '--stuff=such', '--databases', 'foo'),
+        output_file=output_file,
+        extra_environment=None,
+    ).once()
+
+    module.dump_databases(databases, 'test.yaml', dry_run=False)
+
+
+def test_dump_databases_runs_mysqldump_for_all_databases():
+    databases = [{'name': 'all'}]
+    output_file = flexmock()
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/all'
+    )
+    flexmock(module.os).should_receive('makedirs')
+    flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file)
+
+    flexmock(module).should_receive('execute_command').with_args(
+        ('mysqldump', '--add-drop-database', '--all-databases'),
+        output_file=output_file,
+        extra_environment=None,
+    ).once()
+
+    module.dump_databases(databases, 'test.yaml', dry_run=False)
+
+
+def test_make_database_dump_patterns_converts_names_to_glob_paths():
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/*/foo'
+    ).and_return('databases/*/bar')
+
+    assert module.make_database_dump_patterns(flexmock(), flexmock(), ('foo', 'bar')) == [
+        'databases/*/foo',
+        'databases/*/bar',
+    ]
+
+
+def test_make_database_dump_patterns_treats_empty_names_as_matching_all_databases():
+    flexmock(module.dump).should_receive('make_database_dump_filename').with_args(
+        module.DUMP_PATH, '*', '*'
+    ).and_return('databases/*/*')
+
+    assert module.make_database_dump_patterns(flexmock(), flexmock(), ()) == ['databases/*/*']
+
+
+def test_restore_database_dumps_restores_each_database():
+    databases = [{'name': 'foo'}, {'name': 'bar'}]
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/foo'
+    ).and_return('databases/localhost/bar')
+
+    for name in ('foo', 'bar'):
+        dump_filename = 'databases/localhost/{}'.format(name)
+        input_file = flexmock()
+        flexmock(sys.modules['builtins']).should_receive('open').with_args(
+            dump_filename
+        ).and_return(input_file)
+        flexmock(module).should_receive('execute_command').with_args(
+            ('mysql', '--batch'), input_file=input_file, extra_environment=None
+        ).once()
+
+    module.restore_database_dumps(databases, 'test.yaml', dry_run=False)
+
+
+def test_restore_database_dumps_runs_mysql_with_hostname_and_port():
+    databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/foo'
+    )
+    dump_filename = 'databases/localhost/foo'
+    input_file = flexmock()
+    flexmock(sys.modules['builtins']).should_receive('open').with_args(dump_filename).and_return(
+        input_file
+    )
+
+    flexmock(module).should_receive('execute_command').with_args(
+        (
+            'mysql',
+            '--batch',
+            '--host',
+            'database.example.org',
+            '--port',
+            '5433',
+            '--protocol',
+            'tcp',
+        ),
+        input_file=input_file,
+        extra_environment=None,
+    ).once()
+
+    module.restore_database_dumps(databases, 'test.yaml', dry_run=False)
+
+
+def test_restore_database_dumps_runs_mysql_with_username_and_password():
+    databases = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}]
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/foo'
+    )
+    dump_filename = 'databases/localhost/foo'
+    input_file = flexmock()
+    flexmock(sys.modules['builtins']).should_receive('open').with_args(dump_filename).and_return(
+        input_file
+    )
+
+    flexmock(module).should_receive('execute_command').with_args(
+        ('mysql', '--batch', '--user', 'root'),
+        input_file=input_file,
+        extra_environment={'MYSQL_PWD': 'trustsome1'},
+    ).once()
+
+    module.restore_database_dumps(databases, 'test.yaml', dry_run=False)

tests/unit/hooks/test_postgresql.py

@@ -1,31 +1,11 @@
-import pytest
 from flexmock import flexmock
 
 from borgmatic.hooks import postgresql as module
 
 
-def test_make_database_dump_filename_uses_name_and_hostname():
-    flexmock(module.os.path).should_receive('expanduser').and_return('databases')
-
-    assert module.make_database_dump_filename('test', 'hostname') == 'databases/hostname/test'
-
-
-def test_make_database_dump_filename_without_hostname_defaults_to_localhost():
-    flexmock(module.os.path).should_receive('expanduser').and_return('databases')
-
-    assert module.make_database_dump_filename('test') == 'databases/localhost/test'
-
-
-def test_make_database_dump_filename_with_invalid_name_raises():
-    flexmock(module.os.path).should_receive('expanduser').and_return('databases')
-
-    with pytest.raises(ValueError):
-        module.make_database_dump_filename('invalid/name')
-
-
 def test_dump_databases_runs_pg_dump_for_each_database():
     databases = [{'name': 'foo'}, {'name': 'bar'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
     ).and_return('databases/localhost/bar')
     flexmock(module.os).should_receive('makedirs')
@@ -50,22 +30,18 @@ def test_dump_databases_runs_pg_dump_for_each_database():
 
 def test_dump_databases_with_dry_run_skips_pg_dump():
     databases = [{'name': 'foo'}, {'name': 'bar'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
     ).and_return('databases/localhost/bar')
-    flexmock(module.os).should_receive('makedirs')
+    flexmock(module.os).should_receive('makedirs').never()
     flexmock(module).should_receive('execute_command').never()
 
     module.dump_databases(databases, 'test.yaml', dry_run=True)
 
 
-def test_dump_databases_without_databases_does_not_raise():
-    module.dump_databases([], 'test.yaml', dry_run=False)
-
-
 def test_dump_databases_runs_pg_dump_with_hostname_and_port():
     databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/database.example.org/foo'
     )
     flexmock(module.os).should_receive('makedirs')
@@ -93,7 +69,7 @@ def test_dump_databases_runs_pg_dump_with_hostname_and_port():
 
 def test_dump_databases_runs_pg_dump_with_username_and_password():
     databases = [{'name': 'foo', 'username': 'postgres', 'password': 'trustsome1'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
     )
     flexmock(module.os).should_receive('makedirs')
@@ -119,7 +95,7 @@ def test_dump_databases_runs_pg_dump_with_username_and_password():
 
 def test_dump_databases_runs_pg_dump_with_format():
     databases = [{'name': 'foo', 'format': 'tar'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
     )
     flexmock(module.os).should_receive('makedirs')
@@ -143,7 +119,7 @@ def test_dump_databases_runs_pg_dump_with_format():
 
 def test_dump_databases_runs_pg_dump_with_options():
     databases = [{'name': 'foo', 'options': '--stuff=such'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
     )
     flexmock(module.os).should_receive('makedirs')
@@ -168,7 +144,7 @@ def test_dump_databases_runs_pg_dump_with_options():
 
 def test_dump_databases_runs_pg_dumpall_for_all_databases():
     databases = [{'name': 'all'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/all'
     )
     flexmock(module.os).should_receive('makedirs')
@@ -181,102 +157,28 @@ def test_dump_databases_runs_pg_dumpall_for_all_databases():
     module.dump_databases(databases, 'test.yaml', dry_run=False)
 
 
-def test_remove_database_dumps_removes_dump_for_each_database():
-    databases = [{'name': 'foo'}, {'name': 'bar'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
-        'databases/localhost/foo'
-    ).and_return('databases/localhost/bar')
-    flexmock(module.os).should_receive('listdir').and_return([])
-    flexmock(module.os).should_receive('rmdir')
-
-    for name in ('foo', 'bar'):
-        flexmock(module.os).should_receive('remove').with_args(
-            'databases/localhost/{}'.format(name)
-        ).once()
-
-    module.remove_database_dumps(databases, 'test.yaml', dry_run=False)
-
-
-def test_remove_database_dumps_with_dry_run_skips_removal():
-    databases = [{'name': 'foo'}, {'name': 'bar'}]
-    flexmock(module.os).should_receive('remove').never()
-
-    module.remove_database_dumps(databases, 'test.yaml', dry_run=True)
-
-
-def test_remove_database_dumps_without_databases_does_not_raise():
-    module.remove_database_dumps([], 'test.yaml', dry_run=False)
-
-
 def test_make_database_dump_patterns_converts_names_to_glob_paths():
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/*/foo'
     ).and_return('databases/*/bar')
 
-    assert module.make_database_dump_patterns(('foo', 'bar')) == [
+    assert module.make_database_dump_patterns(flexmock(), flexmock(), ('foo', 'bar')) == [
         'databases/*/foo',
         'databases/*/bar',
     ]
 
 
 def test_make_database_dump_patterns_treats_empty_names_as_matching_all_databases():
-    flexmock(module).should_receive('make_database_dump_filename').with_args('*', '*').and_return(
-        'databases/*/*'
-    )
+    flexmock(module.dump).should_receive('make_database_dump_filename').with_args(
+        module.DUMP_PATH, '*', '*'
+    ).and_return('databases/*/*')
 
-    assert module.make_database_dump_patterns(()) == ['databases/*/*']
-
-
-def test_convert_glob_patterns_to_borg_patterns_removes_leading_slash():
-    assert module.convert_glob_patterns_to_borg_patterns(('/etc/foo/bar',)) == ['sh:etc/foo/bar']
-
-
-def test_get_database_names_from_dumps_gets_names_from_filenames_matching_globs():
-    flexmock(module.glob).should_receive('glob').and_return(
-        ('databases/localhost/foo',)
-    ).and_return(('databases/localhost/bar',)).and_return(())
-
-    assert module.get_database_names_from_dumps(
-        ('databases/*/foo', 'databases/*/bar', 'databases/*/baz')
-    ) == ['foo', 'bar']
-
-
-def test_get_database_configurations_only_produces_named_databases():
-    databases = [
-        {'name': 'foo', 'hostname': 'example.org'},
-        {'name': 'bar', 'hostname': 'example.com'},
-        {'name': 'baz', 'hostname': 'example.org'},
-    ]
-
-    assert list(module.get_database_configurations(databases, ('foo', 'baz'))) == [
-        {'name': 'foo', 'hostname': 'example.org'},
-        {'name': 'baz', 'hostname': 'example.org'},
-    ]
-
-
-def test_get_database_configurations_matches_all_database():
-    databases = [
-        {'name': 'foo', 'hostname': 'example.org'},
-        {'name': 'all', 'hostname': 'example.com'},
-    ]
-
-    assert list(module.get_database_configurations(databases, ('foo', 'bar', 'baz'))) == [
-        {'name': 'foo', 'hostname': 'example.org'},
-        {'name': 'bar', 'hostname': 'example.com'},
-        {'name': 'baz', 'hostname': 'example.com'},
-    ]
-
-
-def test_get_database_configurations_with_unknown_database_name_raises():
-    databases = [{'name': 'foo', 'hostname': 'example.org'}]
-
-    with pytest.raises(ValueError):
-        list(module.get_database_configurations(databases, ('foo', 'bar')))
+    assert module.make_database_dump_patterns(flexmock(), flexmock(), ()) == ['databases/*/*']
 
 
 def test_restore_database_dumps_restores_each_database():
     databases = [{'name': 'foo'}, {'name': 'bar'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
     ).and_return('databases/localhost/bar')
 
@@ -302,15 +204,11 @@ def test_restore_database_dumps_restores_each_database():
     module.restore_database_dumps(databases, 'test.yaml', dry_run=False)
 
 
-def test_restore_database_dumps_without_databases_does_not_raise():
-    module.restore_database_dumps({}, 'test.yaml', dry_run=False)
-
-
 def test_restore_database_dumps_runs_pg_restore_with_hostname_and_port():
     databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
-    ).and_return('databases/localhost/bar')
+    )
 
     flexmock(module).should_receive('execute_command').with_args(
         (
@@ -351,9 +249,9 @@ def test_restore_database_dumps_runs_pg_restore_with_hostname_and_port():
 
 def test_restore_database_dumps_runs_pg_restore_with_username_and_password():
     databases = [{'name': 'foo', 'username': 'postgres', 'password': 'trustsome1'}]
-    flexmock(module).should_receive('make_database_dump_filename').and_return(
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
-    ).and_return('databases/localhost/bar')
+    )
 
     flexmock(module).should_receive('execute_command').with_args(
         (

tests/unit/test_execute.py

@@ -1,5 +1,3 @@
-import logging
-
 import pytest
 from flexmock import flexmock
 
@@ -47,31 +45,75 @@ def test_exit_code_indicates_error_with_non_borg_success_is_false():
 def test_execute_command_calls_full_command():
     full_command = ['foo', 'bar']
     flexmock(module.os, environ={'a': 'b'})
-    flexmock(module).should_receive('execute_and_log_output').with_args(
+    flexmock(module.subprocess).should_receive('Popen').with_args(
         full_command,
-        output_log_level=logging.INFO,
+        stdin=None,
+        stdout=module.subprocess.PIPE,
+        stderr=module.subprocess.STDOUT,
         shell=False,
-        environment=None,
-        working_directory=None,
-        error_on_warnings=False,
-    ).once()
+        env=None,
+        cwd=None,
+    ).and_return(flexmock(stdout=None)).once()
+    flexmock(module).should_receive('log_output')
 
     output = module.execute_command(full_command)
 
     assert output is None
 
 
+def test_execute_command_calls_full_command_with_output_file():
+    full_command = ['foo', 'bar']
+    output_file = flexmock(name='test')
+    flexmock(module.os, environ={'a': 'b'})
+    flexmock(module.subprocess).should_receive('Popen').with_args(
+        full_command,
+        stdin=None,
+        stdout=output_file,
+        stderr=module.subprocess.PIPE,
+        shell=False,
+        env=None,
+        cwd=None,
+    ).and_return(flexmock(stderr=None)).once()
+    flexmock(module).should_receive('log_output')
+
+    output = module.execute_command(full_command, output_file=output_file)
+
+    assert output is None
+
+
+def test_execute_command_calls_full_command_with_input_file():
+    full_command = ['foo', 'bar']
+    input_file = flexmock(name='test')
+    flexmock(module.os, environ={'a': 'b'})
+    flexmock(module.subprocess).should_receive('Popen').with_args(
+        full_command,
+        stdin=input_file,
+        stdout=module.subprocess.PIPE,
+        stderr=module.subprocess.STDOUT,
+        shell=False,
+        env=None,
+        cwd=None,
+    ).and_return(flexmock(stdout=None)).once()
+    flexmock(module).should_receive('log_output')
+
+    output = module.execute_command(full_command, input_file=input_file)
+
+    assert output is None
+
+
 def test_execute_command_calls_full_command_with_shell():
     full_command = ['foo', 'bar']
     flexmock(module.os, environ={'a': 'b'})
-    flexmock(module).should_receive('execute_and_log_output').with_args(
+    flexmock(module.subprocess).should_receive('Popen').with_args(
         full_command,
-        output_log_level=logging.INFO,
+        stdin=None,
+        stdout=module.subprocess.PIPE,
+        stderr=module.subprocess.STDOUT,
         shell=True,
-        environment=None,
-        working_directory=None,
-        error_on_warnings=False,
-    ).once()
+        env=None,
+        cwd=None,
+    ).and_return(flexmock(stdout=None)).once()
+    flexmock(module).should_receive('log_output')
 
     output = module.execute_command(full_command, shell=True)
 
@@ -81,14 +123,16 @@ def test_execute_command_calls_full_command_with_shell():
 def test_execute_command_calls_full_command_with_extra_environment():
     full_command = ['foo', 'bar']
     flexmock(module.os, environ={'a': 'b'})
-    flexmock(module).should_receive('execute_and_log_output').with_args(
+    flexmock(module.subprocess).should_receive('Popen').with_args(
         full_command,
-        output_log_level=logging.INFO,
+        stdin=None,
+        stdout=module.subprocess.PIPE,
+        stderr=module.subprocess.STDOUT,
         shell=False,
-        environment={'a': 'b', 'c': 'd'},
-        working_directory=None,
-        error_on_warnings=False,
-    ).once()
+        env={'a': 'b', 'c': 'd'},
+        cwd=None,
+    ).and_return(flexmock(stdout=None)).once()
+    flexmock(module).should_receive('log_output')
 
     output = module.execute_command(full_command, extra_environment={'c': 'd'})
 
@@ -98,37 +142,22 @@ def test_execute_command_calls_full_command_with_extra_environment():
 def test_execute_command_calls_full_command_with_working_directory():
     full_command = ['foo', 'bar']
     flexmock(module.os, environ={'a': 'b'})
-    flexmock(module).should_receive('execute_and_log_output').with_args(
+    flexmock(module.subprocess).should_receive('Popen').with_args(
         full_command,
-        output_log_level=logging.INFO,
+        stdin=None,
+        stdout=module.subprocess.PIPE,
+        stderr=module.subprocess.STDOUT,
         shell=False,
-        environment=None,
-        working_directory='/working',
-        error_on_warnings=False,
-    ).once()
+        env=None,
+        cwd='/working',
+    ).and_return(flexmock(stdout=None)).once()
+    flexmock(module).should_receive('log_output')
 
     output = module.execute_command(full_command, working_directory='/working')
 
     assert output is None
 
 
-def test_execute_command_calls_full_command_with_error_on_warnings():
-    full_command = ['foo', 'bar']
-    flexmock(module.os, environ={'a': 'b'})
-    flexmock(module).should_receive('execute_and_log_output').with_args(
-        full_command,
-        output_log_level=logging.INFO,
-        shell=False,
-        environment=None,
-        working_directory=None,
-        error_on_warnings=True,
-    ).once()
-
-    output = module.execute_command(full_command, error_on_warnings=True)
-
-    assert output is None
-
-
 def test_execute_command_captures_output():
     full_command = ['foo', 'bar']
     expected_output = '[]'

tests/unit/test_logger.py

@@ -203,10 +203,10 @@ def test_configure_logging_to_logfile_instead_of_syslog():
     )
     flexmock(module.os.path).should_receive('exists').with_args('/dev/log').and_return(True)
     flexmock(module.logging.handlers).should_receive('SysLogHandler').never()
-    file_handler = logging.FileHandler('/tmp/logfile')
-    flexmock(module.logging).should_receive('FileHandler').with_args('/tmp/logfile').and_return(
-        file_handler
-    ).once()
+    file_handler = logging.handlers.WatchedFileHandler('/tmp/logfile')
+    flexmock(module.logging.handlers).should_receive('WatchedFileHandler').with_args(
+        '/tmp/logfile'
+    ).and_return(file_handler).once()
 
     module.configure_logging(
         console_log_level=logging.INFO, log_file_log_level=logging.DEBUG, log_file='/tmp/logfile'
@@ -214,12 +214,12 @@ def test_configure_logging_to_logfile_instead_of_syslog():
 
 
 def test_configure_logging_skips_logfile_if_argument_is_none():
-    # No FileHandler added if argument --log-file is None
+    # No WatchedFileHandler added if argument --log-file is None
     flexmock(module).should_receive('interactive_console').and_return(False)
     flexmock(module.logging).should_receive('basicConfig').with_args(
         level=logging.INFO, handlers=tuple
     )
     flexmock(module.os.path).should_receive('exists').and_return(False)
-    flexmock(module.logging).should_receive('FileHandler').never()
+    flexmock(module.logging.handlers).should_receive('WatchedFileHandler').never()
 
     module.configure_logging(console_log_level=logging.INFO, log_file=None)

tox.ini

@@ -20,7 +20,7 @@ commands =
 
 [testenv:black]
 commands =
-    py36,py37,py38: black {posargs} .
+    black {posargs} .
 
 [testenv:test]
 commands =