From df2d059af236ea3962273b7c9768e0bf875f414c Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 10 May 2015 22:00:31 -0700 Subject: [PATCH] New configuration section for customizing which Attic consistency checks run, if any. --- NEWS | 4 ++ README.md | 3 ++ atticmatic/attic.py | 67 +++++++++++++++++++++++--- atticmatic/command.py | 10 ++-- atticmatic/config.py | 51 +++++++++++++++----- atticmatic/tests/unit/test_attic.py | 70 ++++++++++++++++++++++++++-- atticmatic/tests/unit/test_config.py | 70 +++++++++++++++++++++++++--- sample/config | 10 +++- setup.py | 2 +- 9 files changed, 251 insertions(+), 36 deletions(-) diff --git a/NEWS b/NEWS index 43351a5ce..5dbc3bcbc 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +0.0.6 + + * New configuration section for customizing which Attic consistency checks run, if any. + 0.0.5 * Fixed regression with --verbose output being buffered. This means dropping the helpful error diff --git a/README.md b/README.md index f39964623..ee9cb722a 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,9 @@ Here's an example config file: keep_weekly: 4 keep_monthly: 6 + [consistency] + checks: repository archives + Additionally, exclude patterns can be specified in a separate excludes config file, one pattern per line. diff --git a/atticmatic/attic.py b/atticmatic/attic.py index d0c678d68..fc8399781 100644 --- a/atticmatic/attic.py +++ b/atticmatic/attic.py @@ -26,7 +26,7 @@ def create_archive(excludes_filename, verbose, source_directories, repository): subprocess.check_call(command) -def make_prune_flags(retention_config): +def _make_prune_flags(retention_config): ''' Given a retention config dict mapping from option name to value, tranform it into an iterable of command-line name-value flag pairs. @@ -58,22 +58,77 @@ def prune_archives(verbose, repository, retention_config): repository, ) + tuple( element - for pair in make_prune_flags(retention_config) + for pair in _make_prune_flags(retention_config) for element in pair ) + (('--verbose',) if verbose else ()) subprocess.check_call(command) -def check_archives(verbose, repository): +DEFAULT_CHECKS = ('repository', 'archives') + + +def _parse_checks(consistency_config): ''' - Given a verbosity flag and a local or remote repository path, check the contained attic archives - for consistency. + Given a consistency config with a space-separated "checks" option, transform it to a tuple of + named checks to run. + + For example, given a retention config of: + + {'checks': 'repository archives'} + + This will be returned as: + + ('repository', 'archives') + + If no "checks" option is present, return the DEFAULT_CHECKS. If the checks value is the string + "disabled", return an empty tuple, meaning that no checks should be run. ''' + checks = consistency_config.get('checks', '').strip() + if not checks: + return DEFAULT_CHECKS + + return tuple( + check for check in consistency_config['checks'].split(' ') + if check.lower() not in ('disabled', '') + ) + + +def _make_check_flags(checks): + ''' + Given a parsed sequence of checks, transform it into tuple of command-line flags. + + For example, given parsed checks of: + + ('repository',) + + This will be returned as: + + ('--repository-only',) + ''' + if checks == DEFAULT_CHECKS: + return () + + return tuple( + '--{}-only'.format(check) for check in checks + ) + + +def check_archives(verbose, repository, consistency_config): + ''' + Given a verbosity flag, a local or remote repository path, and a consistency config dict, check + the contained attic archives for consistency. + + If there are no consistency checks to run, skip running them. + ''' + checks = _parse_checks(consistency_config) + if not checks: + return + command = ( 'attic', 'check', repository, - ) + (('--verbose',) if verbose else ()) + ) + _make_check_flags(checks) + (('--verbose',) if verbose else ()) # Attic's check command spews to stdout even without the verbose flag. Suppress it. stdout = None if verbose else open(os.devnull, 'w') diff --git a/atticmatic/command.py b/atticmatic/command.py index 92976f0ea..0a844b20f 100644 --- a/atticmatic/command.py +++ b/atticmatic/command.py @@ -40,12 +40,12 @@ def parse_arguments(*arguments): def main(): try: args = parse_arguments(*sys.argv[1:]) - location_config, retention_config = parse_configuration(args.config_filename) - repository = location_config['repository'] + config = parse_configuration(args.config_filename) + repository = config.location['repository'] - create_archive(args.excludes_filename, args.verbose, **location_config) - prune_archives(args.verbose, repository, retention_config) - check_archives(args.verbose, repository) + create_archive(args.excludes_filename, args.verbose, **config.location) + prune_archives(args.verbose, repository, config.retention) + check_archives(args.verbose, repository, config.consistency) except (ValueError, IOError, CalledProcessError) as error: print(error, file=sys.stderr) sys.exit(1) diff --git a/atticmatic/config.py b/atticmatic/config.py index f953860a9..8254c8804 100644 --- a/atticmatic/config.py +++ b/atticmatic/config.py @@ -39,6 +39,12 @@ CONFIG_FORMAT = ( option('keep_yearly', int, required=False), option('prefix', required=False), ), + ), + Section_format( + 'consistency', + ( + option('checks', required=False), + ), ) ) @@ -49,20 +55,34 @@ def validate_configuration_format(parser, config_format): configuration file has the expected sections, that any required options are present in those sections, and that there aren't any unexpected options. + A section is required if any of its contained options are required. + Raise ValueError if anything is awry. ''' - section_names = parser.sections() - required_section_names = tuple(section.name for section in config_format) + section_names = set(parser.sections()) + required_section_names = tuple( + section.name for section in config_format + if any(option.required for option in section.options) + ) - if set(section_names) != set(required_section_names): + unknown_section_names = section_names - set( + section_format.name for section_format in config_format + ) + if unknown_section_names: raise ValueError( - 'Expected config sections {} but found sections: {}'.format( - ', '.join(required_section_names), - ', '.join(section_names) - ) + 'Unknown config sections found: {}'.format(', '.join(unknown_section_names)) + ) + + missing_section_names = set(required_section_names) - section_names + if missing_section_names: + raise ValueError( + 'Missing config sections: {}'.format(', '.join(missing_section_names)) ) for section_format in config_format: + if section_format.name not in section_names: + continue + option_names = parser.options(section_format.name) expected_options = section_format.options @@ -90,6 +110,11 @@ def validate_configuration_format(parser, config_format): ) +# Describes a parsed configuration, where each attribute is the name of a configuration file section +# and each value is a dict of that section's parsed options. +Parsed_config = namedtuple('Config', (section_format.name for section_format in CONFIG_FORMAT)) + + def parse_section_options(parser, section_format): ''' Given an open ConfigParser and an expected section format, return the option values from that @@ -112,8 +137,8 @@ def parse_section_options(parser, section_format): def parse_configuration(config_filename): ''' - Given a config filename of the expected format, return the parsed configuration as a tuple of - (location config, retention config) where each config is a dict of that section's options. + Given a config filename of the expected format, return the parsed configuration as Parsed_config + data structure. Raise IOError if the file cannot be read, or ValueError if the format is not as expected. ''' @@ -122,7 +147,9 @@ def parse_configuration(config_filename): validate_configuration_format(parser, CONFIG_FORMAT) - return tuple( - parse_section_options(parser, section_format) - for section_format in CONFIG_FORMAT + return Parsed_config( + *( + parse_section_options(parser, section_format) + for section_format in CONFIG_FORMAT + ) ) diff --git a/atticmatic/tests/unit/test_attic.py b/atticmatic/tests/unit/test_attic.py index 2c93e8c78..26c629984 100644 --- a/atticmatic/tests/unit/test_attic.py +++ b/atticmatic/tests/unit/test_attic.py @@ -11,6 +11,12 @@ def insert_subprocess_mock(check_call_command, **kwargs): flexmock(module).subprocess = subprocess +def insert_subprocess_never(): + subprocess = flexmock() + subprocess.should_receive('check_call').never() + flexmock(module).subprocess = subprocess + + def insert_platform_mock(): flexmock(module).platform = flexmock().should_receive('node').and_return('host').mock @@ -70,14 +76,14 @@ def test_make_prune_flags_should_return_flags_from_config(): ) ) - result = module.make_prune_flags(retention_config) + result = module._make_prune_flags(retention_config) assert tuple(result) == BASE_PRUNE_FLAGS def test_prune_archives_should_call_attic_with_parameters(): retention_config = flexmock() - flexmock(module).should_receive('make_prune_flags').with_args(retention_config).and_return( + flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return( BASE_PRUNE_FLAGS, ) insert_subprocess_mock( @@ -96,7 +102,7 @@ def test_prune_archives_should_call_attic_with_parameters(): def test_prune_archives_with_verbose_should_call_attic_with_verbose_parameters(): retention_config = flexmock() - flexmock(module).should_receive('make_prune_flags').with_args(retention_config).and_return( + flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return( BASE_PRUNE_FLAGS, ) insert_subprocess_mock( @@ -113,7 +119,46 @@ def test_prune_archives_with_verbose_should_call_attic_with_verbose_parameters() ) +def test_parse_checks_returns_them_as_tuple(): + checks = module._parse_checks({'checks': 'foo disabled bar'}) + + assert checks == ('foo', 'bar') + + +def test_parse_checks_with_missing_value_returns_defaults(): + checks = module._parse_checks({}) + + assert checks == module.DEFAULT_CHECKS + + +def test_parse_checks_with_blank_value_returns_defaults(): + checks = module._parse_checks({'checks': ''}) + + assert checks == module.DEFAULT_CHECKS + + +def test_parse_checks_with_disabled_returns_no_checks(): + checks = module._parse_checks({'checks': 'disabled'}) + + assert checks == () + + +def test_make_check_flags_with_checks_returns_flags(): + flags = module._make_check_flags(('foo', 'bar')) + + assert flags == ('--foo-only', '--bar-only') + + +def test_make_check_flags_with_default_checks_returns_no_flags(): + flags = module._make_check_flags(module.DEFAULT_CHECKS) + + assert flags == () + + def test_check_archives_should_call_attic_with_parameters(): + consistency_config = flexmock() + flexmock(module).should_receive('_parse_checks').and_return(flexmock()) + flexmock(module).should_receive('_make_check_flags').and_return(()) stdout = flexmock() insert_subprocess_mock( ('attic', 'check', 'repo'), @@ -127,10 +172,14 @@ def test_check_archives_should_call_attic_with_parameters(): module.check_archives( verbose=False, repository='repo', + consistency_config=consistency_config, ) def test_check_archives_with_verbose_should_call_attic_with_verbose_parameters(): + consistency_config = flexmock() + flexmock(module).should_receive('_parse_checks').and_return(flexmock()) + flexmock(module).should_receive('_make_check_flags').and_return(()) insert_subprocess_mock( ('attic', 'check', 'repo', '--verbose'), stdout=None, @@ -141,4 +190,19 @@ def test_check_archives_with_verbose_should_call_attic_with_verbose_parameters() module.check_archives( verbose=True, repository='repo', + consistency_config=consistency_config, ) + + +def test_check_archives_without_any_checks_should_bail(): + consistency_config = flexmock() + flexmock(module).should_receive('_parse_checks').and_return(()) + insert_subprocess_never() + + module.check_archives( + verbose=False, + repository='repo', + consistency_config=consistency_config, + ) + + diff --git a/atticmatic/tests/unit/test_config.py b/atticmatic/tests/unit/test_config.py index 0576dc8f6..1c9fcb654 100644 --- a/atticmatic/tests/unit/test_config.py +++ b/atticmatic/tests/unit/test_config.py @@ -41,19 +41,55 @@ def test_validate_configuration_format_with_valid_config_should_not_raise(): module.validate_configuration_format(parser, config_format) -def test_validate_configuration_format_with_missing_section_should_raise(): +def test_validate_configuration_format_with_missing_required_section_should_raise(): parser = flexmock() parser.should_receive('sections').and_return(('section',)) config_format = ( - module.Section_format('section', options=()), - module.Section_format('missing', options=()), + module.Section_format( + 'section', + options=( + module.Config_option('stuff', str, required=True), + ), + ), + # At least one option in this section is required, so the section is required. + module.Section_format( + 'missing', + options=( + module.Config_option('such', str, required=False), + module.Config_option('things', str, required=True), + ), + ), ) with assert_raises(ValueError): module.validate_configuration_format(parser, config_format) -def test_validate_configuration_format_with_extra_section_should_raise(): +def test_validate_configuration_format_with_missing_optional_section_should_not_raise(): + parser = flexmock() + parser.should_receive('sections').and_return(('section',)) + parser.should_receive('options').with_args('section').and_return(('stuff',)) + config_format = ( + module.Section_format( + 'section', + options=( + module.Config_option('stuff', str, required=True), + ), + ), + # No options in the section are required, so the section is optional. + module.Section_format( + 'missing', + options=( + module.Config_option('such', str, required=False), + module.Config_option('things', str, required=False), + ), + ), + ) + + module.validate_configuration_format(parser, config_format) + + +def test_validate_configuration_format_with_unknown_section_should_raise(): parser = flexmock() parser.should_receive('sections').and_return(('section', 'extra')) config_format = ( @@ -139,6 +175,26 @@ def test_parse_section_options_should_return_section_options(): ) +def test_parse_section_options_for_missing_section_should_return_empty_dict(): + parser = flexmock() + parser.should_receive('get').never() + parser.should_receive('getint').never() + parser.should_receive('has_option').with_args('section', 'foo').and_return(False) + parser.should_receive('has_option').with_args('section', 'bar').and_return(False) + + section_format = module.Section_format( + 'section', + ( + module.Config_option('foo', str, required=False), + module.Config_option('bar', int, required=False), + ), + ) + + config = module.parse_section_options(parser, section_format) + + assert config == OrderedDict() + + def insert_mock_parser(): parser = flexmock() parser.should_receive('readfp') @@ -154,13 +210,13 @@ def test_parse_configuration_should_return_section_configs(): mock_module.should_receive('validate_configuration_format').with_args( parser, module.CONFIG_FORMAT, ).once() - mock_section_configs = (flexmock(), flexmock()) + mock_section_configs = (flexmock(),) * len(module.CONFIG_FORMAT) for section_format, section_config in zip(module.CONFIG_FORMAT, mock_section_configs): mock_module.should_receive('parse_section_options').with_args( parser, section_format, ).and_return(section_config).once() - section_configs = module.parse_configuration('filename') + parsed_config = module.parse_configuration('filename') - assert section_configs == mock_section_configs + assert parsed_config == module.Parsed_config(*mock_section_configs) diff --git a/sample/config b/sample/config index aa2acf3f5..b13afbe60 100644 --- a/sample/config +++ b/sample/config @@ -6,8 +6,8 @@ source_directories: /home /etc repository: user@backupserver:sourcehostname.attic [retention] -# Retention policy for how many backups to keep in each category. -# See https://attic-backup.org/usage.html#attic-prune for details. +# Retention policy for how many backups to keep in each category. See +# https://attic-backup.org/usage.html#attic-prune for details. #keep_within: 3h #keep_hourly: 24 keep_daily: 7 @@ -15,3 +15,9 @@ keep_weekly: 4 keep_monthly: 6 keep_yearly: 1 #prefix: sourcehostname + +[consistency] +# Space-separated list of consistency checks to run: "repository", "archives", +# or both. Defaults to both. Set to "disabled" to disable all consistency +# checks. See https://attic-backup.org/usage.html#attic-check for details. +checks: repository archives diff --git a/setup.py b/setup.py index a771454fb..6fcd51d7c 100644 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ from setuptools import setup, find_packages setup( name='atticmatic', - version='0.0.5', + version='0.0.6', description='A wrapper script for Attic backup software that creates and prunes backups', author='Dan Helfman', author_email='witten@torsion.org',