From 462326406ed45e91a8790ee0372fc1cc06ffe367 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 21 Aug 2022 14:25:16 -0700 Subject: [PATCH] Drop only-style actions like "--create", rename "prune --files" to "prune --list", and add "--list" alias to "create" and "export-tar" (#571). --- NEWS | 5 ++ borgmatic/borg/create.py | 6 +- borgmatic/borg/export_tar.py | 6 +- borgmatic/borg/prune.py | 6 +- borgmatic/commands/arguments.py | 30 ++++----- borgmatic/commands/borgmatic.py | 6 +- docs/how-to/monitor-your-backups.md | 6 +- docs/how-to/set-up-backups.md | 14 ++-- tests/integration/commands/test_arguments.py | 69 +++----------------- tests/unit/borg/test_create.py | 4 +- tests/unit/borg/test_export_tar.py | 2 +- tests/unit/borg/test_prune.py | 4 +- tests/unit/commands/test_borgmatic.py | 6 +- 13 files changed, 59 insertions(+), 105 deletions(-) diff --git a/NEWS b/NEWS index 2747053..9b2bc76 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,11 @@ * #565: Fix handling of "repository" and "data" consistency checks to prevent invalid Borg flags. * #566: Modify "mount" and "extract" actions to require the "--repository" flag when multiple repositories are configured. + * #571: BREAKING: Remove old-style command-line action flags like "--create, "--list", etc. If + you're already using actions like "create" and "list" instead, this change should not affect you. + * #571: BREAKING: Rename "--files" flag on "prune" action to "--list", as it lists archives, not + files. + * #571: Add "--list" as alias for "--files" flag on "create" and "export-tar" actions. * Add support for disabling TLS verification in Healthchecks monitoring hook with "verify_tls" option. diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 9977e05..04d7ec1 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -203,7 +203,7 @@ def create_archive( progress=False, stats=False, json=False, - files=False, + list_files=False, stream_processes=None, ): ''' @@ -290,7 +290,7 @@ def create_archive( + (('--remote-path', remote_path) if remote_path else ()) + (('--umask', str(umask)) if umask else ()) + (('--lock-wait', str(lock_wait)) if lock_wait else ()) - + (('--list', '--filter', 'AME-') if files and not json and not progress else ()) + + (('--list', '--filter', 'AME-') if list_files and not json and not progress else ()) + (('--info',) if logger.getEffectiveLevel() == logging.INFO and not json else ()) + (('--stats',) if stats and not json and not dry_run else ()) + (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) and not json else ()) @@ -304,7 +304,7 @@ def create_archive( if json: output_log_level = None - elif (stats or files) and logger.getEffectiveLevel() == logging.WARNING: + elif (stats or list_files) and logger.getEffectiveLevel() == logging.WARNING: output_log_level = logging.WARNING else: output_log_level = logging.INFO diff --git a/borgmatic/borg/export_tar.py b/borgmatic/borg/export_tar.py index 9bbd008..7e14701 100644 --- a/borgmatic/borg/export_tar.py +++ b/borgmatic/borg/export_tar.py @@ -18,7 +18,7 @@ def export_tar_archive( local_path='borg', remote_path=None, tar_filter=None, - files=False, + list_files=False, strip_components=None, ): ''' @@ -39,7 +39,7 @@ def export_tar_archive( + (('--umask', str(umask)) if umask else ()) + (('--lock-wait', str(lock_wait)) if lock_wait else ()) + (('--info',) if logger.getEffectiveLevel() == logging.INFO else ()) - + (('--list',) if files else ()) + + (('--list',) if list_files else ()) + (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) + (('--dry-run',) if dry_run else ()) + (('--tar-filter', tar_filter) if tar_filter else ()) @@ -53,7 +53,7 @@ def export_tar_archive( + (tuple(paths) if paths else ()) ) - if files and logger.getEffectiveLevel() == logging.WARNING: + if list_files and logger.getEffectiveLevel() == logging.WARNING: output_log_level = logging.WARNING else: output_log_level = logging.INFO diff --git a/borgmatic/borg/prune.py b/borgmatic/borg/prune.py index 54c428c..d2c5533 100644 --- a/borgmatic/borg/prune.py +++ b/borgmatic/borg/prune.py @@ -41,7 +41,7 @@ def prune_archives( local_path='borg', remote_path=None, stats=False, - files=False, + list_archives=False, ): ''' Given dry-run flag, a local or remote repository path, a storage config dict, and a @@ -60,14 +60,14 @@ def prune_archives( + (('--lock-wait', str(lock_wait)) if lock_wait else ()) + (('--stats',) if stats and not dry_run else ()) + (('--info',) if logger.getEffectiveLevel() == logging.INFO else ()) - + (('--list',) if files else ()) + + (('--list',) if list_archives else ()) + (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) + (('--dry-run',) if dry_run else ()) + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) + flags.make_repository_flags(repository, local_borg_version) ) - if (stats or files) and logger.getEffectiveLevel() == logging.WARNING: + if (stats or list_archives) and logger.getEffectiveLevel() == logging.WARNING: output_log_level = logging.WARNING else: output_log_level = logging.INFO diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index e5be542..c3676d2 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -4,20 +4,20 @@ from argparse import Action, ArgumentParser from borgmatic.config import collect SUBPARSER_ALIASES = { - 'rcreate': ['init', '--init', '-I'], - 'prune': ['--prune', '-p'], + 'rcreate': ['init', '-I'], + 'prune': ['-p'], 'compact': [], - 'create': ['--create', '-C'], - 'check': ['--check', '-k'], - 'extract': ['--extract', '-x'], - 'export-tar': ['--export-tar'], - 'mount': ['--mount', '-m'], - 'umount': ['--umount', '-u'], - 'restore': ['--restore', '-r'], + 'create': ['-C'], + 'check': ['-k'], + 'extract': ['-x'], + 'export-tar': [], + 'mount': ['-m'], + 'umount': ['-u'], + 'restore': ['-r'], 'rlist': [], - 'list': ['--list', '-l'], + 'list': ['-l'], 'rinfo': [], - 'info': ['--info', '-i'], + 'info': ['-i'], 'transfer': [], 'borg': [], } @@ -328,7 +328,7 @@ def make_parsers(): help='Display statistics of archive', ) prune_group.add_argument( - '--files', dest='files', default=False, action='store_true', help='Show per-file details' + '--list', dest='list_archives', action='store_true', help='List archives kept/pruned' ) prune_group.add_argument('-h', '--help', action='help', help='Show this help message and exit') @@ -387,7 +387,7 @@ def make_parsers(): help='Display statistics of archive', ) create_group.add_argument( - '--files', dest='files', default=False, action='store_true', help='Show per-file details' + '--list', '--files', dest='list_files', action='store_true', help='Show per-file details' ) create_group.add_argument( '--json', dest='json', default=False, action='store_true', help='Output results as JSON' @@ -505,14 +505,14 @@ def make_parsers(): '--destination', metavar='PATH', dest='destination', - help='Path to destination export tar file, or "-" for stdout (but be careful about dirtying output with --verbosity or --files)', + help='Path to destination export tar file, or "-" for stdout (but be careful about dirtying output with --verbosity or --list)', required=True, ) export_tar_group.add_argument( '--tar-filter', help='Name of filter program to pipe data through' ) export_tar_group.add_argument( - '--files', default=False, action='store_true', help='Show per-file details' + '--list', '--files', dest='list_files', action='store_true', help='Show per-file details' ) export_tar_group.add_argument( '--strip-components', diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index c03fc44..05c86df 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -298,7 +298,7 @@ def run_actions( local_path=local_path, remote_path=remote_path, stats=arguments['prune'].stats, - files=arguments['prune'].files, + list_archives=arguments['prune'].list_archives, ) command.execute_hook( hooks.get('after_prune'), @@ -379,7 +379,7 @@ def run_actions( progress=arguments['create'].progress, stats=arguments['create'].stats, json=arguments['create'].json, - files=arguments['create'].files, + list_files=arguments['create'].list_files, stream_processes=stream_processes, ) if json_output: # pragma: nocover @@ -504,7 +504,7 @@ def run_actions( local_path=local_path, remote_path=remote_path, tar_filter=arguments['export-tar'].tar_filter, - files=arguments['export-tar'].files, + list_files=arguments['export-tar'].list_files, strip_components=arguments['export-tar'].strip_components, ) if 'mount' in arguments: diff --git a/docs/how-to/monitor-your-backups.md b/docs/how-to/monitor-your-backups.md index 7e62dac..575f4af 100644 --- a/docs/how-to/monitor-your-backups.md +++ b/docs/how-to/monitor-your-backups.md @@ -158,9 +158,9 @@ itself. But the logs are only included for errors that occur when a `prune`, `compact`, `create`, or `check` action is run. You can customize the verbosity of the logs that are sent to Healthchecks with -borgmatic's `--monitoring-verbosity` flag. The `--files` and `--stats` flags -may also be of use. See `borgmatic --help` for more information. Additionally, -see the [borgmatic configuration +borgmatic's `--monitoring-verbosity` flag. The `--list` and `--stats` flags +may also be of use. See `borgmatic create --help` for more information. +Additionally, see the [borgmatic configuration file](https://torsion.org/borgmatic/docs/reference/configuration/) for additional Healthchecks options. diff --git a/docs/how-to/set-up-backups.md b/docs/how-to/set-up-backups.md index 142ea62..1814204 100644 --- a/docs/how-to/set-up-backups.md +++ b/docs/how-to/set-up-backups.md @@ -233,16 +233,16 @@ idea to test that borgmatic is working. So to run borgmatic and start a backup, you can invoke it like this: ```bash -sudo borgmatic create --verbosity 1 --files --stats +sudo borgmatic create --verbosity 1 --list --stats ``` -(No borgmatic `--files` flag? It's only present in newer versions of -borgmatic. So try leaving it out or upgrade borgmatic!) +(No borgmatic `--list` flag? Try `--files` instead, leave it out, or upgrade +borgmatic!) The `--verbosity` flag makes borgmatic show the steps it's performing. The -`--files` flag lists each file that's new or changed since the last backup. -And `--stats` shows summary information about the created archive. All of -these flags are optional. +`--list` flag lists each file that's new or changed since the last backup. And +`--stats` shows summary information about the created archive. All of these +flags are optional. As the command runs, you should eyeball the output to see if it matches your expectations based on your configuration. @@ -262,7 +262,7 @@ backup, *and* `check` backups for consistency problems due to things like file damage. For instance: ```bash -sudo borgmatic --verbosity 1 --files --stats +sudo borgmatic --verbosity 1 --list --stats ``` ## Autopilot diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 26c5698..00c8f65 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -107,13 +107,6 @@ def test_parse_arguments_with_list_json_overrides_default(): assert arguments['list'].json is True -def test_parse_arguments_with_dashed_list_json_overrides_default(): - arguments = module.parse_arguments('--list', '--json') - - assert 'list' in arguments - assert arguments['list'].json is True - - def test_parse_arguments_with_no_actions_defaults_to_all_actions_enabled(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) @@ -127,14 +120,14 @@ def test_parse_arguments_with_no_actions_defaults_to_all_actions_enabled(): def test_parse_arguments_with_no_actions_passes_argument_to_relevant_actions(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - arguments = module.parse_arguments('--stats', '--files') + arguments = module.parse_arguments('--stats', '--list') assert 'prune' in arguments assert arguments['prune'].stats - assert arguments['prune'].files + assert arguments['prune'].list_archives assert 'create' in arguments assert arguments['create'].stats - assert arguments['create'].files + assert arguments['create'].list_files assert 'check' in arguments @@ -191,16 +184,6 @@ def test_parse_arguments_with_prune_action_leaves_other_actions_disabled(): assert 'check' not in arguments -def test_parse_arguments_with_dashed_prune_action_leaves_other_actions_disabled(): - flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - - arguments = module.parse_arguments('--prune') - - assert 'prune' in arguments - assert 'create' not in arguments - assert 'check' not in arguments - - def test_parse_arguments_with_multiple_actions_leaves_other_action_disabled(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) @@ -211,16 +194,6 @@ def test_parse_arguments_with_multiple_actions_leaves_other_action_disabled(): assert 'check' in arguments -def test_parse_arguments_with_multiple_dashed_actions_leaves_other_action_disabled(): - flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - - arguments = module.parse_arguments('--create', '--check') - - assert 'prune' not in arguments - assert 'create' in arguments - assert 'check' in arguments - - def test_parse_arguments_with_invalid_arguments_exits(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) @@ -248,12 +221,6 @@ def test_parse_arguments_allows_encryption_mode_with_init(): module.parse_arguments('--config', 'myconfig', 'init', '--encryption', 'repokey') -def test_parse_arguments_allows_encryption_mode_with_dashed_init(): - flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - - module.parse_arguments('--config', 'myconfig', '--init', '--encryption', 'repokey') - - def test_parse_arguments_requires_encryption_mode_with_init(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) @@ -352,24 +319,12 @@ def test_parse_arguments_allows_archive_with_mount(): ) -def test_parse_arguments_allows_archive_with_dashed_extract(): - flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - - module.parse_arguments('--config', 'myconfig', '--extract', '--archive', 'test') - - def test_parse_arguments_allows_archive_with_restore(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) module.parse_arguments('--config', 'myconfig', 'restore', '--archive', 'test') -def test_parse_arguments_allows_archive_with_dashed_restore(): - flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - - module.parse_arguments('--config', 'myconfig', '--restore', '--archive', 'test') - - def test_parse_arguments_allows_archive_with_list(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) @@ -448,23 +403,23 @@ def test_parse_arguments_with_stats_flag_but_no_create_or_prune_flag_raises_valu module.parse_arguments('--stats', 'list') -def test_parse_arguments_with_files_and_create_flags_does_not_raise(): +def test_parse_arguments_with_list_and_create_flags_does_not_raise(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - module.parse_arguments('--files', 'create', 'list') + module.parse_arguments('--list', 'create') -def test_parse_arguments_with_files_and_prune_flags_does_not_raise(): +def test_parse_arguments_with_list_and_prune_flags_does_not_raise(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - module.parse_arguments('--files', 'prune', 'list') + module.parse_arguments('--list', 'prune') -def test_parse_arguments_with_files_flag_but_no_create_or_prune_or_restore_flag_raises_value_error(): +def test_parse_arguments_with_list_flag_but_no_relevant_action_raises_value_error(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) with pytest.raises(SystemExit): - module.parse_arguments('--files', 'list') + module.parse_arguments('--list', 'rcreate') def test_parse_arguments_allows_json_with_list_or_info(): @@ -474,12 +429,6 @@ def test_parse_arguments_allows_json_with_list_or_info(): module.parse_arguments('info', '--json') -def test_parse_arguments_allows_json_with_dashed_info(): - flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - - module.parse_arguments('--info', '--json') - - def test_parse_arguments_disallows_json_with_both_list_and_info(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 453dbf4..35fcd4d 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -1450,7 +1450,7 @@ def test_create_archive_with_files_calls_borg_with_list_parameter_and_warning_ou }, storage_config={}, local_borg_version='1.2.3', - files=True, + list_files=True, ) @@ -1490,7 +1490,7 @@ def test_create_archive_with_files_and_log_info_calls_borg_with_list_parameter_a }, storage_config={}, local_borg_version='1.2.3', - files=True, + list_files=True, ) diff --git a/tests/unit/borg/test_export_tar.py b/tests/unit/borg/test_export_tar.py index f8d5637..63fbcf9 100644 --- a/tests/unit/borg/test_export_tar.py +++ b/tests/unit/borg/test_export_tar.py @@ -219,7 +219,7 @@ def test_export_tar_archive_calls_borg_with_list_parameter(): destination_path='test.tar', storage_config={}, local_borg_version='1.2.3', - files=True, + list_files=True, ) diff --git a/tests/unit/borg/test_prune.py b/tests/unit/borg/test_prune.py index e57ce7f..7261280 100644 --- a/tests/unit/borg/test_prune.py +++ b/tests/unit/borg/test_prune.py @@ -219,7 +219,7 @@ def test_prune_archives_with_files_calls_borg_with_list_parameter_and_warning_ou storage_config={}, retention_config=retention_config, local_borg_version='1.2.3', - files=True, + list_archives=True, ) @@ -238,7 +238,7 @@ def test_prune_archives_with_files_and_log_info_calls_borg_with_list_parameter_a storage_config={}, retention_config=retention_config, local_borg_version='1.2.3', - files=True, + list_archives=True, ) diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 5a49aff..4b9f40f 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -400,7 +400,7 @@ def test_run_actions_calls_hooks_for_prune_action(): flexmock(module.command).should_receive('execute_hook').twice() arguments = { 'global': flexmock(monitoring_verbosity=1, dry_run=False), - 'prune': flexmock(stats=flexmock(), files=flexmock()), + 'prune': flexmock(stats=flexmock(), list_archives=flexmock()), } list( @@ -453,7 +453,7 @@ def test_run_actions_executes_and_calls_hooks_for_create_action(): arguments = { 'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock( - progress=flexmock(), stats=flexmock(), json=flexmock(), files=flexmock() + progress=flexmock(), stats=flexmock(), json=flexmock(), list_files=flexmock() ), } @@ -546,7 +546,7 @@ def test_run_actions_does_not_raise_for_export_tar_action(): paths=flexmock(), destination=flexmock(), tar_filter=flexmock(), - files=flexmock(), + list_files=flexmock(), strip_components=flexmock(), ), }