Fix the "create" action with the "--dry-run" flag querying for databases when a PostgreSQL/MySQL "all" database is configured.

This commit is contained in:
Dan Helfman 2023-02-26 22:15:12 -08:00
parent c6582e1171
commit da321e180d
5 changed files with 77 additions and 24 deletions

2
NEWS
View file

@ -4,6 +4,8 @@
This lines up with the new behavior in Borg 2.0.0b5. This lines up with the new behavior in Borg 2.0.0b5.
* Internally support new Borg 2.0.0b5 "--filter" status characters / item flags for the "create" * Internally support new Borg 2.0.0b5 "--filter" status characters / item flags for the "create"
action. action.
* Fix the "create" action with the "--dry-run" flag querying for databases when a PostgreSQL/MySQL
"all" database is configured. Now, these queries are skipped due to the dry run.
1.7.7 1.7.7
* #642: Add MySQL database hook "add_drop_database" configuration option to control whether dumped * #642: Add MySQL database hook "add_drop_database" configuration option to control whether dumped

View file

@ -24,7 +24,7 @@ def make_dump_path(location_config): # pragma: no cover
SYSTEM_DATABASE_NAMES = ('information_schema', 'mysql', 'performance_schema', 'sys') SYSTEM_DATABASE_NAMES = ('information_schema', 'mysql', 'performance_schema', 'sys')
def database_names_to_dump(database, extra_environment, log_prefix, dry_run_label): def database_names_to_dump(database, extra_environment, log_prefix, dry_run):
''' '''
Given a requested database config, return the corresponding sequence of database names to dump. Given a requested database config, return the corresponding sequence of database names to dump.
In the case of "all", query for the names of databases on the configured host and return them, In the case of "all", query for the names of databases on the configured host and return them,
@ -32,6 +32,8 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run_labe
''' '''
if database['name'] != 'all': if database['name'] != 'all':
return (database['name'],) return (database['name'],)
if dry_run:
return ()
show_command = ( show_command = (
('mysql',) ('mysql',)
@ -43,9 +45,7 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run_labe
+ ('--skip-column-names', '--batch') + ('--skip-column-names', '--batch')
+ ('--execute', 'show schemas') + ('--execute', 'show schemas')
) )
logger.debug( logger.debug(f'{log_prefix}: Querying for "all" MySQL databases to dump')
'{}: Querying for "all" MySQL databases to dump{}'.format(log_prefix, dry_run_label)
)
show_output = execute_command_and_capture_output( show_output = execute_command_and_capture_output(
show_command, extra_environment=extra_environment show_command, extra_environment=extra_environment
) )
@ -125,9 +125,13 @@ def dump_databases(databases, log_prefix, location_config, dry_run):
dump_path = make_dump_path(location_config) dump_path = make_dump_path(location_config)
extra_environment = {'MYSQL_PWD': database['password']} if 'password' in database else None extra_environment = {'MYSQL_PWD': database['password']} if 'password' in database else None
dump_database_names = database_names_to_dump( dump_database_names = database_names_to_dump(
database, extra_environment, log_prefix, dry_run_label database, extra_environment, log_prefix, dry_run
) )
if not dump_database_names: if not dump_database_names:
if dry_run:
continue
raise ValueError('Cannot find any MySQL databases to dump.') raise ValueError('Cannot find any MySQL databases to dump.')
if database['name'] == 'all' and database.get('format'): if database['name'] == 'all' and database.get('format'):

View file

