From ebeb5efe0543d19b9b36f0fa4571e7741629c771 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 12 Nov 2019 11:10:47 -0800 Subject: [PATCH] More accurately detecting restore of unconfigured database (#228). --- borgmatic/hooks/dump.py | 27 +++++++++++++++++---------- tests/unit/hooks/test_dump.py | 21 +++++++++++++-------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/borgmatic/hooks/dump.py b/borgmatic/hooks/dump.py index c72b45d4d..a366f8980 100644 --- a/borgmatic/hooks/dump.py +++ b/borgmatic/hooks/dump.py @@ -79,9 +79,6 @@ def get_database_configurations(databases, names): database names, filter down and yield the configuration for just the named databases. Additionally, if a database configuration is named "all", project out that configuration for each named database. - - Raise ValueError if one of the database names cannot be matched to a database in borgmatic's - database configuration. ''' named_databases = {database['name']: database for database in databases} @@ -95,12 +92,6 @@ def get_database_configurations(databases, names): yield {**named_databases['all'], **{'name': name}} continue - raise ValueError( - 'Cannot restore database "{}", as it is not defined in borgmatic\'s configuration'.format( - name - ) - ) - def get_per_hook_database_configurations(hooks, names, dump_patterns): ''' @@ -119,7 +110,7 @@ def get_per_hook_database_configurations(hooks, names, dump_patterns): database configuration. ''' # TODO: Need to filter names by database type? Maybe take a database --type argument to disambiguate. - return { + hook_databases = { hook_name: list( get_database_configurations( hooks.get(hook_name), @@ -129,3 +120,19 @@ def get_per_hook_database_configurations(hooks, names, dump_patterns): for hook_name in DATABASE_HOOK_NAMES if hook_name in hooks } + + if not names or 'all' in names: + return hook_databases + + found_names = { + database['name'] for databases in hook_databases.values() for database in databases + } + missing_names = sorted(set(names) - found_names) + if missing_names: + raise ValueError( + 'Cannot restore database(s) {} missing from borgmatic\'s configuration'.format( + ', '.join(missing_names) + ) + ) + + return hook_databases diff --git a/tests/unit/hooks/test_dump.py b/tests/unit/hooks/test_dump.py index 0a54e2abf..dd86ba0f9 100644 --- a/tests/unit/hooks/test_dump.py +++ b/tests/unit/hooks/test_dump.py @@ -94,18 +94,11 @@ def test_get_database_configurations_matches_all_database(): ] -def test_get_database_configurations_with_unknown_database_name_raises(): - databases = [{'name': 'foo', 'hostname': 'example.org'}] - - with pytest.raises(ValueError): - list(module.get_database_configurations(databases, ('foo', 'bar'))) - - def test_get_per_hook_database_configurations_partitions_by_hook(): hooks = {'postgresql_databases': [flexmock()]} names = ('foo', 'bar') dump_patterns = flexmock() - expected_config = {'postgresql_databases': [flexmock()]} + expected_config = {'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}]} flexmock(module).should_receive('get_database_configurations').with_args( hooks['postgresql_databases'], names ).and_return(expected_config['postgresql_databases']) @@ -129,3 +122,15 @@ def test_get_per_hook_database_configurations_defaults_to_detected_database_name config = module.get_per_hook_database_configurations(hooks, names, dump_patterns) assert config == expected_config + + +def test_get_per_hook_database_configurations_with_unknown_database_name_raises(): + hooks = {'postgresql_databases': [flexmock()]} + names = ('foo', 'bar') + dump_patterns = flexmock() + flexmock(module).should_receive('get_database_configurations').with_args( + hooks['postgresql_databases'], names + ).and_return([]) + + with pytest.raises(ValueError): + module.get_per_hook_database_configurations(hooks, names, dump_patterns)