From 9db31bd1e9c0d001dda959574ddd1e6783b0f5ef Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 8 Mar 2023 13:19:41 -0800 Subject: [PATCH] Run any command-line actions in the order specified instead of using a fixed ordering (#304). --- NEWS | 3 +- borgmatic/commands/arguments.py | 9 +- borgmatic/commands/borgmatic.py | 305 ++++++++++---------- docs/how-to/deal-with-very-large-backups.md | 14 +- tests/unit/commands/test_arguments.py | 22 ++ tests/unit/commands/test_borgmatic.py | 27 ++ 6 files changed, 222 insertions(+), 158 deletions(-) diff --git a/NEWS b/NEWS index c422387..aa62d5e 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ 1.7.9.dev0 * #295: Add a SQLite database dump/restore hook. - * #628: Add Healthchecks "log" state to send borgmatic logs to Healthchecks without signalling + * #304: Run any command-line actions in the order specified instead of using a fixed ordering. + * #628: Add a Healthchecks "log" state to send borgmatic logs to Healthchecks without signalling success or failure. 1.7.8 diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 3acf0d2..9b88271 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -46,11 +46,12 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): if 'borg' in unparsed_arguments: subparsers = {'borg': subparsers['borg']} - for subparser_name, subparser in subparsers.items(): - if subparser_name not in remaining_arguments: - continue + for argument in remaining_arguments: + canonical_name = alias_to_subparser_name.get(argument, argument) + subparser = subparsers.get(canonical_name) - canonical_name = alias_to_subparser_name.get(subparser_name, subparser_name) + if not subparser: + continue # If a parsed value happens to be the same as the name of a subparser, remove it from the # remaining arguments. This prevents, for instance, "check --only extract" from triggering diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 6bc2456..4cf6011 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -281,155 +281,162 @@ def run_actions( **hook_context, ) - if 'rcreate' in arguments: - borgmatic.actions.rcreate.run_rcreate( - repository, - storage, - local_borg_version, - arguments['rcreate'], - global_arguments, - local_path, - remote_path, - ) - if 'transfer' in arguments: - borgmatic.actions.transfer.run_transfer( - repository, - storage, - local_borg_version, - arguments['transfer'], - global_arguments, - local_path, - remote_path, - ) - if 'prune' in arguments: - borgmatic.actions.prune.run_prune( - config_filename, - repository, - storage, - retention, - hooks, - hook_context, - local_borg_version, - arguments['prune'], - global_arguments, - dry_run_label, - local_path, - remote_path, - ) - if 'compact' in arguments: - borgmatic.actions.compact.run_compact( - config_filename, - repository, - storage, - retention, - hooks, - hook_context, - local_borg_version, - arguments['compact'], - global_arguments, - dry_run_label, - local_path, - remote_path, - ) - if 'create' in arguments: - yield from borgmatic.actions.create.run_create( - config_filename, - repository, - location, - storage, - hooks, - hook_context, - local_borg_version, - arguments['create'], - global_arguments, - dry_run_label, - local_path, - remote_path, - ) - if 'check' in arguments and checks.repository_enabled_for_checks(repository, consistency): - borgmatic.actions.check.run_check( - config_filename, - repository, - location, - storage, - consistency, - hooks, - hook_context, - local_borg_version, - arguments['check'], - global_arguments, - local_path, - remote_path, - ) - if 'extract' in arguments: - borgmatic.actions.extract.run_extract( - config_filename, - repository, - location, - storage, - hooks, - hook_context, - local_borg_version, - arguments['extract'], - global_arguments, - local_path, - remote_path, - ) - if 'export-tar' in arguments: - borgmatic.actions.export_tar.run_export_tar( - repository, - storage, - local_borg_version, - arguments['export-tar'], - global_arguments, - local_path, - remote_path, - ) - if 'mount' in arguments: - borgmatic.actions.mount.run_mount( - repository, storage, local_borg_version, arguments['mount'], local_path, remote_path, - ) - if 'restore' in arguments: - borgmatic.actions.restore.run_restore( - repository, - location, - storage, - hooks, - local_borg_version, - arguments['restore'], - global_arguments, - local_path, - remote_path, - ) - if 'rlist' in arguments: - yield from borgmatic.actions.rlist.run_rlist( - repository, storage, local_borg_version, arguments['rlist'], local_path, remote_path, - ) - if 'list' in arguments: - yield from borgmatic.actions.list.run_list( - repository, storage, local_borg_version, arguments['list'], local_path, remote_path, - ) - if 'rinfo' in arguments: - yield from borgmatic.actions.rinfo.run_rinfo( - repository, storage, local_borg_version, arguments['rinfo'], local_path, remote_path, - ) - if 'info' in arguments: - yield from borgmatic.actions.info.run_info( - repository, storage, local_borg_version, arguments['info'], local_path, remote_path, - ) - if 'break-lock' in arguments: - borgmatic.actions.break_lock.run_break_lock( - repository, - storage, - local_borg_version, - arguments['break-lock'], - local_path, - remote_path, - ) - if 'borg' in arguments: - borgmatic.actions.borg.run_borg( - repository, storage, local_borg_version, arguments['borg'], local_path, remote_path, - ) + for (action_name, action_arguments) in arguments.items(): + if action_name == 'rcreate': + borgmatic.actions.rcreate.run_rcreate( + repository, + storage, + local_borg_version, + action_arguments, + global_arguments, + local_path, + remote_path, + ) + elif action_name == 'transfer': + borgmatic.actions.transfer.run_transfer( + repository, + storage, + local_borg_version, + action_arguments, + global_arguments, + local_path, + remote_path, + ) + elif action_name == 'prune': + borgmatic.actions.prune.run_prune( + config_filename, + repository, + storage, + retention, + hooks, + hook_context, + local_borg_version, + action_arguments, + global_arguments, + dry_run_label, + local_path, + remote_path, + ) + elif action_name == 'compact': + borgmatic.actions.compact.run_compact( + config_filename, + repository, + storage, + retention, + hooks, + hook_context, + local_borg_version, + action_arguments, + global_arguments, + dry_run_label, + local_path, + remote_path, + ) + elif action_name == 'create': + yield from borgmatic.actions.create.run_create( + config_filename, + repository, + location, + storage, + hooks, + hook_context, + local_borg_version, + action_arguments, + global_arguments, + dry_run_label, + local_path, + remote_path, + ) + elif action_name == 'check': + if checks.repository_enabled_for_checks(repository, consistency): + borgmatic.actions.check.run_check( + config_filename, + repository, + location, + storage, + consistency, + hooks, + hook_context, + local_borg_version, + action_arguments, + global_arguments, + local_path, + remote_path, + ) + elif action_name == 'extract': + borgmatic.actions.extract.run_extract( + config_filename, + repository, + location, + storage, + hooks, + hook_context, + local_borg_version, + action_arguments, + global_arguments, + local_path, + remote_path, + ) + elif action_name == 'export-tar': + borgmatic.actions.export_tar.run_export_tar( + repository, + storage, + local_borg_version, + action_arguments, + global_arguments, + local_path, + remote_path, + ) + elif action_name == 'mount': + borgmatic.actions.mount.run_mount( + repository, + storage, + local_borg_version, + arguments['mount'], + local_path, + remote_path, + ) + elif action_name == 'restore': + borgmatic.actions.restore.run_restore( + repository, + location, + storage, + hooks, + local_borg_version, + action_arguments, + global_arguments, + local_path, + remote_path, + ) + elif action_name == 'rlist': + yield from borgmatic.actions.rlist.run_rlist( + repository, storage, local_borg_version, action_arguments, local_path, remote_path, + ) + elif action_name == 'list': + yield from borgmatic.actions.list.run_list( + repository, storage, local_borg_version, action_arguments, local_path, remote_path, + ) + elif action_name == 'rinfo': + yield from borgmatic.actions.rinfo.run_rinfo( + repository, storage, local_borg_version, action_arguments, local_path, remote_path, + ) + elif action_name == 'info': + yield from borgmatic.actions.info.run_info( + repository, storage, local_borg_version, action_arguments, local_path, remote_path, + ) + elif action_name == 'break-lock': + borgmatic.actions.break_lock.run_break_lock( + repository, + storage, + local_borg_version, + arguments['break-lock'], + local_path, + remote_path, + ) + elif action_name == 'borg': + borgmatic.actions.borg.run_borg( + repository, storage, local_borg_version, action_arguments, local_path, remote_path, + ) command.execute_hook( hooks.get('after_actions'), diff --git a/docs/how-to/deal-with-very-large-backups.md b/docs/how-to/deal-with-very-large-backups.md index 720d433..343f692 100644 --- a/docs/how-to/deal-with-very-large-backups.md +++ b/docs/how-to/deal-with-very-large-backups.md @@ -36,10 +36,16 @@ skipping certain actions while running others. For instance, this skips borgmatic create check ``` -Or, you can make backups with `create` on a frequent schedule (e.g. with -`borgmatic create` called from one cron job), while only running expensive -consistency checks with `check` on a much less frequent basis (e.g. with -`borgmatic check` called from a separate cron job). +New in version 1.7.9 borgmatic +now respects your specified command-line action order, running actions in the +order you specify. In previous versions, borgmatic ran your specified actions +in a fixed ordering regardless of the order they appeared on the command-line. + +But instead of running actions together, another option is to run backups with +`create` on a frequent schedule (e.g. with `borgmatic create` called from one +cron job), while only running expensive consistency checks with `check` on a +much less frequent basis (e.g. with `borgmatic check` called from a separate +cron job). ### Consistency check configuration diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index 640d6ad..9354cf5 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -1,3 +1,5 @@ +import collections + from flexmock import flexmock from borgmatic.commands import arguments as module @@ -70,6 +72,26 @@ def test_parse_subparser_arguments_consumes_multiple_subparser_arguments(): assert remaining_arguments == [] +def test_parse_subparser_arguments_respects_command_line_action_ordering(): + other_namespace = flexmock() + action_namespace = flexmock(foo=True) + subparsers = { + 'action': flexmock( + parse_known_args=lambda arguments: (action_namespace, ['action', '--foo', 'true']) + ), + 'other': flexmock(parse_known_args=lambda arguments: (other_namespace, ['other'])), + } + + arguments, remaining_arguments = module.parse_subparser_arguments( + ('other', '--foo', 'true', 'action'), subparsers + ) + + assert arguments == collections.OrderedDict( + [('other', other_namespace), ('action', action_namespace)] + ) + assert remaining_arguments == [] + + def test_parse_subparser_arguments_applies_default_subparsers(): prune_namespace = flexmock() compact_namespace = flexmock() diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index fc3f34d..a6899d9 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -778,6 +778,33 @@ def test_run_actions_runs_borg(): ) +def test_run_actions_runs_multiple_actions_in_argument_order(): + flexmock(module).should_receive('add_custom_log_levels') + flexmock(module.command).should_receive('execute_hook') + flexmock(borgmatic.actions.borg).should_receive('run_borg').once().ordered() + flexmock(borgmatic.actions.restore).should_receive('run_restore').once().ordered() + + tuple( + module.run_actions( + arguments={ + 'global': flexmock(dry_run=False), + 'borg': flexmock(), + 'restore': flexmock(), + }, + config_filename=flexmock(), + location={'repositories': []}, + storage=flexmock(), + retention=flexmock(), + consistency=flexmock(), + hooks={}, + local_path=flexmock(), + remote_path=flexmock(), + local_borg_version=flexmock(), + repository_path='repo', + ) + ) + + def test_load_configurations_collects_parsed_configurations_and_logs(): configuration = flexmock() other_configuration = flexmock()