@ -43,7 +43,7 @@ def make_extra_environment(database):
EXCLUDED_DATABASE_NAMES = ('template0', 'template1') EXCLUDED_DATABASE_NAMES = ('template0', 'template1')
def database_names_to_dump(database, extra_environment, log_prefix, dry_run_label): def database_names_to_dump(database, extra_environment, log_prefix, dry_run):
''' '''
Given a requested database config, return the corresponding sequence of database names to dump. Given a requested database config, return the corresponding sequence of database names to dump.
In the case of "all" when a database format is given, query for the names of databases on the In the case of "all" when a database format is given, query for the names of databases on the
@ -56,6 +56,8 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run_labe
return (requested_name,) return (requested_name,)
if not database.get('format'): if not database.get('format'):
return ('all',) return ('all',)
if dry_run:
return ()
list_command = ( list_command = (
('psql', '--list', '--no-password', '--csv', '--tuples-only') ('psql', '--list', '--no-password', '--csv', '--tuples-only')
@ -64,9 +66,7 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run_labe
+ (('--username', database['username']) if 'username' in database else ()) + (('--username', database['username']) if 'username' in database else ())
+ (tuple(database['list_options'].split(' ')) if 'list_options' in database else ()) + (tuple(database['list_options'].split(' ')) if 'list_options' in database else ())
) )
logger.debug( logger.debug(f'{log_prefix}: Querying for "all" PostgreSQL databases to dump')
'{}: Querying for "all" PostgreSQL databases to dump{}'.format(log_prefix, dry_run_label)
)
list_output = execute_command_and_capture_output( list_output = execute_command_and_capture_output(
list_command, extra_environment=extra_environment list_command, extra_environment=extra_environment
) )
@ -99,10 +99,13 @@ def dump_databases(databases, log_prefix, location_config, dry_run):
extra_environment = make_extra_environment(database) extra_environment = make_extra_environment(database)
dump_path = make_dump_path(location_config) dump_path = make_dump_path(location_config)
dump_database_names = database_names_to_dump( dump_database_names = database_names_to_dump(
database, extra_environment, log_prefix, dry_run_label database, extra_environment, log_prefix, dry_run
) )
if not dump_database_names: if not dump_database_names:
if dry_run:
continue
raise ValueError('Cannot find any PostgreSQL databases to dump.') raise ValueError('Cannot find any PostgreSQL databases to dump.')
for database_name in dump_database_names: for database_name in dump_database_names:

View file

@ -9,26 +9,36 @@ from borgmatic.hooks import mysql as module
def test_database_names_to_dump_passes_through_name(): def test_database_names_to_dump_passes_through_name():
extra_environment = flexmock() extra_environment = flexmock()
log_prefix = '' log_prefix = ''
dry_run_label = ''
names = module.database_names_to_dump( names = module.database_names_to_dump(
{'name': 'foo'}, extra_environment, log_prefix, dry_run_label {'name': 'foo'}, extra_environment, log_prefix, dry_run=False
) )
assert names == ('foo',) assert names == ('foo',)
def test_database_names_to_dump_bails_for_dry_run():
extra_environment = flexmock()
log_prefix = ''
flexmock(module).should_receive('execute_command_and_capture_output').never()
names = module.database_names_to_dump(
{'name': 'all'}, extra_environment, log_prefix, dry_run=True
)
assert names == ()
def test_database_names_to_dump_queries_mysql_for_database_names(): def test_database_names_to_dump_queries_mysql_for_database_names():
extra_environment = flexmock() extra_environment = flexmock()
log_prefix = '' log_prefix = ''
dry_run_label = ''
flexmock(module).should_receive('execute_command_and_capture_output').with_args( flexmock(module).should_receive('execute_command_and_capture_output').with_args(
('mysql', '--skip-column-names', '--batch', '--execute', 'show schemas'), ('mysql', '--skip-column-names', '--batch', '--execute', 'show schemas'),
extra_environment=extra_environment, extra_environment=extra_environment,
).and_return('foo\nbar\nmysql\n').once() ).and_return('foo\nbar\nmysql\n').once()
names = module.database_names_to_dump( names = module.database_names_to_dump(
{'name': 'all'}, extra_environment, log_prefix, dry_run_label {'name': 'all'}, extra_environment, log_prefix, dry_run=False
) )
assert names == ('foo', 'bar') assert names == ('foo', 'bar')
@ -323,7 +333,6 @@ def test_execute_dump_command_with_dry_run_skips_mysqldump():
def test_dump_databases_errors_for_missing_all_databases(): def test_dump_databases_errors_for_missing_all_databases():
databases = [{'name': 'all'}] databases = [{'name': 'all'}]
process = flexmock()
flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module).should_receive('make_dump_path').and_return('')
flexmock(module.dump).should_receive('make_database_dump_filename').and_return( flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
'databases/localhost/all' 'databases/localhost/all'
@ -331,7 +340,18 @@ def test_dump_databases_errors_for_missing_all_databases():
flexmock(module).should_receive('database_names_to_dump').and_return(()) flexmock(module).should_receive('database_names_to_dump').and_return(())
with pytest.raises(ValueError): with pytest.raises(ValueError):
assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == [process] assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False)
def test_dump_databases_does_not_error_for_missing_all_databases_with_dry_run():
databases = [{'name': 'all'}]
flexmock(module).should_receive('make_dump_path').and_return('')
flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
'databases/localhost/all'
)
flexmock(module).should_receive('database_names_to_dump').and_return(())
assert module.dump_databases(databases, 'test.yaml', {}, dry_run=True) == []
def test_restore_database_dump_runs_mysql_to_restore(): def test_restore_database_dump_runs_mysql_to_restore():

View file

@ -9,19 +9,32 @@ from borgmatic.hooks import postgresql as module
def test_database_names_to_dump_passes_through_individual_database_name(): def test_database_names_to_dump_passes_through_individual_database_name():
database = {'name': 'foo'} database = {'name': 'foo'}
assert module.database_names_to_dump(database, flexmock(), flexmock(), flexmock()) == ('foo',) assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=False) == (
'foo',
)
def test_database_names_to_dump_passes_through_individual_database_name_with_format(): def test_database_names_to_dump_passes_through_individual_database_name_with_format():
database = {'name': 'foo', 'format': 'custom'} database = {'name': 'foo', 'format': 'custom'}
assert module.database_names_to_dump(database, flexmock(), flexmock(), flexmock()) == ('foo',) assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=False) == (
'foo',
)
def test_database_names_to_dump_passes_through_all_without_format(): def test_database_names_to_dump_passes_through_all_without_format():
database = {'name': 'all'} database = {'name': 'all'}
assert module.database_names_to_dump(database, flexmock(), flexmock(), flexmock()) == ('all',) assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=False) == (
'all',
)
def test_database_names_to_dump_with_all_and_format_and_dry_run_bails():
database = {'name': 'all', 'format': 'custom'}
flexmock(module).should_receive('execute_command_and_capture_output').never()
assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=True) == ()
def test_database_names_to_dump_with_all_and_format_lists_databases(): def test_database_names_to_dump_with_all_and_format_lists_databases():
@ -30,7 +43,7 @@ def test_database_names_to_dump_with_all_and_format_lists_databases():
'foo,test,\nbar,test,"stuff and such"' 'foo,test,\nbar,test,"stuff and such"'
) )
assert module.database_names_to_dump(database, flexmock(), flexmock(), flexmock()) == ( assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=False) == (
'foo', 'foo',
'bar', 'bar',
) )
@ -53,7 +66,7 @@ def test_database_names_to_dump_with_all_and_format_lists_databases_with_hostnam
extra_environment=object, extra_environment=object,
).and_return('foo,test,\nbar,test,"stuff and such"') ).and_return('foo,test,\nbar,test,"stuff and such"')
assert module.database_names_to_dump(database, flexmock(), flexmock(), flexmock()) == ( assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=False) == (
'foo', 'foo',
'bar', 'bar',
) )
@ -66,7 +79,7 @@ def test_database_names_to_dump_with_all_and_format_lists_databases_with_usernam
extra_environment=object, extra_environment=object,
).and_return('foo,test,\nbar,test,"stuff and such"') ).and_return('foo,test,\nbar,test,"stuff and such"')
assert module.database_names_to_dump(database, flexmock(), flexmock(), flexmock()) == ( assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=False) == (
'foo', 'foo',
'bar', 'bar',
) )
@ -79,7 +92,7 @@ def test_database_names_to_dump_with_all_and_format_lists_databases_with_options
extra_environment=object, extra_environment=object,
).and_return('foo,test,\nbar,test,"stuff and such"') ).and_return('foo,test,\nbar,test,"stuff and such"')
assert module.database_names_to_dump(database, flexmock(), flexmock(), flexmock()) == ( assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=False) == (
'foo', 'foo',
'bar', 'bar',
) )
@ -91,7 +104,9 @@ def test_database_names_to_dump_with_all_and_format_excludes_particular_database
'foo,test,\ntemplate0,test,blah' 'foo,test,\ntemplate0,test,blah'
) )
assert module.database_names_to_dump(database, flexmock(), flexmock(), flexmock()) == ('foo',) assert module.database_names_to_dump(database, flexmock(), flexmock(), dry_run=False) == (
'foo',
)
def test_dump_databases_runs_pg_dump_for_each_database(): def test_dump_databases_runs_pg_dump_for_each_database():
@ -139,6 +154,15 @@ def test_dump_databases_raises_when_no_database_names_to_dump():
module.dump_databases(databases, 'test.yaml', {}, dry_run=False) module.dump_databases(databases, 'test.yaml', {}, dry_run=False)
def test_dump_databases_does_not_raise_when_no_database_names_to_dump():
databases = [{'name': 'foo'}, {'name': 'bar'}]
flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
flexmock(module).should_receive('make_dump_path').and_return('')
flexmock(module).should_receive('database_names_to_dump').and_return(())
module.dump_databases(databases, 'test.yaml', {}, dry_run=True) == []
def test_dump_databases_with_duplicate_dump_skips_pg_dump(): def test_dump_databases_with_duplicate_dump_skips_pg_dump():
databases = [{'name': 'foo'}, {'name': 'bar'}] databases = [{'name': 'foo'}, {'name': 'bar'}]
flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})