address some review comments

This commit is contained in:
Florian Apolloner
2025-09-17 08:46:02 +02:00
parent 0e90087dc4
commit 4de879d86e
5 changed files with 52 additions and 36 deletions
+2 -2
View File
@@ -188,7 +188,7 @@ def restore_single_dump(
data_source['name'],
data_source.get('hostname'),
data_source.get('port'),
data_source.get('label') or data_source.get('container') or UNSPECIFIED,
data_source.get('label') or UNSPECIFIED,
),
)
@@ -388,7 +388,7 @@ def collect_dumps_from_archive(
except (ValueError, TypeError):
port = None
dumps_from_archive.add(Dump(hook_name, data_source_name, hostname, port, host_and_port))
dumps_from_archive.add(Dump(hook_name, data_source_name, hostname, port))
# We've successfully parsed the dump path, so need to probe any further.
break
+12 -6
View File
@@ -6,7 +6,7 @@ import borgmatic.borg.pattern
import borgmatic.config.paths
import borgmatic.hooks.credential.parse
from borgmatic.execute import execute_command, execute_command_with_processes
from borgmatic.hooks.data_source import config as ds_config
from borgmatic.hooks.data_source import config as database_config
from borgmatic.hooks.data_source import dump
logger = logging.getLogger(__name__)
@@ -143,7 +143,7 @@ def build_dump_command(database, config, dump_filename, dump_format):
dump_command = tuple(
shlex.quote(part) for part in shlex.split(database.get('mongodump_command') or 'mongodump')
)
hostname = ds_config.resolve_database_option('hostname', database)
hostname = database_config.resolve_database_option('hostname', database)
return (
dump_command
+ (('--out', shlex.quote(dump_filename)) if dump_format == 'directory' else ())
@@ -277,16 +277,22 @@ def build_restore_command(extract_process, database, config, dump_filename, conn
'''
Return the custom mongorestore_command from a single database configuration.
'''
hostname = ds_config.resolve_database_option(
hostname = database_config.resolve_database_option(
'hostname', database, connection_params, restore=True
)
port = ds_config.resolve_database_option('port', database, connection_params, restore=True)
port = database_config.resolve_database_option(
'port', database, connection_params, restore=True
)
username = borgmatic.hooks.credential.parse.resolve_credential(
ds_config.resolve_database_option('username', database, connection_params, restore=True),
database_config.resolve_database_option(
'username', database, connection_params, restore=True
),
config,
)
password = borgmatic.hooks.credential.parse.resolve_credential(
ds_config.resolve_database_option('password', database, connection_params, restore=True),
database_config.resolve_database_option(
'password', database, connection_params, restore=True
),
config,
)
+14 -8
View File
@@ -12,7 +12,7 @@ from borgmatic.execute import (
execute_command_and_capture_output,
execute_command_with_processes,
)
from borgmatic.hooks.data_source import config as ds_config
from borgmatic.hooks.data_source import config as database_config
from borgmatic.hooks.data_source import dump
logger = logging.getLogger(__name__)
@@ -48,7 +48,7 @@ def database_names_to_dump(database, config, username, password, environment, dr
borgmatic.hooks.data_source.mariadb.parse_extra_options(database.get('list_options'))
)
password_transport = database.get('password_transport', 'pipe')
hostname = ds_config.resolve_database_option('hostname', database)
hostname = database_config.resolve_database_option('hostname', database)
show_command = (
mysql_show_command
+ (
@@ -123,7 +123,7 @@ def execute_dump_command(
borgmatic.hooks.data_source.mariadb.parse_extra_options(database.get('options'))
)
password_transport = database.get('password_transport', 'pipe')
hostname = ds_config.resolve_database_option('hostname', database)
hostname = database_config.resolve_database_option('hostname', database)
dump_command = (
mysql_dump_command
+ (
@@ -354,17 +354,23 @@ def restore_data_source_dump(
subprocess.Popen) to produce output to consume.
'''
dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
hostname = ds_config.resolve_database_option(
hostname = database_config.resolve_database_option(
'hostname', data_source, connection_params, restore=True
)
port = ds_config.resolve_database_option('port', data_source, connection_params, restore=True)
tls = ds_config.resolve_database_option('tls', data_source, restore=True)
port = database_config.resolve_database_option(
'port', data_source, connection_params, restore=True
)
tls = database_config.resolve_database_option('tls', data_source, restore=True)
username = borgmatic.hooks.credential.parse.resolve_credential(
ds_config.resolve_database_option('username', data_source, connection_params, restore=True),
database_config.resolve_database_option(
'username', data_source, connection_params, restore=True
),
config,
)
password = borgmatic.hooks.credential.parse.resolve_credential(
ds_config.resolve_database_option('password', data_source, connection_params, restore=True),
database_config.resolve_database_option(
'password', data_source, connection_params, restore=True
),
config,
)
+11 -7
View File
@@ -13,7 +13,7 @@ from borgmatic.execute import (
execute_command_and_capture_output,
execute_command_with_processes,
)
from borgmatic.hooks.data_source import config as ds_config
from borgmatic.hooks.data_source import config as database_config
from borgmatic.hooks.data_source import dump
logger = logging.getLogger(__name__)
@@ -33,7 +33,7 @@ def make_environment(database, config, restore_connection_params=None):
'''
environment = dict(os.environ)
password = ds_config.resolve_database_option(
password = database_config.resolve_database_option(
'password', database, restore_connection_params, restore=restore_connection_params
)
@@ -85,7 +85,7 @@ def database_names_to_dump(database, config, environment, dry_run):
psql_command = tuple(
shlex.quote(part) for part in shlex.split(database.get('psql_command') or 'psql')
)
hostname = ds_config.resolve_database_option('hostname', database)
hostname = database_config.resolve_database_option('hostname', database)
list_command = (
psql_command
+ ('--list', '--no-password', '--no-psqlrc', '--csv', '--tuples-only')
@@ -193,7 +193,7 @@ def dump_data_sources(
)
continue
hostname = ds_config.resolve_database_option('hostname', database)
hostname = database_config.resolve_database_option('hostname', database)
command = (
dump_command
+ (
@@ -339,12 +339,16 @@ def restore_data_source_dump(
hostname, port, username, and password.
'''
dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
hostname = ds_config.resolve_database_option(
hostname = database_config.resolve_database_option(
'hostname', data_source, connection_params, restore=True
)
port = ds_config.resolve_database_option('port', data_source, connection_params, restore=True)
port = database_config.resolve_database_option(
'port', data_source, connection_params, restore=True
)
username = borgmatic.hooks.credential.parse.resolve_credential(
ds_config.resolve_database_option('username', data_source, connection_params, restore=True),
database_config.resolve_database_option(
'username', data_source, connection_params, restore=True
),
config,
)
+13 -13
View File
@@ -564,9 +564,9 @@ def test_collect_dumps_from_archive_with_empty_dumps_metadata_path_falls_back_to
)
assert archive_dumps == {
module.Dump('postgresql_databases', 'foo', label='localhost'),
module.Dump('postgresql_databases', 'bar', 'host', 1234, label='host:1234'),
module.Dump('mysql_databases', 'quux', label='localhost'),
module.Dump('postgresql_databases', 'foo'),
module.Dump('postgresql_databases', 'bar', 'host', 1234),
module.Dump('mysql_databases', 'quux'),
}
@@ -606,9 +606,9 @@ def test_collect_dumps_from_archive_without_dumps_metadata_falls_back_to_parsing
)
assert archive_dumps == {
module.Dump('postgresql_databases', 'foo', label='localhost'),
module.Dump('postgresql_databases', 'bar', 'host', 1234, 'host:1234'),
module.Dump('mysql_databases', 'quux', label='localhost'),
module.Dump('postgresql_databases', 'foo'),
module.Dump('postgresql_databases', 'bar', 'host', 1234),
module.Dump('mysql_databases', 'quux'),
}
@@ -649,10 +649,10 @@ def test_collect_dumps_from_archive_parses_archive_paths_with_different_base_dir
)
assert archive_dumps == {
module.Dump('postgresql_databases', 'foo', label='localhost'),
module.Dump('postgresql_databases', 'bar', label='localhost'),
module.Dump('postgresql_databases', 'baz', label='localhost'),
module.Dump('mysql_databases', 'quux', label='localhost'),
module.Dump('postgresql_databases', 'foo'),
module.Dump('postgresql_databases', 'bar'),
module.Dump('postgresql_databases', 'baz'),
module.Dump('mysql_databases', 'quux'),
}
@@ -691,7 +691,7 @@ def test_collect_dumps_from_archive_parses_directory_format_archive_paths():
)
assert archive_dumps == {
module.Dump('postgresql_databases', 'foo', label='localhost'),
module.Dump('postgresql_databases', 'foo'),
}
@@ -733,8 +733,8 @@ def test_collect_dumps_from_archive_skips_bad_archive_paths_or_bad_path_componen
)
assert archive_dumps == {
module.Dump('postgresql_databases', 'foo', label='localhost'),
module.Dump('postgresql_databases', 'bar', label='localhost:abcd'),
module.Dump('postgresql_databases', 'foo'),
module.Dump('postgresql_databases', 'bar'),
}