From 007ec0644ca3d35912d7af14d2e698f092931cf9 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 20 May 2018 22:11:40 -0700 Subject: [PATCH] Ignore "check_last" and consistency "prefix" when "archives" not in consistency checks. (#59) --- NEWS | 1 + borgmatic/borg/check.py | 32 +++++++---- borgmatic/config/schema.yaml | 2 +- borgmatic/tests/unit/borg/test_check.py | 71 +++++++++++++++++-------- 4 files changed, 72 insertions(+), 34 deletions(-) diff --git a/NEWS b/NEWS index 8de78b17c..b056d1ff7 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ * Add introductory screencast link to documentation. * Update tox.ini to only assume Python 3.x instead of Python 3.4 specifically. * #62: Update README to mention other ways of installing borgmatic. + * #59: Ignore "check_last" and consistency "prefix" when "archives" not in consistency checks. 1.1.15 * Support for Borg BORG_PASSCOMMAND environment variable to read a password from an external file. diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index 01bfafb9b..d5bdb8202 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -7,6 +7,7 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS DEFAULT_CHECKS = ('repository', 'archives') +DEFAULT_PREFIX = '{hostname}-' logger = logging.getLogger(__name__) @@ -34,7 +35,7 @@ def _parse_checks(consistency_config): return tuple(check for check in checks if check.lower() not in ('disabled', '')) or DEFAULT_CHECKS -def _make_check_flags(checks, check_last=None): +def _make_check_flags(checks, check_last=None, prefix=None): ''' Given a parsed sequence of checks, transform it into tuple of command-line flags. @@ -47,17 +48,30 @@ def _make_check_flags(checks, check_last=None): ('--repository-only',) However, if both "repository" and "archives" are in checks, then omit them from the returned - flags because Borg does both checks by default. Additionally, if a check_last value is given, - a "--last" flag will be added. + flags because Borg does both checks by default. + + Additionally, if a check_last value is given and "archives" is in checks, then include a + "--last" flag. And if a prefix value is given and "archives" is in checks, then include a + "--prefix" flag. ''' - last_flag = ('--last', str(check_last)) if check_last else () + if 'archives' in checks: + last_flags = ('--last', str(check_last)) if check_last else () + prefix_flags = ('--prefix', prefix) if prefix else ('--prefix', DEFAULT_PREFIX) + else: + last_flags = () + prefix_flags = () + if check_last: + logger.warn('Ignoring check_last option, as "archives" is not in consistency checks.') + if prefix: + logger.warn('Ignoring consistency prefix option, as "archives" is not in consistency checks.') + if set(DEFAULT_CHECKS).issubset(set(checks)): - return last_flag + return last_flags + prefix_flags return tuple( '--{}-only'.format(check) for check in checks if check in DEFAULT_CHECKS - ) + last_flag + ) + last_flags + prefix_flags def check_archives(verbosity, repository, storage_config, consistency_config, local_path='borg', @@ -81,14 +95,12 @@ def check_archives(verbosity, repository, storage_config, consistency_config, lo VERBOSITY_SOME: ('--info',), VERBOSITY_LOTS: ('--debug',), }.get(verbosity, ()) - - prefix = consistency_config.get('prefix', '{hostname}-') - prefix_flags = ('--prefix', prefix) if prefix else () + prefix = consistency_config.get('prefix') full_command = ( local_path, 'check', repository, - ) + _make_check_flags(checks, check_last) + prefix_flags + remote_path_flags + lock_wait_flags + verbosity_flags + ) + _make_check_flags(checks, check_last, prefix) + remote_path_flags + lock_wait_flags + verbosity_flags # The check command spews to stdout/stderr even without the verbose flag. Suppress it. stdout = None if verbosity_flags else open(os.devnull, 'w') diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 1a61ec2a0..8706f43d6 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -222,7 +222,7 @@ map: prefix: type: scalar desc: | - When performing consistency checks, only consider archive names starting with + When performing the "archives" check, only consider archive names starting with this prefix. Borg placeholders can be used. See the output of "borg help placeholders" for details. Default is "{hostname}-". example: sourcehostname diff --git a/borgmatic/tests/unit/borg/test_check.py b/borgmatic/tests/unit/borg/test_check.py index 22c403aea..b5d83786f 100644 --- a/borgmatic/tests/unit/borg/test_check.py +++ b/borgmatic/tests/unit/borg/test_check.py @@ -42,40 +42,64 @@ def test_parse_checks_with_disabled_returns_no_checks(): assert checks == () -def test_make_check_flags_with_checks_returns_flags(): +def test_make_check_flags_with_repository_check_returns_flag(): flags = module._make_check_flags(('repository',)) assert flags == ('--repository-only',) -def test_make_check_flags_with_extract_check_does_not_make_extract_flag(): +def test_make_check_flags_with_extract_omits_extract_flag(): flags = module._make_check_flags(('extract',)) assert flags == () -def test_make_check_flags_with_default_checks_returns_no_flags(): +def test_make_check_flags_with_default_checks_returns_default_flags(): flags = module._make_check_flags(module.DEFAULT_CHECKS) - assert flags == () + assert flags == ('--prefix', module.DEFAULT_PREFIX) -def test_make_check_flags_with_all_checks_returns_no_flags(): +def test_make_check_flags_with_all_checks_returns_default_flags(): flags = module._make_check_flags(module.DEFAULT_CHECKS + ('extract',)) - assert flags == () + assert flags == ('--prefix', module.DEFAULT_PREFIX) -def test_make_check_flags_with_checks_and_last_returns_flags_including_last(): +def test_make_check_flags_with_archives_check_and_last_includes_last_flag(): + flags = module._make_check_flags(('archives',), check_last=3) + + assert flags == ('--archives-only', '--last', '3', '--prefix', module.DEFAULT_PREFIX) + + +def test_make_check_flags_with_repository_check_and_last_omits_last_flag(): flags = module._make_check_flags(('repository',), check_last=3) - assert flags == ('--repository-only', '--last', '3') + assert flags == ('--repository-only',) -def test_make_check_flags_with_default_checks_and_last_returns_last_flag(): +def test_make_check_flags_with_default_checks_and_last_includes_last_flag(): flags = module._make_check_flags(module.DEFAULT_CHECKS, check_last=3) - assert flags == ('--last', '3') + assert flags == ('--last', '3', '--prefix', module.DEFAULT_PREFIX) + + +def test_make_check_flags_with_archives_check_and_prefix_includes_prefix_flag(): + flags = module._make_check_flags(('archives',), prefix='foo-') + + assert flags == ('--archives-only', '--prefix', 'foo-') + + +def test_make_check_flags_with_repository_check_and_prefix_omits_prefix_flag(): + flags = module._make_check_flags(('repository',), prefix='foo-') + + assert flags == ('--repository-only',) + + +def test_make_check_flags_with_default_checks_and_prefix_includes_prefix_flag(): + flags = module._make_check_flags(module.DEFAULT_CHECKS, prefix='foo-') + + assert flags == ('--prefix', 'foo-') @pytest.mark.parametrize( @@ -91,10 +115,10 @@ def test_check_archives_calls_borg_with_parameters(checks): check_last = flexmock() consistency_config = {'check_last': check_last} flexmock(module).should_receive('_parse_checks').and_return(checks) - flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last).and_return(()) + flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last, None).and_return(()) stdout = flexmock() insert_subprocess_mock( - ('borg', 'check', 'repo', '--prefix', '{hostname}-'), + ('borg', 'check', 'repo'), stdout=stdout, stderr=STDOUT, ) flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout) @@ -131,7 +155,7 @@ def test_check_archives_with_verbosity_some_calls_borg_with_info_parameter(): flexmock(module).should_receive('_parse_checks').and_return(checks) flexmock(module).should_receive('_make_check_flags').and_return(()) insert_subprocess_mock( - ('borg', 'check', 'repo', '--prefix', '{hostname}-', '--info'), + ('borg', 'check', 'repo', '--info'), stdout=None, stderr=STDOUT, ) @@ -149,7 +173,7 @@ def test_check_archives_with_verbosity_lots_calls_borg_with_debug_parameter(): flexmock(module).should_receive('_parse_checks').and_return(checks) flexmock(module).should_receive('_make_check_flags').and_return(()) insert_subprocess_mock( - ('borg', 'check', 'repo', '--prefix', '{hostname}-', '--debug'), + ('borg', 'check', 'repo', '--debug'), stdout=None, stderr=STDOUT, ) @@ -179,10 +203,10 @@ def test_check_archives_with_local_path_calls_borg_via_local_path(): check_last = flexmock() consistency_config = {'check_last': check_last} flexmock(module).should_receive('_parse_checks').and_return(checks) - flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last).and_return(()) + flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last, None).and_return(()) stdout = flexmock() insert_subprocess_mock( - ('borg1', 'check', 'repo', '--prefix', '{hostname}-'), + ('borg1', 'check', 'repo'), stdout=stdout, stderr=STDOUT, ) flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout) @@ -202,10 +226,10 @@ def test_check_archives_with_remote_path_calls_borg_with_remote_path_parameters( check_last = flexmock() consistency_config = {'check_last': check_last} flexmock(module).should_receive('_parse_checks').and_return(checks) - flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last).and_return(()) + flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last, None).and_return(()) stdout = flexmock() insert_subprocess_mock( - ('borg', 'check', 'repo', '--prefix', '{hostname}-', '--remote-path', 'borg1'), + ('borg', 'check', 'repo', '--remote-path', 'borg1'), stdout=stdout, stderr=STDOUT, ) flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout) @@ -225,10 +249,10 @@ def test_check_archives_with_lock_wait_calls_borg_with_lock_wait_parameters(): check_last = flexmock() consistency_config = {'check_last': check_last} flexmock(module).should_receive('_parse_checks').and_return(checks) - flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last).and_return(()) + flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last, None).and_return(()) stdout = flexmock() insert_subprocess_mock( - ('borg', 'check', 'repo', '--prefix', '{hostname}-', '--lock-wait', '5'), + ('borg', 'check', 'repo', '--lock-wait', '5'), stdout=stdout, stderr=STDOUT, ) flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout) @@ -245,12 +269,13 @@ def test_check_archives_with_lock_wait_calls_borg_with_lock_wait_parameters(): def test_check_archives_with_retention_prefix(): checks = ('repository',) check_last = flexmock() - consistency_config = {'check_last': check_last, 'prefix': 'foo-'} + prefix = 'foo-' + consistency_config = {'check_last': check_last, 'prefix': prefix} flexmock(module).should_receive('_parse_checks').and_return(checks) - flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last).and_return(()) + flexmock(module).should_receive('_make_check_flags').with_args(checks, check_last, prefix).and_return(()) stdout = flexmock() insert_subprocess_mock( - ('borg', 'check', 'repo', '--prefix', 'foo-'), + ('borg', 'check', 'repo'), stdout=stdout, stderr=STDOUT, )