Browse Source

address some review comments

Florian Apolloner 1 month ago
parent
commit
16c8098b06

+ 1 - 1
borgmatic/commands/arguments.py

@@ -1493,7 +1493,7 @@ def make_parsers(schema, unparsed_arguments):  # noqa: PLR0915
     )
     )
     restore_group.add_argument(
     restore_group.add_argument(
         '--original-label',
         '--original-label',
-        help='The label where to dump to restore came from, only necessary if you need to disambiguate dumps',
+        help='The label where the dump to restore came from, only necessary if you need to disambiguate dumps',
     )
     )
     restore_group.add_argument(
     restore_group.add_argument(
         '--original-hostname',
         '--original-hostname',

+ 5 - 7
borgmatic/hooks/data_source/config.py

@@ -13,7 +13,7 @@ logger = logging.getLogger(__name__)
 def resolve_database_option(option, data_source, connection_params=None, restore=False):
 def resolve_database_option(option, data_source, connection_params=None, restore=False):
     # Special case `hostname` since it overlaps with `container`
     # Special case `hostname` since it overlaps with `container`
     if option == 'hostname':
     if option == 'hostname':
-        return _get_hostname_from_config(data_source, connection_params, restore)
+        return get_hostname_from_config(data_source, connection_params, restore)
     if connection_params and (value := connection_params.get(option)):
     if connection_params and (value := connection_params.get(option)):
         return value
         return value
     if restore and f'restore_{option}' in data_source:
     if restore and f'restore_{option}' in data_source:
@@ -21,7 +21,7 @@ def resolve_database_option(option, data_source, connection_params=None, restore
     return data_source.get(option)
     return data_source.get(option)
 
 
 
 
-def _get_hostname_from_config(data_source, connection_params=None, restore=False):
+def get_hostname_from_config(data_source, connection_params=None, restore=False):
     # connection params win, full stop
     # connection params win, full stop
     if connection_params:
     if connection_params:
         if container := connection_params.get('container'):
         if container := connection_params.get('container'):
@@ -37,7 +37,7 @@ def _get_hostname_from_config(data_source, connection_params=None, restore=False
     # ... and finally fall back to the normal options
     # ... and finally fall back to the normal options
     if 'container' in data_source:
     if 'container' in data_source:
         return get_ip_from_container(data_source['container'])
         return get_ip_from_container(data_source['container'])
-    return data_source.get('hostname', '')
+    return data_source.get('hostname')
 
 
 
 
 def get_ip_from_container(container):
 def get_ip_from_container(container):
@@ -68,13 +68,11 @@ def get_ip_from_container(container):
             network_data = json.loads(output.strip())
             network_data = json.loads(output.strip())
         except json.JSONDecodeError as e:
         except json.JSONDecodeError as e:
             raise ValueError(f'Could not decode JSON output from {engine}') from e
             raise ValueError(f'Could not decode JSON output from {engine}') from e
-        main_ip = network_data.get('IPAddress')
-        if main_ip:
+        if main_ip := network_data.get('IPAddress'):
             return main_ip
             return main_ip
         # No main IP found, try the networks
         # No main IP found, try the networks
         for network in network_data.get('Networks', {}).values():
         for network in network_data.get('Networks', {}).values():
-            ip = network.get('IPAddress')
-            if ip:
+            if ip := network.get('IPAddress'):
                 return ip
                 return ip
 
 
     if last_error:
     if last_error:

+ 14 - 8
borgmatic/hooks/data_source/mariadb.py

@@ -12,7 +12,7 @@ from borgmatic.execute import (
     execute_command_and_capture_output,
     execute_command_and_capture_output,
     execute_command_with_processes,
     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
 from borgmatic.hooks.data_source import dump
 
 
 logger = logging.getLogger(__name__)
 logger = logging.getLogger(__name__)
@@ -123,7 +123,7 @@ def database_names_to_dump(database, config, username, password, environment, dr
     )
     )
     extra_options, defaults_extra_filename = parse_extra_options(database.get('list_options'))
     extra_options, defaults_extra_filename = parse_extra_options(database.get('list_options'))
     password_transport = database.get('password_transport', 'pipe')
     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 = (
     show_command = (
         mariadb_show_command
         mariadb_show_command
         + (
         + (
@@ -195,7 +195,7 @@ def execute_dump_command(
     )
     )
     extra_options, defaults_extra_filename = parse_extra_options(database.get('options'))
     extra_options, defaults_extra_filename = parse_extra_options(database.get('options'))
     password_transport = database.get('password_transport', 'pipe')
     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 = (
     dump_command = (
         mariadb_dump_command
         mariadb_dump_command
         + (
         + (
@@ -418,17 +418,23 @@ def restore_data_source_dump(
     subprocess.Popen) to produce output to consume.
     subprocess.Popen) to produce output to consume.
     '''
     '''
     dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
     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
         '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(
     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,
         config,
     )
     )
     password = borgmatic.hooks.credential.parse.resolve_credential(
     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,
         config,
     )
     )
 
 

+ 2 - 2
borgmatic/hooks/data_source/postgresql.py

@@ -349,8 +349,8 @@ def restore_data_source_dump(
     dump_filename = dump.make_data_source_dump_filename(
     dump_filename = dump.make_data_source_dump_filename(
         make_dump_path(borgmatic_runtime_directory),
         make_dump_path(borgmatic_runtime_directory),
         data_source['name'],
         data_source['name'],
-        hostname=data_source.get('hostname'),
-        port=data_source.get('port'),
+        hostname=hostname,
+        port=port,
         label=data_source.get('label', data_source.get('container')),
         label=data_source.get('label', data_source.get('container')),
     )
     )
     psql_command = tuple(
     psql_command = tuple(