From c5ef93c03411e7220986e394e977b2dc0775d1cf Mon Sep 17 00:00:00 2001 From: shivansh02 Date: Thu, 8 Aug 2024 17:08:41 +0530 Subject: [PATCH 1/5] mysql directory format --- borgmatic/hooks/mysql.py | 141 ++++++++++++++++++++++++++------------- 1 file changed, 95 insertions(+), 46 deletions(-) diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index 8ffc778a..5efd3254 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -86,6 +86,10 @@ def execute_dump_command( mysql_dump_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_dump_command') or 'mysqldump') ) + + dump_format = database.get('format') + is_directory_format = dump_format == 'directory' + dump_command = ( mysql_dump_command + (tuple(database['options'].split(' ')) if 'options' in database else ()) @@ -96,7 +100,7 @@ def execute_dump_command( + (('--user', database['username']) if 'username' in database else ()) + ('--databases',) + database_names - + ('--result-file', dump_filename) + + (('--tab', dump_filename) if is_directory_format else ('--result-file', dump_filename)) ) logger.debug( @@ -105,13 +109,20 @@ def execute_dump_command( if dry_run: return None - dump.create_named_pipe_for_dump(dump_filename) - - return execute_command( - dump_command, - extra_environment=extra_environment, - run_to_completion=False, - ) + if is_directory_format: + dump.create_parent_directory_for_dump(dump_filename) + execute_command( + dump_command, + extra_environment=extra_environment, + ) + return None + else: + dump.create_named_pipe_for_dump(dump_filename) + return execute_command( + dump_command, + extra_environment=extra_environment, + run_to_completion=False, + ) def use_streaming(databases, config, log_prefix): @@ -153,31 +164,31 @@ def dump_data_sources(databases, config, log_prefix, dry_run): for dump_name in dump_database_names: renamed_database = copy.copy(database) renamed_database['name'] = dump_name - processes.append( - execute_dump_command( - renamed_database, - log_prefix, - dump_path, - (dump_name,), - extra_environment, - dry_run, - dry_run_label, - ) - ) - else: - processes.append( - execute_dump_command( - database, + result = execute_dump_command( + renamed_database, log_prefix, dump_path, - dump_database_names, + (dump_name,), extra_environment, dry_run, dry_run_label, ) + if result: + processes.append(result) + else: + result = execute_dump_command( + database, + log_prefix, + dump_path, + dump_database_names, + extra_environment, + dry_run, + dry_run_label, ) + if result: + processes.append(result) - return [process for process in processes if process] + return processes def remove_data_source_dumps(databases, config, log_prefix, dry_run): # pragma: no cover @@ -225,31 +236,69 @@ def restore_data_source_dump( mysql_restore_command = tuple( shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') ) - restore_command = ( - mysql_restore_command - + ('--batch',) - + ( - tuple(data_source['restore_options'].split(' ')) - if 'restore_options' in data_source - else () + + dump_format = data_source.get('format') + is_directory_format = dump_format == 'directory' + + if is_directory_format: + mysql_restore_command = tuple( + shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') ) - + (('--host', hostname) if hostname else ()) - + (('--port', str(port)) if port else ()) - + (('--protocol', 'tcp') if hostname or port else ()) - + (('--user', username) if username else ()) - ) + restore_command = ( + mysql_restore_command + + ('--batch',) + + ( + tuple(data_source['restore_options'].split(' ')) + if 'restore_options' in data_source + else () + ) + + (('--host', hostname) if hostname else ()) + + (('--port', str(port)) if port else ()) + + (('--protocol', 'tcp') if hostname or port else ()) + + (('--user', username) if username else ()) + ) + else: + restore_command = ( + mysql_restore_command + + ('--batch',) + + ( + tuple(data_source['restore_options'].split(' ')) + if 'restore_options' in data_source + else () + ) + + (('--host', hostname) if hostname else ()) + + (('--port', str(port)) if port else ()) + + (('--protocol', 'tcp') if hostname or port else ()) + + (('--user', username) if username else ()) + ) + extra_environment = {'MYSQL_PWD': password} if password else None logger.debug(f"{log_prefix}: Restoring MySQL database {data_source['name']}{dry_run_label}") if dry_run: return - # Don't give Borg local path so as to error on warnings, as "borg extract" only gives a warning - # if the restore paths don't exist in the archive. - execute_command_with_processes( - restore_command, - [extract_process], - output_log_level=logging.DEBUG, - input_file=extract_process.stdout, - extra_environment=extra_environment, - ) + if is_directory_format: + dump_path = make_dump_path(config) + dump_directory = dump.make_data_source_dump_filename( + dump_path, data_source['name'], data_source.get('hostname') + ) + for file in os.listdir(dump_directory): + file_path = os.path.join(dump_directory, file) + if file.endswith('.sql'): + with open(file_path, 'r') as sql_file: + execute_command_with_processes( + restore_command, + [], + output_log_level=logging.DEBUG, + input_file=sql_file, + extra_environment=extra_environment, + ) + else: + execute_command_with_processes( + restore_command, + [extract_process], + output_log_level=logging.DEBUG, + input_file=extract_process.stdout, + extra_environment=extra_environment, + ) From e7b34ae47a03803a74e4657b9007a56c7d65cbb8 Mon Sep 17 00:00:00 2001 From: shivansh02 Date: Fri, 9 Aug 2024 20:39:22 +0530 Subject: [PATCH 2/5] mysql unit tests --- tests/unit/hooks/test_mysql.py | 42 ++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/unit/hooks/test_mysql.py b/tests/unit/hooks/test_mysql.py index fa6145ac..c2d3090a 100644 --- a/tests/unit/hooks/test_mysql.py +++ b/tests/unit/hooks/test_mysql.py @@ -54,6 +54,48 @@ def test_use_streaming_false_for_no_databases(): assert not module.use_streaming(databases=[], config=flexmock(), log_prefix=flexmock()) +def test_use_streaming_true_for_any_non_directory_format_databases(): + assert module.use_streaming( + databases=[{'format': 'stuff'}, {'format': 'directory'}, {}], + config=flexmock(), + log_prefix=flexmock(), + ) + + +def test_use_streaming_true_for_all_directory_format_databases(): + assert module.use_streaming( + databases=[{'format': 'directory'}, {'format': 'directory'}], + config=flexmock(), + log_prefix=flexmock(), + ) + + +def test_dump_data_sources_runs_mysql_dump_with_directory_format(): + databases = [{'name': 'foo', 'format': 'directory'}] + flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)) + flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return( + 'databases/localhost/foo' + ) + flexmock(module.os.path).should_receive('exists').and_return(False) + flexmock(module.dump).should_receive('create_parent_directory_for_dump') + flexmock(module.dump).should_receive('create_named_pipe_for_dump').never() + + flexmock(module).should_receive('execute_command').with_args( + ( + 'mysqldump', + '--add-drop-database', + '--databases', + 'foo', + '--tab', + 'databases/localhost/foo', + ), + extra_environment=None, + ).and_return(flexmock()).once() + + assert module.dump_data_sources(databases, {}, 'test.yaml', dry_run=False) == [] + + def test_dump_data_sources_dumps_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] processes = [flexmock(), flexmock()] From 8b2647e6bc09f253a263a3419b042edada3283c0 Mon Sep 17 00:00:00 2001 From: shivansh02 Date: Mon, 12 Aug 2024 12:42:02 +0530 Subject: [PATCH 3/5] mysql file variable clarified --- borgmatic/hooks/mysql.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index 5efd3254..2226a375 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -283,9 +283,9 @@ def restore_data_source_dump( dump_directory = dump.make_data_source_dump_filename( dump_path, data_source['name'], data_source.get('hostname') ) - for file in os.listdir(dump_directory): - file_path = os.path.join(dump_directory, file) - if file.endswith('.sql'): + for filename in os.listdir(dump_directory): + file_path = os.path.join(dump_directory, filename) + if filename.endswith('.sql'): with open(file_path, 'r') as sql_file: execute_command_with_processes( restore_command, From 78267ab019ca0788e5ee612e3328e095cbf4e031 Mon Sep 17 00:00:00 2001 From: shivansh02 Date: Wed, 21 Aug 2024 19:32:56 +0530 Subject: [PATCH 4/5] restore logic updated and variables inlined --- borgmatic/hooks/mysql.py | 69 ++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 42 deletions(-) diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index 2226a375..62b91d18 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -87,8 +87,7 @@ def execute_dump_command( shlex.quote(part) for part in shlex.split(database.get('mysql_dump_command') or 'mysqldump') ) - dump_format = database.get('format') - is_directory_format = dump_format == 'directory' + is_directory_format = database.get('format') == 'directory' dump_command = ( mysql_dump_command @@ -116,13 +115,12 @@ def execute_dump_command( extra_environment=extra_environment, ) return None - else: - dump.create_named_pipe_for_dump(dump_filename) - return execute_command( - dump_command, - extra_environment=extra_environment, - run_to_completion=False, - ) + dump.create_named_pipe_for_dump(dump_filename) + return execute_command( + dump_command, + extra_environment=extra_environment, + run_to_completion=False, + ) def use_streaming(databases, config, log_prefix): @@ -237,44 +235,25 @@ def restore_data_source_dump( shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') ) - dump_format = data_source.get('format') - is_directory_format = dump_format == 'directory' + is_directory_format = data_source.get('format') == 'directory' - if is_directory_format: - mysql_restore_command = tuple( - shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') - ) - restore_command = ( - mysql_restore_command - + ('--batch',) - + ( - tuple(data_source['restore_options'].split(' ')) - if 'restore_options' in data_source - else () - ) - + (('--host', hostname) if hostname else ()) - + (('--port', str(port)) if port else ()) - + (('--protocol', 'tcp') if hostname or port else ()) - + (('--user', username) if username else ()) - ) - else: - restore_command = ( - mysql_restore_command - + ('--batch',) - + ( - tuple(data_source['restore_options'].split(' ')) - if 'restore_options' in data_source - else () - ) - + (('--host', hostname) if hostname else ()) - + (('--port', str(port)) if port else ()) - + (('--protocol', 'tcp') if hostname or port else ()) - + (('--user', username) if username else ()) + restore_command = ( + mysql_restore_command + + ( + tuple(data_source['restore_options'].split(' ')) + if 'restore_options' in data_source + else () ) + + (('--host', hostname) if hostname else ()) + + (('--port', str(port)) if port else ()) + + (('--protocol', 'tcp') if hostname or port else ()) + + (('--user', username) if username else ()) + ) extra_environment = {'MYSQL_PWD': password} if password else None logger.debug(f"{log_prefix}: Restoring MySQL database {data_source['name']}{dry_run_label}") + if dry_run: return @@ -283,9 +262,15 @@ def restore_data_source_dump( dump_directory = dump.make_data_source_dump_filename( dump_path, data_source['name'], data_source.get('hostname') ) + + if not os.path.exists(dump_directory): + logger.warning(f"{log_prefix}: Dump directory {dump_directory} does not exist.") + return + for filename in os.listdir(dump_directory): - file_path = os.path.join(dump_directory, filename) if filename.endswith('.sql'): + file_path = os.path.join(dump_directory, filename) + logger.debug(f"{log_prefix}: Restoring from {file_path}") with open(file_path, 'r') as sql_file: execute_command_with_processes( restore_command, From 171e9eb2e690e4d316bb1033742fa3da8acaeb2a Mon Sep 17 00:00:00 2001 From: shivansh02 Date: Thu, 22 Aug 2024 03:50:38 +0530 Subject: [PATCH 5/5] Skipping duplicate dump error fixed --- borgmatic/hooks/mysql.py | 101 ++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 49 deletions(-) diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index 62b91d18..a275ba99 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -61,34 +61,43 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run): ) +def ensure_directory_exists(path): + if not os.path.exists(path): + os.makedirs(path) + + def execute_dump_command( database, log_prefix, dump_path, database_names, extra_environment, dry_run, dry_run_label ): ''' Kick off a dump for the given MySQL/MariaDB database (provided as a configuration dict) to a - named pipe constructed from the given dump path and database name. Use the given log prefix in - any log entries. + directory if --tab is used, or to a file if not. Use the given log prefix in any log entries. - Return a subprocess.Popen instance for the dump process ready to spew to a named pipe. But if - this is a dry run, then don't actually dump anything and return None. + Return a subprocess.Popen instance for the dump process ready to spew to a named pipe or directory. ''' database_name = database['name'] - dump_filename = dump.make_data_source_dump_filename( - dump_path, database['name'], database.get('hostname') - ) - if os.path.exists(dump_filename): - logger.warning( - f'{log_prefix}: Skipping duplicate dump of MySQL database "{database_name}" to {dump_filename}' + use_tab = '--tab' in (database.get('options') or '') + + if use_tab: + ensure_directory_exists(dump_path) + dump_dir = os.path.join(dump_path, database_name) + ensure_directory_exists(dump_dir) + result_file_option = ('--tab', dump_dir) + else: + dump_filename = dump.make_data_source_dump_filename( + dump_path, database['name'], database.get('hostname') ) - return None + if os.path.exists(dump_filename): + logger.warning( + f'{log_prefix}: Skipping duplicate dump of MySQL database "{database_name}" to {dump_filename}' + ) + return None + result_file_option = ('--result-file', dump_filename) mysql_dump_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_dump_command') or 'mysqldump') ) - - is_directory_format = database.get('format') == 'directory' - dump_command = ( mysql_dump_command + (tuple(database['options'].split(' ')) if 'options' in database else ()) @@ -99,23 +108,18 @@ def execute_dump_command( + (('--user', database['username']) if 'username' in database else ()) + ('--databases',) + database_names - + (('--tab', dump_filename) if is_directory_format else ('--result-file', dump_filename)) + + result_file_option ) logger.debug( - f'{log_prefix}: Dumping MySQL database "{database_name}" to {dump_filename}{dry_run_label}' + f'{log_prefix}: Dumping MySQL database "{database_name}" to {dump_path if use_tab else dump_filename}{dry_run_label}' ) if dry_run: return None - if is_directory_format: - dump.create_parent_directory_for_dump(dump_filename) - execute_command( - dump_command, - extra_environment=extra_environment, - ) - return None - dump.create_named_pipe_for_dump(dump_filename) + if not use_tab: + dump.create_named_pipe_for_dump(result_file_option[1]) + return execute_command( dump_command, extra_environment=extra_environment, @@ -125,20 +129,19 @@ def execute_dump_command( def use_streaming(databases, config, log_prefix): ''' - Given a sequence of MySQL database configuration dicts, a configuration dict (ignored), and a - log prefix (ignored), return whether streaming will be using during dumps. + Given a sequence of MySQL database configuration dicts, return whether streaming will be used during dumps. + Streaming is not used when using --tab. ''' - return any(databases) + return not any('--tab' in (db.get('options') or '') for db in databases) def dump_data_sources(databases, config, log_prefix, dry_run): ''' - Dump the given MySQL/MariaDB databases to a named pipe. The databases are supplied as a sequence - of dicts, one dict describing each database as per the configuration schema. Use the given - configuration dict to construct the destination path and the given log prefix in any log entries. + Dump the given MySQL/MariaDB databases to a named pipe or directory based on configuration. + The databases are supplied as a sequence of dicts, one dict describing each database as per the configuration schema. + Use the given configuration dict to construct the destination path and the given log prefix in any log entries. - Return a sequence of subprocess.Popen instances for the dump processes ready to spew to a named - pipe. But if this is a dry run, then don't actually dump anything and return an empty sequence. + Return a sequence of subprocess.Popen instances for the dump processes. ''' dry_run_label = ' (dry run; not actually dumping anything)' if dry_run else '' processes = [] @@ -162,31 +165,31 @@ def dump_data_sources(databases, config, log_prefix, dry_run): for dump_name in dump_database_names: renamed_database = copy.copy(database) renamed_database['name'] = dump_name - result = execute_dump_command( - renamed_database, + processes.append( + execute_dump_command( + renamed_database, + log_prefix, + dump_path, + (dump_name,), + extra_environment, + dry_run, + dry_run_label, + ) + ) + else: + processes.append( + execute_dump_command( + database, log_prefix, dump_path, - (dump_name,), + dump_database_names, extra_environment, dry_run, dry_run_label, ) - if result: - processes.append(result) - else: - result = execute_dump_command( - database, - log_prefix, - dump_path, - dump_database_names, - extra_environment, - dry_run, - dry_run_label, ) - if result: - processes.append(result) - return processes + return [process for process in processes if process] def remove_data_source_dumps(databases, config, log_prefix, dry_run): # pragma: no cover