From 3b0f6de883a9125690473d586260e47a8951b149 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 25 Oct 2023 13:40:54 +0530 Subject: [PATCH 01/18] perf: don't extract backup files unless required Read from the gzipped file wherever possible Signed-off-by: Akhil Narang --- frappe/commands/site.py | 65 +++++++++------------ frappe/installer.py | 126 ++++++++++++++++++++++++++++------------ frappe/utils/backups.py | 7 ++- 3 files changed, 121 insertions(+), 77 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 9b8d2d9131..a6deb0a16c 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -194,8 +194,7 @@ def _restore( _backup = Backup(sql_file_path) try: - decompressed_file_name = extract_sql_from_archive(sql_file_path) - if is_partial(decompressed_file_name): + if is_partial(sql_file_path): click.secho( "Partial Backup file detected. You cannot use a partial file to restore a Frappe site.", fg="red", @@ -218,16 +217,14 @@ def _restore( encryption_key = get_or_generate_backup_encryption_key() _backup.backup_decryption(encryption_key) - # Rollback on unsuccessful decryrption + # Rollback on unsuccessful decryption if not os.path.exists(sql_file_path): click.secho("Decryption failed. Please provide a valid key and try again.", fg="red") _backup.decryption_rollback() sys.exit(1) - decompressed_file_name = extract_sql_from_archive(sql_file_path) - - if is_partial(decompressed_file_name): + if is_partial(sql_file_path): click.secho( "Partial Backup file detected. You cannot use a partial file to restore a Frappe site.", fg="red", @@ -239,16 +236,20 @@ def _restore( _backup.decryption_rollback() sys.exit(1) - validate_database_sql(decompressed_file_name, _raise=not force) - - # dont allow downgrading to older versions of frappe without force - if not force and is_downgrade(decompressed_file_name, verbose=True): + # don't allow downgrading to older versions of frappe without force + if not force and is_downgrade(sql_file_path, verbose=True): warn_message = ( "This is not recommended and may lead to unexpected behaviour. " "Do you want to continue anyway?" ) click.confirm(warn_message, abort=True) + # Extract file if its gzipped + decompressed_file_name = extract_sql_from_archive(sql_file_path) + + # Validate the sql file + validate_database_sql(decompressed_file_name, _raise=not force) + try: _new_site( frappe.conf.db_name, @@ -312,7 +313,7 @@ def _restore( @click.option("--encryption-key", help="Backup encryption key") @pass_context def partial_restore(context, sql_file_path, verbose, encryption_key=None): - from frappe.installer import extract_sql_from_archive, partial_restore + from frappe.installer import is_partial, partial_restore from frappe.utils.backups import Backup, get_or_generate_backup_encryption_key if not os.path.exists(sql_file_path): @@ -328,19 +329,13 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): frappe.connect(site=site) try: - decompressed_file_name = extract_sql_from_archive(sql_file_path) - - with open(decompressed_file_name) as f: - header = " ".join(f.readline() for _ in range(5)) - - # Check for full backup file - if "Partial Backup" not in header: - click.secho( - "Full backup file detected.Use `bench restore` to restore a Frappe Site.", - fg="red", - ) - _backup.decryption_rollback() - sys.exit(1) + if not is_partial(sql_file_path): + click.secho( + "Full backup file detected.Use `bench restore` to restore a Frappe Site.", + fg="red", + ) + _backup.decryption_rollback() + sys.exit(1) except UnicodeDecodeError: _backup.decryption_rollback() @@ -354,25 +349,19 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): _backup.backup_decryption(key) - # Rollback on unsuccessful decryrption + # Rollback on unsuccessful decryption if not os.path.exists(sql_file_path): click.secho("Decryption failed. Please provide a valid key and try again.", fg="red") _backup.decryption_rollback() sys.exit(1) - decompressed_file_name = extract_sql_from_archive(sql_file_path) - - with open(decompressed_file_name) as f: - header = " ".join(f.readline() for _ in range(5)) - - # Check for Full backup file. - if "Partial Backup" not in header: - click.secho( - "Full Backup file detected.Use `bench restore` to restore a Frappe Site.", - fg="red", - ) - _backup.decryption_rollback() - sys.exit(1) + if not is_partial(sql_file_path): + click.secho( + "Full Backup file detected.Use `bench restore` to restore a Frappe Site.", + fg="red", + ) + _backup.decryption_rollback() + sys.exit(1) partial_restore(sql_file_path, verbose) diff --git a/frappe/installer.py b/frappe/installer.py index 891de5f2e8..742a0c454a 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -1,6 +1,7 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE - +import configparser +import gzip import json import os import re @@ -52,7 +53,7 @@ def _new_site( ): """Install a new Frappe site""" - from frappe.utils import get_site_path, scheduler, touch_file + from frappe.utils import get_site_path, scheduler if not force and os.path.exists(site): print(f"Site {site} already exists") @@ -793,48 +794,81 @@ def is_downgrade(sql_file_path, verbose=False): from semantic_version import Version - head = "INSERT INTO `tabInstalled Application` VALUES" + backup_version = None + try: + backup_version = extract_version_from_dump(sql_file_path) + except Exception: + # Handle older backups in the same way + head = "INSERT INTO `tabInstalled Application` VALUES" - with open(sql_file_path) as f: - for line in f: - if head in line: - # 'line' (str) format: ('2056588823','2020-05-11 18:21:31.488367','2020-06-12 11:49:31.079506','Administrator','Administrator',0,'Installed Applications','installed_applications','Installed Applications',1,'frappe','v10.1.71-74 (3c50d5e) (v10.x.x)','v10.x.x'),('855c640b8e','2020-05-11 18:21:31.488367','2020-06-12 11:49:31.079506','Administrator','Administrator',0,'Installed Applications','installed_applications','Installed Applications',2,'your_custom_app','0.0.1','master') - line = line.strip().lstrip(head).rstrip(";").strip() - app_rows = frappe.safe_eval(line) - # check if iterable consists of tuples before trying to transform - apps_list = ( - app_rows - if all(isinstance(app_row, (tuple, list, set)) for app_row in app_rows) - else (app_rows,) - ) - # 'all_apps' (list) format: [('frappe', '12.x.x-develop ()', 'develop'), ('your_custom_app', '0.0.1', 'master')] - all_apps = [x[-3:] for x in apps_list] + with open(sql_file_path) as f: + for line in f: + if head in line: + # 'line' (str) format: ('2056588823','2020-05-11 18:21:31.488367','2020-06-12 11:49:31.079506','Administrator','Administrator',0,'Installed Applications','installed_applications','Installed Applications',1,'frappe','v10.1.71-74 (3c50d5e) (v10.x.x)','v10.x.x'),('855c640b8e','2020-05-11 18:21:31.488367','2020-06-12 11:49:31.079506','Administrator','Administrator',0,'Installed Applications','installed_applications','Installed Applications',2,'your_custom_app','0.0.1','master') + line = line.strip().lstrip(head).rstrip(";").strip() + app_rows = frappe.safe_eval(line) + # check if iterable consists of tuples before trying to transform + apps_list = ( + app_rows + if all(isinstance(app_row, (tuple, list, set)) for app_row in app_rows) + else (app_rows,) + ) + # 'all_apps' (list) format: [('frappe', '12.x.x-develop ()', 'develop'), ('your_custom_app', '0.0.1', 'master')] + all_apps = [x[-3:] for x in apps_list] - for app in all_apps: - app_name = app[0] - app_version = app[1].split(" ", 1)[0] + for app in all_apps: + app_name = app[0] + app_version = app[1].split(" ", 1)[0] - if app_name == "frappe": - try: - current_version = Version(frappe.__version__) - backup_version = Version(app_version[1:] if app_version[0] == "v" else app_version) - except ValueError: - return False + if app_name == "frappe": + try: + backup_version = app_version[1:] if app_version[0] == "v" else app_version + break + except ValueError: + return False - downgrade = backup_version > current_version + # Assume it's not a downgrade if we can't determine backup version + if backup_version is None: + return False - if verbose and downgrade: - print(f"Your site will be downgraded from Frappe {backup_version} to {current_version}") + current_version = Version(frappe.__version__) + downgrade = Version(backup_version) > current_version - return downgrade + if verbose and downgrade: + print(f"Your site will be downgraded from Frappe {backup_version} to {current_version}") + + return downgrade -def is_partial(sql_file_path): - with open(sql_file_path) as f: - header = " ".join(f.readline() for _ in range(5)) - if "Partial Backup" in header: - return True - return False +def extract_version_from_dump(sql_file_path: str) -> str | None: + """ + Extract frappe version from DB dump + + :param sql_file_path: The path to the dump file + :return: The frappe version used to create the backup + """ + header = get_db_dump_header(sql_file_path).split("\n") + metadata = "" + if "begin frappe metadata" in header[0]: + for line in header[1:]: + if "end frappe metadata" in line: + break + metadata += line.replace("--", "").strip() + "\n" + parser = configparser.ConfigParser() + parser.read_string(metadata) + return parser["frappe"]["version"] + return None + + +def is_partial(sql_file_path: str) -> bool: + """ + Function to return whether the database dump is a partial backup or not + + :param sql_file_path: path to the database dump file + :return: True if the database dump is a partial backup, False otherwise + """ + header = get_db_dump_header(sql_file_path) + return "Partial Backup" in header def partial_restore(sql_file_path, verbose=False): @@ -877,7 +911,7 @@ def validate_database_sql(path, _raise=True): error_message = f"{path} is an empty file!" empty_file = True - # dont bother checking if empty file + # don't bother checking if empty file if not empty_file: with open(path) as f: for line in f: @@ -893,3 +927,21 @@ def validate_database_sql(path, _raise=True): if _raise and (missing_table or empty_file): raise frappe.InvalidDatabaseFile + + +def get_db_dump_header(file_path: str, file_bytes: int = 256) -> str: + """ + Get the header of a database dump file + + :param file_path: path to the database dump file + :param file_bytes: number of bytes to read from the file + :return: The first few bytes of the file as requested + """ + + # Use `gzip` to open the file if the extension is `.gz` + if file_path.endswith(".gz"): + with gzip.open(file_path, "rb") as f: + return f.read(file_bytes).decode() + + with open(file_path, "rb") as f: + return f.read(file_bytes).decode() diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index e716ff6e7a..48aa060377 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -373,7 +373,11 @@ class BackupGenerator: ) database_header_content = [ - f"Backup generated by Frappe {frappe.__version__} on branch {get_app_branch('frappe') or 'N/A'}", + "begin frappe metadata", + "[frappe]", + f"version = {frappe.__version__}", + f"branch = {get_app_branch('frappe') or 'N/A'}", + "end frappe metadata", "", ] @@ -673,7 +677,6 @@ def backup( backup_path_files=None, backup_path_private_files=None, backup_path_conf=None, - quiet=False, ): "Backup" odb = scheduled_backup( From cb7c0e653cbdf3e48cfd3686e19076d38b276519 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 26 Oct 2023 15:35:16 +0530 Subject: [PATCH 02/18] fix(Backup): automatically rollback decryption when object is being deleted This allows us to not have to call it everytime before returning Signed-off-by: Akhil Narang --- frappe/utils/backups.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 48aa060377..ba546801e5 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -670,6 +670,9 @@ class Backup: os.remove(self.file_path.rstrip(".gz")) os.rename(self.file_path + ".gpg", self.file_path) + def __del__(self): + self.decryption_rollback() + def backup( with_files=False, From 7db8baa7c40dc5d348c65809d3affcc6f14d110b Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 26 Oct 2023 15:36:32 +0530 Subject: [PATCH 03/18] feat(installer): drop actual gzip file extraction Use `zgrep` to check for table name match where required Also use a table that's at the top of the dump files (`__Auth`) Signed-off-by: Akhil Narang --- frappe/installer.py | 89 ++++++++++++--------------------------------- 1 file changed, 23 insertions(+), 66 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 742a0c454a..a81955833a 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -665,32 +665,6 @@ def remove_missing_apps(): frappe.db.set_global("installed_apps", json.dumps(installed_apps)) -def extract_sql_from_archive(sql_file_path): - """Return the path of an SQL file if the passed argument is the path of a gzipped - SQL file or an SQL file path. The path may be absolute or relative from the bench - root directory or the sites sub-directory. - - Args: - sql_file_path (str): Path of the SQL file - - Return: - str: Path of the decompressed SQL file - """ - from frappe.utils import get_bench_relative_path - - sql_file_path = get_bench_relative_path(sql_file_path) - # Extract the gzip file if user has passed *.sql.gz file instead of *.sql file - if sql_file_path.endswith("sql.gz"): - decompressed_file_name = extract_sql_gzip(sql_file_path) - else: - decompressed_file_name = sql_file_path - - # convert archive sql to latest compatible - convert_archive_content(decompressed_file_name) - - return decompressed_file_name - - def convert_archive_content(sql_file_path): if frappe.conf.db_type == "mariadb": # ever since mariaDB 10.6, row_format COMPRESSED has been deprecated and removed @@ -724,20 +698,6 @@ def convert_archive_content(sql_file_path): old_sql_file_path.unlink() -def extract_sql_gzip(sql_gz_path): - import subprocess - - try: - original_file = sql_gz_path - decompressed_file = original_file.rstrip(".gz") - cmd = f"gzip --decompress --force < {original_file} > {decompressed_file}" - subprocess.check_call(cmd, shell=True) - except Exception: - raise - - return decompressed_file - - def _guess_mariadb_version() -> tuple[int] | None: # Using command-line because we *might* not have a connection yet and this command is required # in non-interactive mode. @@ -872,8 +832,6 @@ def is_partial(sql_file_path: str) -> bool: def partial_restore(sql_file_path, verbose=False): - sql_file = extract_sql_from_archive(sql_file_path) - if frappe.conf.db_type == "mariadb": from frappe.database.mariadb.setup_db import import_db_from_sql elif frappe.conf.db_type == "postgres": @@ -887,45 +845,44 @@ def partial_restore(sql_file_path, verbose=False): fg="yellow", ) warnings.warn(warn) + else: + click.secho("Unsupported database type", fg="red") + return - import_db_from_sql(source_sql=sql_file, verbose=verbose) - - # Removing temporarily created file - if sql_file != sql_file_path: - os.remove(sql_file) + import_db_from_sql(source_sql=sql_file_path, verbose=verbose) -def validate_database_sql(path, _raise=True): - """Check if file has contents and if DefaultValue table exists +def validate_database_sql(path: str, _raise: bool = True) -> None: + """Check if file has contents and if `__Auth` table exists Args: path (str): Path of the decompressed SQL file _raise (bool, optional): Raise exception if invalid file. Defaults to True. """ - empty_file = False - missing_table = True - error_message = "" + if path.endswith(".gz"): + executable_name = "zgrep" + else: + executable_name = "grep" - if not os.path.getsize(path): + if os.path.getsize(path): + if (executable := which(executable_name)) is None: + frappe.throw( + f"`{executable_name}` not found in PATH! This is required to take a backup.", + exc=frappe.ExecutableNotFound, + ) + try: + frappe.utils.execute_in_shell(f"{executable} -m1 __Auth {path}", check_exit_code=True) + return + except Exception: + error_message = "Table `__Auth` not found in file." + else: error_message = f"{path} is an empty file!" - empty_file = True - - # don't bother checking if empty file - if not empty_file: - with open(path) as f: - for line in f: - if "tabDefaultValue" in line: - missing_table = False - break - - if missing_table: - error_message = "Table `tabDefaultValue` not found in file." if error_message: click.secho(error_message, fg="red") - if _raise and (missing_table or empty_file): + if _raise: raise frappe.InvalidDatabaseFile From 0b508e2a961807ea8784255a64b71cb6afe6ae22 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 26 Oct 2023 15:37:49 +0530 Subject: [PATCH 04/18] feat(db_manager): avoid extraction of DB dump if gzipped Use `gzip -cd` to directly get the contents onto stdout and pipe to mariadb Signed-off-by: Akhil Narang --- frappe/database/db_manager.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index 68cd39f2f5..27704fb472 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -61,12 +61,23 @@ class DbManager: command = [] - if pv: - command.extend([pv, source, "|"]) - source = [] - print("Restoring Database file...") + if source.endswith(".gz"): + if gzip := which("gzip"): + source = [] + command.extend([gzip, "-cd", source, "|"]) + if pv: + command.extend([pv, "|"]) + print("Restoring Database file...") + else: + raise Exception("`gzip` not installed") + else: - source = ["<", source] + if pv: + command.extend([pv, source, "|"]) + source = [] + print("Restoring Database file...") + else: + source = ["<", source] bin, args, bin_name = get_command( host=frappe.conf.db_host, From 2b7c74dd5e8ba7d3c7e6ac37a891a1258693d138 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 26 Oct 2023 15:39:33 +0530 Subject: [PATCH 05/18] feat(restore): handle encrypted backups better Determine the mimetype based on the file contents instead of waiting for an exception Cleaner + no need of duplicate code Signed-off-by: Akhil Narang --- frappe/commands/site.py | 102 ++++++++++------------------------------ 1 file changed, 25 insertions(+), 77 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index a6deb0a16c..9a7b659390 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -5,6 +5,7 @@ import sys # imports - third party imports import click +import magic # imports - module imports import frappe @@ -72,13 +73,10 @@ def new_site( setup_db=True, ): "Create a new site" - from frappe.installer import _new_site, extract_sql_from_archive + from frappe.installer import _new_site frappe.init(site=site, new_site=True) - if source_sql: - source_sql = extract_sql_from_archive(source_sql) - _new_site( db_name, site, @@ -180,11 +178,9 @@ def _restore( with_public_files=None, with_private_files=None, ): - from frappe.installer import ( _new_site, extract_files, - extract_sql_from_archive, is_downgrade, is_partial, validate_database_sql, @@ -192,22 +188,8 @@ def _restore( from frappe.utils.backups import Backup, get_or_generate_backup_encryption_key _backup = Backup(sql_file_path) - - try: - if is_partial(sql_file_path): - click.secho( - "Partial Backup file detected. You cannot use a partial file to restore a Frappe site.", - fg="red", - ) - click.secho( - "Use `bench partial-restore` to restore a partial backup to an existing site.", - fg="yellow", - ) - _backup.decryption_rollback() - sys.exit(1) - - except UnicodeDecodeError: - _backup.decryption_rollback() + backup_mimetype = magic.from_file(sql_file_path) + if "cipher" in backup_mimetype: if encryption_key: click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") _backup.backup_decryption(encryption_key) @@ -220,21 +202,18 @@ def _restore( # Rollback on unsuccessful decryption if not os.path.exists(sql_file_path): click.secho("Decryption failed. Please provide a valid key and try again.", fg="red") - - _backup.decryption_rollback() sys.exit(1) - if is_partial(sql_file_path): - click.secho( - "Partial Backup file detected. You cannot use a partial file to restore a Frappe site.", - fg="red", - ) - click.secho( - "Use `bench partial-restore` to restore a partial backup to an existing site.", - fg="yellow", - ) - _backup.decryption_rollback() - sys.exit(1) + if is_partial(sql_file_path): + click.secho( + "Partial Backup file detected. You cannot use a partial file to restore a Frappe site.", + fg="red", + ) + click.secho( + "Use `bench partial-restore` to restore a partial backup to an existing site.", + fg="yellow", + ) + sys.exit(1) # don't allow downgrading to older versions of frappe without force if not force and is_downgrade(sql_file_path, verbose=True): @@ -244,11 +223,8 @@ def _restore( ) click.confirm(warn_message, abort=True) - # Extract file if its gzipped - decompressed_file_name = extract_sql_from_archive(sql_file_path) - # Validate the sql file - validate_database_sql(decompressed_file_name, _raise=not force) + validate_database_sql(sql_file_path, _raise=not force) try: _new_site( @@ -259,47 +235,35 @@ def _restore( admin_password=admin_password, verbose=verbose, install_apps=install_app, - source_sql=decompressed_file_name, + source_sql=sql_file_path, force=True, db_type=frappe.conf.db_type, ) except Exception as err: print(err.args[1]) - _backup.decryption_rollback() sys.exit(1) - # Removing temporarily created file - if decompressed_file_name != sql_file_path: - os.remove(decompressed_file_name) - _backup.decryption_rollback() - # Extract public and/or private files to the restored site, if user has given the path if with_public_files: # Decrypt data if there is a Key if encryption_key: _backup = Backup(with_public_files) _backup.backup_decryption(encryption_key) - if not os.path.exists(with_public_files): - _backup.decryption_rollback() public = extract_files(site, with_public_files) # Removing temporarily created file os.remove(public) - _backup.decryption_rollback() if with_private_files: # Decrypt data if there is a Key if encryption_key: _backup = Backup(with_private_files) _backup.backup_decryption(encryption_key) - if not os.path.exists(with_private_files): - _backup.decryption_rollback() private = extract_files(site, with_private_files) # Removing temporarily created file os.remove(private) - _backup.decryption_rollback() success_message = "Site {} has been restored{}".format( site, " with files" if (with_public_files or with_private_files) else "" @@ -328,17 +292,9 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): verbose = context.verbose or verbose frappe.connect(site=site) - try: - if not is_partial(sql_file_path): - click.secho( - "Full backup file detected.Use `bench restore` to restore a Frappe Site.", - fg="red", - ) - _backup.decryption_rollback() - sys.exit(1) - - except UnicodeDecodeError: - _backup.decryption_rollback() + _backup = Backup(sql_file_path) + backup_mimetype = magic.from_file(sql_file_path) + if "cipher" in backup_mimetype: if encryption_key: click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") key = encryption_key @@ -352,24 +308,16 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): # Rollback on unsuccessful decryption if not os.path.exists(sql_file_path): click.secho("Decryption failed. Please provide a valid key and try again.", fg="red") - _backup.decryption_rollback() sys.exit(1) - if not is_partial(sql_file_path): - click.secho( - "Full Backup file detected.Use `bench restore` to restore a Frappe Site.", - fg="red", - ) - _backup.decryption_rollback() - sys.exit(1) + if not is_partial(sql_file_path): + click.secho( + "Full backup file detected.Use `bench restore` to restore a Frappe Site.", + fg="red", + ) + sys.exit(1) partial_restore(sql_file_path, verbose) - - # Removing temporarily created file - _backup.decryption_rollback() - if os.path.exists(sql_file_path.rstrip(".gz")): - os.remove(sql_file_path.rstrip(".gz")) - frappe.destroy() From 50d21677b872ff89d2912c7b57467757a35f549a Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 26 Oct 2023 17:51:34 +0530 Subject: [PATCH 06/18] chore: drop use of `magic` library, run `file` command directly Signed-off-by: Akhil Narang --- frappe/commands/site.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 9a7b659390..d35e24a223 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -5,7 +5,6 @@ import sys # imports - third party imports import click -import magic # imports - module imports import frappe @@ -188,8 +187,12 @@ def _restore( from frappe.utils.backups import Backup, get_or_generate_backup_encryption_key _backup = Backup(sql_file_path) - backup_mimetype = magic.from_file(sql_file_path) - if "cipher" in backup_mimetype: + err, out = frappe.utils.execute_in_shell(f"file {sql_file_path}", check_exit_code=True) + if err: + click.secho("Failed to detect type of backup file", fg="red") + sys.exit(1) + + if "cipher" in out.decode().split(":")[-1].strip(): if encryption_key: click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") _backup.backup_decryption(encryption_key) @@ -293,8 +296,12 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): frappe.connect(site=site) _backup = Backup(sql_file_path) - backup_mimetype = magic.from_file(sql_file_path) - if "cipher" in backup_mimetype: + err, out = frappe.utils.execute_in_shell(f"file {sql_file_path}", check_exit_code=True) + if err: + click.secho("Failed to detect type of backup file", fg="red") + sys.exit(1) + + if "cipher" in out.decode().split(":")[-1].strip(): if encryption_key: click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") key = encryption_key From 012b0fdb7e5c63a99dfc5f62a7895b450c5afb21 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 1 Nov 2023 19:11:15 +0530 Subject: [PATCH 07/18] fix(postgres/setup): use `gzip` to get backup contents if the file is an archive Signed-off-by: Akhil Narang --- frappe/database/postgres/setup_db.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 8de3e532b9..35bc51d12a 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -50,12 +50,22 @@ def import_db_from_sql(source_sql=None, verbose=False): command = [] - if pv: - command.extend([pv, source_sql, "|"]) - source = [] - print("Restoring Database file...") + if source_sql.endswith(".gz"): + if gzip := which("gzip"): + source = [] + if pv: + command.extend([gzip, "-cd", source_sql, "|", pv, "|"]) + else: + command.extend([gzip, "-cd", source_sql, "|"]) + else: + raise Exception("`gzip` not installed") else: - source = ["-f", source_sql] + if pv: + command.extend([pv, source_sql, "|"]) + source = [] + print("Restoring Database file...") + else: + source = ["-f", source_sql] bin, args, bin_name = get_command( host=frappe.conf.db_host, From 148efbc3ed1cb5ce087d3c95d1ac1a464559318d Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 2 Nov 2023 20:59:07 +0530 Subject: [PATCH 08/18] refactor(restore): adjust downgrade check First actually check whether its a downgrade, then check for the force flag Signed-off-by: Akhil Narang --- frappe/commands/site.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index d35e24a223..b5253ecf8d 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -218,8 +218,8 @@ def _restore( ) sys.exit(1) - # don't allow downgrading to older versions of frappe without force - if not force and is_downgrade(sql_file_path, verbose=True): + # Check if the backup is of an older version of frappe and the user hasn't specified force + if is_downgrade(sql_file_path, verbose=True) and not force: warn_message = ( "This is not recommended and may lead to unexpected behaviour. " "Do you want to continue anyway?" From 2e382040cd96b7aacbe7926a2227038cffb3649f Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 2 Nov 2023 20:59:39 +0530 Subject: [PATCH 09/18] fix: simplify version detection logic Our version detection code is relatively simple, so we shouldn't have any exceptions arising there Just check for a None return value to decide whether we should use the older logic Signed-off-by: Akhil Narang --- frappe/installer.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index a81955833a..461343fd27 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -754,10 +754,8 @@ def is_downgrade(sql_file_path, verbose=False): from semantic_version import Version - backup_version = None - try: - backup_version = extract_version_from_dump(sql_file_path) - except Exception: + backup_version = extract_version_from_dump(sql_file_path) + if backup_version is None: # Handle older backups in the same way head = "INSERT INTO `tabInstalled Application` VALUES" From a06e402f38c4e823ad995749d405bdc488fe3cba Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 6 Nov 2023 13:42:36 +0530 Subject: [PATCH 10/18] refactor: use a function with context manager for backup decryption Signed-off-by: Akhil Narang --- frappe/commands/site.py | 145 ++++++++++++++++++++++++---------------- frappe/utils/backups.py | 62 +++++++---------- 2 files changed, 112 insertions(+), 95 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index b5253ecf8d..859f2cfc85 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -177,16 +177,9 @@ def _restore( with_public_files=None, with_private_files=None, ): - from frappe.installer import ( - _new_site, - extract_files, - is_downgrade, - is_partial, - validate_database_sql, - ) - from frappe.utils.backups import Backup, get_or_generate_backup_encryption_key + from frappe.installer import extract_files + from frappe.utils.backups import decrypt_backup, get_or_generate_backup_encryption_key - _backup = Backup(sql_file_path) err, out = frappe.utils.execute_in_shell(f"file {sql_file_path}", check_exit_code=True) if err: click.secho("Failed to detect type of backup file", fg="red") @@ -195,17 +188,79 @@ def _restore( if "cipher" in out.decode().split(":")[-1].strip(): if encryption_key: click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") - _backup.backup_decryption(encryption_key) else: click.secho("Encrypted backup file detected. Decrypting using site config.", fg="yellow") encryption_key = get_or_generate_backup_encryption_key() - _backup.backup_decryption(encryption_key) - # Rollback on unsuccessful decryption - if not os.path.exists(sql_file_path): - click.secho("Decryption failed. Please provide a valid key and try again.", fg="red") - sys.exit(1) + with decrypt_backup(sql_file_path, encryption_key): + # Rollback on unsuccessful decryption + if not os.path.exists(sql_file_path): + click.secho("Decryption failed. Please provide a valid key and try again.", fg="red") + sys.exit(1) + + restore_backup( + sql_file_path, + site, + db_root_username, + db_root_password, + verbose, + install_app, + admin_password, + force, + ) + else: + restore_backup( + sql_file_path, + site, + db_root_username, + db_root_password, + verbose, + install_app, + admin_password, + force, + ) + + # Extract public and/or private files to the restored site, if user has given the path + if with_public_files: + # Decrypt data if there is a Key + if encryption_key: + with decrypt_backup(with_public_files, encryption_key): + public = extract_files(site, with_public_files) + else: + public = extract_files(site, with_public_files) + + # Removing temporarily created file + os.remove(public) + + if with_private_files: + # Decrypt data if there is a Key + if encryption_key: + with decrypt_backup(with_private_files, encryption_key): + private = extract_files(site, with_private_files) + else: + private = extract_files(site, with_private_files) + + # Removing temporarily created file + os.remove(private) + + success_message = "Site {} has been restored{}".format( + site, " with files" if (with_public_files or with_private_files) else "" + ) + click.secho(success_message, fg="green") + + +def restore_backup( + sql_file_path: str, + site, + db_root_username, + db_root_password, + verbose, + install_app, + admin_password, + force, +): + from frappe.installer import _new_site, is_downgrade, is_partial, validate_database_sql if is_partial(sql_file_path): click.secho( @@ -247,32 +302,6 @@ def _restore( print(err.args[1]) sys.exit(1) - # Extract public and/or private files to the restored site, if user has given the path - if with_public_files: - # Decrypt data if there is a Key - if encryption_key: - _backup = Backup(with_public_files) - _backup.backup_decryption(encryption_key) - public = extract_files(site, with_public_files) - - # Removing temporarily created file - os.remove(public) - - if with_private_files: - # Decrypt data if there is a Key - if encryption_key: - _backup = Backup(with_private_files) - _backup.backup_decryption(encryption_key) - private = extract_files(site, with_private_files) - - # Removing temporarily created file - os.remove(private) - - success_message = "Site {} has been restored{}".format( - site, " with files" if (with_public_files or with_private_files) else "" - ) - click.secho(success_message, fg="green") - @click.command("partial-restore") @click.argument("sql-file-path") @@ -281,21 +310,16 @@ def _restore( @pass_context def partial_restore(context, sql_file_path, verbose, encryption_key=None): from frappe.installer import is_partial, partial_restore - from frappe.utils.backups import Backup, get_or_generate_backup_encryption_key + from frappe.utils.backups import decrypt_backup, get_or_generate_backup_encryption_key if not os.path.exists(sql_file_path): print("Invalid path", sql_file_path) sys.exit(1) site = get_site(context) - frappe.init(site=site) - - _backup = Backup(sql_file_path) - verbose = context.verbose or verbose - + frappe.init(site=site) frappe.connect(site=site) - _backup = Backup(sql_file_path) err, out = frappe.utils.execute_in_shell(f"file {sql_file_path}", check_exit_code=True) if err: click.secho("Failed to detect type of backup file", fg="red") @@ -310,21 +334,30 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): click.secho("Encrypted backup file detected. Decrypting using site config.", fg="yellow") key = get_or_generate_backup_encryption_key() - _backup.backup_decryption(key) + with decrypt_backup(sql_file_path, key): + if not is_partial(sql_file_path): + click.secho( + "Full backup file detected.Use `bench restore` to restore a Frappe Site.", + fg="red", + ) + sys.exit(1) + + partial_restore(sql_file_path, verbose) # Rollback on unsuccessful decryption if not os.path.exists(sql_file_path): click.secho("Decryption failed. Please provide a valid key and try again.", fg="red") sys.exit(1) - if not is_partial(sql_file_path): - click.secho( - "Full backup file detected.Use `bench restore` to restore a Frappe Site.", - fg="red", - ) - sys.exit(1) + else: + if not is_partial(sql_file_path): + click.secho( + "Full backup file detected.Use `bench restore` to restore a Frappe Site.", + fg="red", + ) + sys.exit(1) - partial_restore(sql_file_path, verbose) + partial_restore(sql_file_path, verbose) frappe.destroy() diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index ba546801e5..0e52580927 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -1,5 +1,6 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import contextlib # imports - standard imports import gzip @@ -632,46 +633,29 @@ def get_or_generate_backup_encryption_key(): return key -class Backup: - def __init__(self, file_path): - self.file_path = file_path +@contextlib.contextmanager +def decrypt_backup(file_path: str, passphrase: str): + if not os.path.exists(file_path): + print("Invalid path: ", file_path) + return + else: + file_path_with_ext = file_path + ".gpg" + os.rename(file_path, file_path_with_ext) - def backup_decryption(self, passphrase): - """ - Decrypts backup at the given path using the passphrase. - """ - if not os.path.exists(self.file_path): - print("Invalid path", self.file_path) - return - else: - file_path_with_ext = self.file_path + ".gpg" - os.rename(self.file_path, file_path_with_ext) - - cmd_string = "gpg --yes --passphrase {passphrase} --pinentry-mode loopback -o {decrypted_file} -d {file_location}" - command = cmd_string.format( - passphrase=passphrase, - file_location=file_path_with_ext, - decrypted_file=self.file_path, - ) - frappe.utils.execute_in_shell(command) - - def decryption_rollback(self): - """ - Checks if the decrypted file exists at the given path. - if exists - Renames the orginal encrypted file. - else - Removes the decrypted file and rename the original file. - """ - if os.path.exists(self.file_path + ".gpg"): - if os.path.exists(self.file_path): - os.remove(self.file_path) - if os.path.exists(self.file_path.rstrip(".gz")): - os.remove(self.file_path.rstrip(".gz")) - os.rename(self.file_path + ".gpg", self.file_path) - - def __del__(self): - self.decryption_rollback() + cmd_string = "gpg --yes --passphrase {passphrase} --pinentry-mode loopback -o {decrypted_file} -d {file_location}" + command = cmd_string.format( + passphrase=passphrase, + file_location=file_path_with_ext, + decrypted_file=file_path, + ) + frappe.utils.execute_in_shell(command) + yield + if os.path.exists(file_path + ".gpg"): + if os.path.exists(file_path): + os.remove(file_path) + if os.path.exists(file_path.rstrip(".gz")): + os.remove(file_path.rstrip(".gz")) + os.rename(file_path + ".gpg", file_path) def backup( From 998f2c10d67f5d6af35d2f35872bd5442e40d71b Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 6 Nov 2023 13:57:46 +0530 Subject: [PATCH 11/18] fix: handle older gzipped backups as well Also fix the actual comparison Signed-off-by: Akhil Narang --- frappe/installer.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 461343fd27..22913fe2ee 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -53,7 +53,7 @@ def _new_site( ): """Install a new Frappe site""" - from frappe.utils import get_site_path, scheduler + from frappe.utils import scheduler if not force and os.path.exists(site): print(f"Site {site} already exists") @@ -756,11 +756,17 @@ def is_downgrade(sql_file_path, verbose=False): backup_version = extract_version_from_dump(sql_file_path) if backup_version is None: + if sql_file_path.endswith(".sql.gz"): + open_method = gzip.open + else: + open_method = open # Handle older backups in the same way head = "INSERT INTO `tabInstalled Application` VALUES" - with open(sql_file_path) as f: + with open_method(sql_file_path) as f: for line in f: + if isinstance(line, bytes): + line = line.decode("utf-8").strip() if head in line: # 'line' (str) format: ('2056588823','2020-05-11 18:21:31.488367','2020-06-12 11:49:31.079506','Administrator','Administrator',0,'Installed Applications','installed_applications','Installed Applications',1,'frappe','v10.1.71-74 (3c50d5e) (v10.x.x)','v10.x.x'),('855c640b8e','2020-05-11 18:21:31.488367','2020-06-12 11:49:31.079506','Administrator','Administrator',0,'Installed Applications','installed_applications','Installed Applications',2,'your_custom_app','0.0.1','master') line = line.strip().lstrip(head).rstrip(";").strip() @@ -790,7 +796,7 @@ def is_downgrade(sql_file_path, verbose=False): return False current_version = Version(frappe.__version__) - downgrade = Version(backup_version) > current_version + downgrade = Version(backup_version) < current_version if verbose and downgrade: print(f"Your site will be downgraded from Frappe {backup_version} to {current_version}") From 7f433b84afef89d716f20a702075eff25eb76dda Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 9 Nov 2023 18:13:23 +0530 Subject: [PATCH 12/18] feat: allow creating a backup with the older metadata style Signed-off-by: Akhil Narang --- frappe/commands/site.py | 5 +++++ frappe/utils/backups.py | 28 ++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 859f2cfc85..48a4feea57 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -842,6 +842,9 @@ def use(site, sites_path="."): ) @click.option("--verbose", default=False, is_flag=True, help="Add verbosity") @click.option("--compress", default=False, is_flag=True, help="Compress private and public files") +@click.option( + "--old-backup-metadata", default=False, is_flag=True, help="Use older backup metadata" +) @pass_context def backup( context, @@ -856,6 +859,7 @@ def backup( compress=False, include="", exclude="", + old_backup_metadata=False, ): "Backup" @@ -881,6 +885,7 @@ def backup( compress=compress, verbose=verbose, force=True, + old_backup_metadata=old_backup_metadata, ) except Exception: click.secho( diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 0e52580927..bdba23ac2f 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -55,6 +55,7 @@ class BackupGenerator: include_doctypes="", exclude_doctypes="", verbose=False, + old_backup_metadata=False, ): global _verbose self.compress_files = compress_files or compress @@ -73,6 +74,7 @@ class BackupGenerator: self.include_doctypes = include_doctypes self.exclude_doctypes = exclude_doctypes self.partial = False + self.old_backup_metadata = old_backup_metadata site = frappe.local.site or frappe.generate_hash(length=8) self.site_slug = site.replace(".", "_") @@ -373,14 +375,20 @@ class BackupGenerator: _("gzip not found in PATH! This is required to take a backup."), exc=frappe.ExecutableNotFound ) - database_header_content = [ - "begin frappe metadata", - "[frappe]", - f"version = {frappe.__version__}", - f"branch = {get_app_branch('frappe') or 'N/A'}", - "end frappe metadata", - "", - ] + if self.old_backup_metadata: + database_header_content = [ + f"Backup generated by Frappe {frappe.__version__} on branch {get_app_branch('frappe') or 'N/A'}", + "", + ] + else: + database_header_content = [ + "begin frappe metadata", + "[frappe]", + f"version = {frappe.__version__}", + f"branch = {get_app_branch('frappe') or 'N/A'}", + "end frappe metadata", + "", + ] if self.backup_includes: backup_info = ("Backing Up Tables: ", ", ".join(self.backup_includes)) @@ -516,6 +524,7 @@ def scheduled_backup( compress=False, force=False, verbose=False, + old_backup_metadata=False, ): """this function is called from scheduler deletes backups older than 7 days @@ -534,6 +543,7 @@ def scheduled_backup( compress=compress, force=force, verbose=verbose, + old_backup_metadata=old_backup_metadata, ) @@ -551,6 +561,7 @@ def new_backup( compress=False, force=False, verbose=False, + old_backup_metadata=False, ): delete_temp_backups() odb = BackupGenerator( @@ -570,6 +581,7 @@ def new_backup( exclude_doctypes=exclude_doctypes, verbose=verbose, compress_files=compress, + old_backup_metadata=old_backup_metadata, ) odb.get_backup(older_than, ignore_files, force=force) return odb From 9bf818eb4a655adf945431f46fa9a7ca8d13f3c7 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 9 Nov 2023 18:21:56 +0530 Subject: [PATCH 13/18] chore: add in some tests for backup Signed-off-by: Akhil Narang --- frappe/tests/test_commands.py | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index e6af22dd0c..1de2365713 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -547,6 +547,42 @@ class TestBackups(BaseTestCommands): self.assertIn("successfully completed", self.stdout) self.assertNotEqual(before_backup["database"], after_backup["database"]) + @skipIf( + not (frappe.conf.db_type == "mariadb"), + "Only for MariaDB", + ) + def test_backup_extract_restore(self): + """Restore a backup after extracting""" + self.execute("bench --site {site} backup") + self.assertEqual(self.returncode, 0) + backup = fetch_latest_backups() + self.execute(f"gunzip {backup['database']}") + self.assertEqual(self.returncode, 0) + backup_sql = backup["database"].replace(".gz", "") + assert os.path.isfile(backup_sql) + self.execute( + "bench --site {site} restore {backup_sql}", + { + "backup_sql": backup_sql, + }, + ) + self.assertEqual(self.returncode, 0) + + @skipIf( + not (frappe.conf.db_type == "mariadb"), + "Only for MariaDB", + ) + def test_old_backup_restore(self): + """Restore a backup after extracting""" + self.execute("bench --site {site} backup --old-backup-metadata") + self.assertEqual(self.returncode, 0) + backup = fetch_latest_backups() + self.execute( + "bench --site {site} restore {database}", + backup, + ) + self.assertEqual(self.returncode, 0) + def test_backup_fails_with_exit_code(self): """Provide incorrect options to check if exit code is 1""" odb = BackupGenerator( From b010dc584f4c86941f3405025c4b0991a610ca70 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 20 Nov 2023 16:40:51 +0530 Subject: [PATCH 14/18] chore(installer): fix output when prompting user about downgrade Signed-off-by: Akhil Narang --- frappe/installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/installer.py b/frappe/installer.py index 22913fe2ee..b8962a9cb9 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -799,7 +799,7 @@ def is_downgrade(sql_file_path, verbose=False): downgrade = Version(backup_version) < current_version if verbose and downgrade: - print(f"Your site will be downgraded from Frappe {backup_version} to {current_version}") + print(f"Your site will be downgraded from Frappe {current_version} to {backup_version}") return downgrade From 76c9fbd0cddbd507704906d4f6667b010984bc2b Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 20 Nov 2023 16:50:30 +0530 Subject: [PATCH 15/18] fix: use source before changing its contents Signed-off-by: Akhil Narang --- frappe/database/db_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index 27704fb472..05aff3f9ab 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -63,8 +63,8 @@ class DbManager: if source.endswith(".gz"): if gzip := which("gzip"): - source = [] command.extend([gzip, "-cd", source, "|"]) + source = [] if pv: command.extend([pv, "|"]) print("Restoring Database file...") From 3fe840fb31de57727eaa22ef200ad9f1e90f2602 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 21 Nov 2023 11:00:53 +0530 Subject: [PATCH 16/18] fix(postgres): make use of common helper Signed-off-by: Akhil Narang --- frappe/database/postgres/setup_db.py | 55 ++++------------------------ 1 file changed, 8 insertions(+), 47 deletions(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 35bc51d12a..f5f3b14006 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -1,7 +1,7 @@ import os import frappe -from frappe import _ +from frappe.database.db_manager import DbManager def setup_database(): @@ -36,55 +36,16 @@ def bootstrap_database(db_name, verbose, source_sql=None): def import_db_from_sql(source_sql=None, verbose=False): - import shlex - from shutil import which - - from frappe.database import get_command - from frappe.utils import execute_in_shell - - # bootstrap db + if verbose: + print("Starting database import...") + db_name = frappe.conf.db_name if not source_sql: source_sql = os.path.join(os.path.dirname(__file__), "framework_postgres.sql") - - pv = which("pv") - - command = [] - - if source_sql.endswith(".gz"): - if gzip := which("gzip"): - source = [] - if pv: - command.extend([gzip, "-cd", source_sql, "|", pv, "|"]) - else: - command.extend([gzip, "-cd", source_sql, "|"]) - else: - raise Exception("`gzip` not installed") - else: - if pv: - command.extend([pv, source_sql, "|"]) - source = [] - print("Restoring Database file...") - else: - source = ["-f", source_sql] - - bin, args, bin_name = get_command( - host=frappe.conf.db_host, - port=frappe.conf.db_port, - user=frappe.conf.db_name, - password=frappe.conf.db_password, - db_name=frappe.conf.db_name, + DbManager(frappe.local.db).restore_database( + verbose, db_name, source_sql, db_name, frappe.conf.db_password ) - - if not bin: - frappe.throw( - _("{} not found in PATH! This is required to restore the database.").format(bin_name), - exc=frappe.ExecutableNotFound, - ) - command.append(bin) - command.append(shlex.join(args)) - command.extend(source) - execute_in_shell(" ".join(command), check_exit_code=True, verbose=verbose) - frappe.cache.delete_keys("") # Delete all keys associated with this site. + if verbose: + print("Imported from database %s" % source_sql) def get_root_connection(root_login=None, root_password=None): From 6e8f32af58d3e0dd5d1c15709fa59ec7730647b1 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 30 Nov 2023 13:54:05 +0530 Subject: [PATCH 17/18] chore: don't pipe output through `pv` No point if we're using `execute_in_shell()` Signed-off-by: Akhil Narang --- frappe/database/db_manager.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index 05aff3f9ab..01c18d69c4 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -57,27 +57,17 @@ class DbManager: from frappe.database import get_command from frappe.utils import execute_in_shell - pv = which("pv") - command = [] if source.endswith(".gz"): if gzip := which("gzip"): command.extend([gzip, "-cd", source, "|"]) source = [] - if pv: - command.extend([pv, "|"]) - print("Restoring Database file...") else: raise Exception("`gzip` not installed") else: - if pv: - command.extend([pv, source, "|"]) - source = [] - print("Restoring Database file...") - else: - source = ["<", source] + source = ["<", source] bin, args, bin_name = get_command( host=frappe.conf.db_host, From 43021911ffc6d7c014e17c89874fd9e3b214fe73 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 4 Dec 2023 12:28:29 +0530 Subject: [PATCH 18/18] fix: simplify parsing version for older backups Signed-off-by: Akhil Narang --- frappe/installer.py | 39 ++++----------------------------------- 1 file changed, 4 insertions(+), 35 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index b8962a9cb9..d96f1167f1 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -450,7 +450,6 @@ def _delete_modules(modules: list[str], dry_run: bool) -> list[str]: def _delete_linked_documents( module_name: str, doctype_linkfield_map: dict[str, str], dry_run: bool ) -> None: - """Deleted all records linked with module def""" for doctype, fieldname in doctype_linkfield_map.items(): for record in frappe.get_all(doctype, filters={fieldname: module_name}, pluck="name"): @@ -756,40 +755,10 @@ def is_downgrade(sql_file_path, verbose=False): backup_version = extract_version_from_dump(sql_file_path) if backup_version is None: - if sql_file_path.endswith(".sql.gz"): - open_method = gzip.open - else: - open_method = open - # Handle older backups in the same way - head = "INSERT INTO `tabInstalled Application` VALUES" - - with open_method(sql_file_path) as f: - for line in f: - if isinstance(line, bytes): - line = line.decode("utf-8").strip() - if head in line: - # 'line' (str) format: ('2056588823','2020-05-11 18:21:31.488367','2020-06-12 11:49:31.079506','Administrator','Administrator',0,'Installed Applications','installed_applications','Installed Applications',1,'frappe','v10.1.71-74 (3c50d5e) (v10.x.x)','v10.x.x'),('855c640b8e','2020-05-11 18:21:31.488367','2020-06-12 11:49:31.079506','Administrator','Administrator',0,'Installed Applications','installed_applications','Installed Applications',2,'your_custom_app','0.0.1','master') - line = line.strip().lstrip(head).rstrip(";").strip() - app_rows = frappe.safe_eval(line) - # check if iterable consists of tuples before trying to transform - apps_list = ( - app_rows - if all(isinstance(app_row, (tuple, list, set)) for app_row in app_rows) - else (app_rows,) - ) - # 'all_apps' (list) format: [('frappe', '12.x.x-develop ()', 'develop'), ('your_custom_app', '0.0.1', 'master')] - all_apps = [x[-3:] for x in apps_list] - - for app in all_apps: - app_name = app[0] - app_version = app[1].split(" ", 1)[0] - - if app_name == "frappe": - try: - backup_version = app_version[1:] if app_version[0] == "v" else app_version - break - except ValueError: - return False + # This is likely an older backup, so try to extract another way + header = get_db_dump_header(sql_file_path).split("\n") + if "Version" in header[0]: + backup_version = header[0].split(":")[-1].strip() # Assume it's not a downgrade if we can't determine backup version if backup_version is None: