diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index d81374039..666540cac 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -42,15 +42,16 @@ def dump_databases(databases, log_prefix, location_config, dry_run): '--no-password', '--clean', '--if-exists', - '--no-sync', ) - + ('--file', dump_filename) + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) + (('--username', database['username']) if 'username' in database else ()) + (() if all_databases else ('--format', database.get('format', 'custom'))) + (tuple(database['options'].split(' ')) if 'options' in database else ()) + (() if all_databases else (name,)) + # Use shell redirection rather than the --file flag to sidestep synchronization issues + # when pg_dump/pg_dumpall tries to write to a named pipe. + + ('>', dump_filename) ) extra_environment = {'PGPASSWORD': database['password']} if 'password' in database else None @@ -65,7 +66,9 @@ def dump_databases(databases, log_prefix, location_config, dry_run): dump.create_named_pipe_for_dump(dump_filename) processes.append( - execute_command(command, extra_environment=extra_environment, run_to_completion=False) + execute_command( + command, shell=True, extra_environment=extra_environment, run_to_completion=False + ) ) return processes diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 1f9533289..68b551a3c 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -20,13 +20,13 @@ def test_dump_databases_runs_pg_dump_for_each_database(): '--no-password', '--clean', '--if-exists', - '--no-sync', - '--file', - 'databases/localhost/{}'.format(name), '--format', 'custom', name, + '>', + 'databases/localhost/{}'.format(name), ), + shell=True, extra_environment=None, run_to_completion=False, ).and_return(process).once() @@ -61,9 +61,6 @@ def test_dump_databases_runs_pg_dump_with_hostname_and_port(): '--no-password', '--clean', '--if-exists', - '--no-sync', - '--file', - 'databases/database.example.org/foo', '--host', 'database.example.org', '--port', @@ -71,7 +68,10 @@ def test_dump_databases_runs_pg_dump_with_hostname_and_port(): '--format', 'custom', 'foo', + '>', + 'databases/database.example.org/foo', ), + shell=True, extra_environment=None, run_to_completion=False, ).and_return(process).once() @@ -94,15 +94,15 @@ def test_dump_databases_runs_pg_dump_with_username_and_password(): '--no-password', '--clean', '--if-exists', - '--no-sync', - '--file', - 'databases/localhost/foo', '--username', 'postgres', '--format', 'custom', 'foo', + '>', + 'databases/localhost/foo', ), + shell=True, extra_environment={'PGPASSWORD': 'trustsome1'}, run_to_completion=False, ).and_return(process).once() @@ -125,13 +125,13 @@ def test_dump_databases_runs_pg_dump_with_format(): '--no-password', '--clean', '--if-exists', - '--no-sync', - '--file', - 'databases/localhost/foo', '--format', 'tar', 'foo', + '>', + 'databases/localhost/foo', ), + shell=True, extra_environment=None, run_to_completion=False, ).and_return(process).once() @@ -154,14 +154,14 @@ def test_dump_databases_runs_pg_dump_with_options(): '--no-password', '--clean', '--if-exists', - '--no-sync', - '--file', - 'databases/localhost/foo', '--format', 'custom', '--stuff=such', 'foo', + '>', + 'databases/localhost/foo', ), + shell=True, extra_environment=None, run_to_completion=False, ).and_return(process).once() @@ -179,15 +179,8 @@ def test_dump_databases_runs_pg_dumpall_for_all_databases(): flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( - ( - 'pg_dumpall', - '--no-password', - '--clean', - '--if-exists', - '--no-sync', - '--file', - 'databases/localhost/all', - ), + ('pg_dumpall', '--no-password', '--clean', '--if-exists', '>', 'databases/localhost/all'), + shell=True, extra_environment=None, run_to_completion=False, ).and_return(process).once()