From 5e231af95d1818d0c80ce25d7e34b8ada5ea4a99 Mon Sep 17 00:00:00 2001 From: Thomas LEVEIL Date: Sun, 1 Jul 2018 20:55:59 +0200 Subject: [PATCH 1/5] add support for `--list --json` --- borgmatic/borg/list.py | 17 ++++++++++++++-- borgmatic/commands/borgmatic.py | 20 +++++++++++++++++-- .../integration/commands/test_borgmatic.py | 11 ++++++++++ borgmatic/tests/unit/borg/test_list.py | 13 +++++++++++- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index 989f8817..89fe1303 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -1,5 +1,8 @@ +import sys + import logging import subprocess +import tempfile from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS @@ -7,7 +10,7 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS logger = logging.getLogger(__name__) -def list_archives(verbosity, repository, storage_config, local_path='borg', remote_path=None): +def list_archives(verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False): ''' Given a verbosity flag, a local or remote repository path, and a storage config dict, list Borg archives in the repository. @@ -18,6 +21,7 @@ def list_archives(verbosity, repository, storage_config, local_path='borg', remo (local_path, 'list', repository) + (('--remote-path', remote_path) if remote_path else ()) + (('--lock-wait', str(lock_wait)) if lock_wait else ()) + + (('--json',) if json else ()) + { VERBOSITY_SOME: ('--info',), VERBOSITY_LOTS: ('--debug',), @@ -25,4 +29,13 @@ def list_archives(verbosity, repository, storage_config, local_path='borg', remo ) logger.debug(' '.join(full_command)) - subprocess.check_call(full_command) + + with tempfile.TemporaryFile() as f_output: + subprocess.check_call(full_command, stdout=f_output) + f_output.seek(0) + output = f_output.read().decode() + + if json: + return output + else: + sys.stdout.write(output) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index ee225088..3358333d 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -1,4 +1,4 @@ - +import json from argparse import ArgumentParser import logging import os @@ -76,6 +76,13 @@ def parse_arguments(*arguments): action='store_true', help='Display summary information on archives', ) + parser.add_argument( + '--json', + dest='json', + default=False, + action='store_true', + help='Output results from the --list option as json', + ) parser.add_argument( '-n', '--dry-run', dest='dry_run', @@ -90,6 +97,9 @@ def parse_arguments(*arguments): args = parser.parse_args(arguments) + if args.json and not args.list: + raise ValueError("The --json option can only be used with the --list option") + # If any of the action flags are explicitly requested, leave them as-is. Otherwise, assume # defaults: Mutate the given arguments to enable the default actions. if args.prune or args.create or args.check or args.list or args.info: @@ -121,6 +131,7 @@ def run_configuration(config_filename, args): # pragma: no cover if args.create: hook.execute_hook(hooks.get('before_backup'), config_filename, 'pre-backup') + json_results = [] for unexpanded_repository in location['repositories']: repository = os.path.expanduser(unexpanded_repository) dry_run_label = ' (dry run; not making any changes)' if args.dry_run else '' @@ -158,13 +169,16 @@ def run_configuration(config_filename, args): # pragma: no cover ) if args.list: logger.info('{}: Listing archives'.format(repository)) - borg_list.list_archives( + result = borg_list.list_archives( args.verbosity, repository, storage, local_path=local_path, remote_path=remote_path, + json=args.json, ) + if args.json: + json_results.append(json.loads(result)) if args.info: logger.info('{}: Displaying summary info for archives'.format(repository)) borg_info.display_archives_info( @@ -174,6 +188,8 @@ def run_configuration(config_filename, args): # pragma: no cover local_path=local_path, remote_path=remote_path, ) + if args.json: + sys.stdout.write(json.dumps(json_results)) if args.create: hook.execute_hook(hooks.get('after_backup'), config_filename, 'post-backup') diff --git a/borgmatic/tests/integration/commands/test_borgmatic.py b/borgmatic/tests/integration/commands/test_borgmatic.py index 6207cdb5..8cd1f76a 100644 --- a/borgmatic/tests/integration/commands/test_borgmatic.py +++ b/borgmatic/tests/integration/commands/test_borgmatic.py @@ -15,6 +15,7 @@ def test_parse_arguments_with_no_arguments_uses_defaults(): assert parser.config_paths == config_paths assert parser.excludes_filename == None assert parser.verbosity is None + assert parser.json is False def test_parse_arguments_with_path_arguments_overrides_defaults(): @@ -47,6 +48,11 @@ def test_parse_arguments_with_verbosity_flag_overrides_default(): assert parser.verbosity == 1 +def test_parse_arguments_with_json_flag_overrides_default(): + parser = module.parse_arguments('--list', '--json') + assert parser.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']) @@ -82,3 +88,8 @@ def test_parse_arguments_with_invalid_arguments_exits(): with pytest.raises(SystemExit): module.parse_arguments('--posix-me-harder') + + +def test_parse_arguments_with_json_flag_but_no_list_flag_raises_value_error(): + with pytest.raises(ValueError): + module.parse_arguments('--json') diff --git a/borgmatic/tests/unit/borg/test_list.py b/borgmatic/tests/unit/borg/test_list.py index 1c6a69f7..f0e178da 100644 --- a/borgmatic/tests/unit/borg/test_list.py +++ b/borgmatic/tests/unit/borg/test_list.py @@ -8,7 +8,7 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS def insert_subprocess_mock(check_call_command, **kwargs): subprocess = flexmock(module.subprocess) - subprocess.should_receive('check_call').with_args(check_call_command, **kwargs).once() + subprocess.should_receive('check_call').with_args(check_call_command, stdout=object, **kwargs).once() LIST_COMMAND = ('borg', 'list', 'repo') @@ -44,6 +44,17 @@ def test_list_archives_with_verbosity_lots_calls_borg_with_debug_parameter(): ) +def test_list_archives_with_json_calls_borg_with_json_parameter(): + insert_subprocess_mock(LIST_COMMAND + ('--json',)) + + module.list_archives( + verbosity=None, + repository='repo', + storage_config={}, + json=True, + ) + + def test_list_archives_with_local_path_calls_borg_via_local_path(): insert_subprocess_mock(('borg1',) + LIST_COMMAND[1:]) -- 2.40.1 From 8e14846872283dfa175b5e6548ed320690dccb36 Mon Sep 17 00:00:00 2001 From: Thomas LEVEIL Date: Mon, 23 Jul 2018 00:38:14 +0200 Subject: [PATCH 2/5] [style] alphabetical ordering on the imports --- borgmatic/borg/list.py | 3 +-- borgmatic/commands/borgmatic.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index 89fe1303..ebfb66fc 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -1,7 +1,6 @@ -import sys - import logging import subprocess +import sys import tempfile from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 3358333d..c9bacdb7 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -1,5 +1,5 @@ -import json from argparse import ArgumentParser +import json import logging import os from subprocess import CalledProcessError -- 2.40.1 From 76b59539593fcb1fe3e825bd0e1cd452cbc8d806 Mon Sep 17 00:00:00 2001 From: Thomas LEVEIL Date: Mon, 23 Jul 2018 00:44:58 +0200 Subject: [PATCH 3/5] `list_archives()` unconditionally returns the output value (regardless of JSON or not) --- borgmatic/borg/list.py | 7 +------ borgmatic/commands/borgmatic.py | 6 ++++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index ebfb66fc..dca73393 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -32,9 +32,4 @@ def list_archives(verbosity, repository, storage_config, local_path='borg', remo with tempfile.TemporaryFile() as f_output: subprocess.check_call(full_command, stdout=f_output) f_output.seek(0) - output = f_output.read().decode() - - if json: - return output - else: - sys.stdout.write(output) + return f_output.read().decode() diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index c9bacdb7..609028f2 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -169,7 +169,7 @@ def run_configuration(config_filename, args): # pragma: no cover ) if args.list: logger.info('{}: Listing archives'.format(repository)) - result = borg_list.list_archives( + output = borg_list.list_archives( args.verbosity, repository, storage, @@ -178,7 +178,9 @@ def run_configuration(config_filename, args): # pragma: no cover json=args.json, ) if args.json: - json_results.append(json.loads(result)) + json_results.append(json.loads(output)) + else: + sys.stdout.write(output) if args.info: logger.info('{}: Displaying summary info for archives'.format(repository)) borg_info.display_archives_info( -- 2.40.1 From 7a4c41c12cda5602ef59c3964e62271cf277bdea Mon Sep 17 00:00:00 2001 From: Thomas LEVEIL Date: Mon, 23 Jul 2018 00:58:35 +0200 Subject: [PATCH 4/5] `list_archives()` uses `subprocess.check_output()` instead of `subprocess.check_call(..., stdout=temp_file)` --- borgmatic/borg/list.py | 8 ++------ borgmatic/tests/unit/borg/test_list.py | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index dca73393..4bc24f7d 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -1,7 +1,5 @@ import logging import subprocess -import sys -import tempfile from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS @@ -29,7 +27,5 @@ def list_archives(verbosity, repository, storage_config, local_path='borg', remo logger.debug(' '.join(full_command)) - with tempfile.TemporaryFile() as f_output: - subprocess.check_call(full_command, stdout=f_output) - f_output.seek(0) - return f_output.read().decode() + output = subprocess.check_output(full_command) + return output.decode() if output is not None else None diff --git a/borgmatic/tests/unit/borg/test_list.py b/borgmatic/tests/unit/borg/test_list.py index f0e178da..48bd095a 100644 --- a/borgmatic/tests/unit/borg/test_list.py +++ b/borgmatic/tests/unit/borg/test_list.py @@ -8,7 +8,7 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS def insert_subprocess_mock(check_call_command, **kwargs): subprocess = flexmock(module.subprocess) - subprocess.should_receive('check_call').with_args(check_call_command, stdout=object, **kwargs).once() + subprocess.should_receive('check_output').with_args(check_call_command, **kwargs).once() LIST_COMMAND = ('borg', 'list', 'repo') -- 2.40.1 From 35402c28e1fa50e3c4667a56138405c6baba04bf Mon Sep 17 00:00:00 2001 From: Thomas LEVEIL Date: Mon, 23 Jul 2018 02:08:56 +0200 Subject: [PATCH 5/5] [TEST] add unit test for `_run_commands` --- borgmatic/commands/borgmatic.py | 132 ++++++++++-------- borgmatic/tests/unit/commands/__init__.py | 0 .../tests/unit/commands/test_borgmatic.py | 38 +++++ 3 files changed, 109 insertions(+), 61 deletions(-) create mode 100644 borgmatic/tests/unit/commands/__init__.py create mode 100644 borgmatic/tests/unit/commands/test_borgmatic.py diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 609028f2..c8993ba1 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -131,67 +131,7 @@ def run_configuration(config_filename, args): # pragma: no cover if args.create: hook.execute_hook(hooks.get('before_backup'), config_filename, 'pre-backup') - json_results = [] - for unexpanded_repository in location['repositories']: - repository = os.path.expanduser(unexpanded_repository) - dry_run_label = ' (dry run; not making any changes)' if args.dry_run else '' - if args.prune: - logger.info('{}: Pruning archives{}'.format(repository, dry_run_label)) - borg_prune.prune_archives( - args.verbosity, - args.dry_run, - repository, - storage, - retention, - local_path=local_path, - remote_path=remote_path, - ) - if args.create: - logger.info('{}: Creating archive{}'.format(repository, dry_run_label)) - borg_create.create_archive( - args.verbosity, - args.dry_run, - repository, - location, - storage, - local_path=local_path, - remote_path=remote_path, - ) - if args.check: - logger.info('{}: Running consistency checks'.format(repository)) - borg_check.check_archives( - args.verbosity, - repository, - storage, - consistency, - local_path=local_path, - remote_path=remote_path, - ) - if args.list: - logger.info('{}: Listing archives'.format(repository)) - output = borg_list.list_archives( - args.verbosity, - repository, - storage, - local_path=local_path, - remote_path=remote_path, - json=args.json, - ) - if args.json: - json_results.append(json.loads(output)) - else: - sys.stdout.write(output) - if args.info: - logger.info('{}: Displaying summary info for archives'.format(repository)) - borg_info.display_archives_info( - args.verbosity, - repository, - storage, - local_path=local_path, - remote_path=remote_path, - ) - if args.json: - sys.stdout.write(json.dumps(json_results)) + _run_commands(args, consistency, local_path, location, remote_path, retention, storage) if args.create: hook.execute_hook(hooks.get('after_backup'), config_filename, 'post-backup') @@ -200,6 +140,76 @@ def run_configuration(config_filename, args): # pragma: no cover raise +def _run_commands(args, consistency, local_path, location, remote_path, retention, storage): + json_results = [] + for unexpanded_repository in location['repositories']: + _run_commands_on_repository(args, consistency, json_results, local_path, location, remote_path, retention, + storage, unexpanded_repository) + if args.json: + sys.stdout.write(json.dumps(json_results)) + + +def _run_commands_on_repository(args, consistency, json_results, local_path, location, remote_path, retention, storage, + unexpanded_repository): # pragma: no cover + repository = os.path.expanduser(unexpanded_repository) + dry_run_label = ' (dry run; not making any changes)' if args.dry_run else '' + if args.prune: + logger.info('{}: Pruning archives{}'.format(repository, dry_run_label)) + borg_prune.prune_archives( + args.verbosity, + args.dry_run, + repository, + storage, + retention, + local_path=local_path, + remote_path=remote_path, + ) + if args.create: + logger.info('{}: Creating archive{}'.format(repository, dry_run_label)) + borg_create.create_archive( + args.verbosity, + args.dry_run, + repository, + location, + storage, + local_path=local_path, + remote_path=remote_path, + ) + if args.check: + logger.info('{}: Running consistency checks'.format(repository)) + borg_check.check_archives( + args.verbosity, + repository, + storage, + consistency, + local_path=local_path, + remote_path=remote_path, + ) + if args.list: + logger.info('{}: Listing archives'.format(repository)) + output = borg_list.list_archives( + args.verbosity, + repository, + storage, + local_path=local_path, + remote_path=remote_path, + json=args.json, + ) + if args.json: + json_results.append(json.loads(output)) + else: + sys.stdout.write(output) + if args.info: + logger.info('{}: Displaying summary info for archives'.format(repository)) + borg_info.display_archives_info( + args.verbosity, + repository, + storage, + local_path=local_path, + remote_path=remote_path, + ) + + def main(): # pragma: no cover try: configure_signals() diff --git a/borgmatic/tests/unit/commands/__init__.py b/borgmatic/tests/unit/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/borgmatic/tests/unit/commands/test_borgmatic.py b/borgmatic/tests/unit/commands/test_borgmatic.py new file mode 100644 index 00000000..cab0b0f3 --- /dev/null +++ b/borgmatic/tests/unit/commands/test_borgmatic.py @@ -0,0 +1,38 @@ +from borgmatic.commands import borgmatic +from flexmock import flexmock +import json +import pytest +import sys + + +def test__run_commands_handles_multiple_json_outputs_in_array(): + # THEN + (flexmock(borgmatic) + .should_receive("_run_commands_on_repository") + .times(3) + .replace_with(lambda args, consistency, json_results, local_path, location, remote_path, retention, storage, + unexpanded_repository: json_results.append({"whatever": unexpanded_repository})) + ) + + (flexmock(sys.stdout) + .should_call("write") + .with_args(json.dumps(json.loads(''' + [ + {"whatever": "fake_repo1"}, + {"whatever": "fake_repo2"}, + {"whatever": "fake_repo3"} + ] + '''))) + ) + + borgmatic._run_commands(args=flexmock(json=True), + consistency=None, + local_path=None, + location={"repositories": [ + "fake_repo1", + "fake_repo2", + "fake_repo3" + ]}, + remote_path=None, + retention=None, + storage=None) -- 2.40.1