From 2bbe464f59e2808a35f2d8d2bc938e05b6ef8af2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 24 Jul 2020 17:53:22 +0530 Subject: [PATCH 01/38] feat: Allow skip tables during backups --- frappe/utils/backups.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 3b905de6bd..d0867e40e6 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -148,7 +148,11 @@ class BackupGenerator: args = dict([item[0], frappe.utils.esc(str(item[1]), '$ ')] for item in self.__dict__.copy().items()) - cmd_string = """mysqldump --single-transaction --quick --lock-tables=false -u %(user)s -p%(password)s %(db_name)s -h %(db_host)s -P %(db_port)s | gzip > %(backup_path_db)s """ % args + if self.verbose: + print("Skipping Tables: {0}\n".format(", ".join(frappe.conf.ignore_tables_in_backup))) + + args["skip_tables"] = " ".join(["--ignore-table={0}.{1}".format(frappe.conf.db_name, table) for table in frappe.conf.ignore_tables_in_backup]) + cmd_string = """mysqldump --single-transaction --quick --lock-tables=false -u %(user)s -p%(password)s %(db_name)s -h %(db_host)s -P %(db_port)s %(skip_tables)s | gzip > %(backup_path_db)s """ % args if self.db_type == 'postgres': cmd_string = "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name} | gzip > {backup_path_db}".format( From befea91e6d5b7081960807976107f823ab559d18 Mon Sep 17 00:00:00 2001 From: gavin Date: Mon, 27 Jul 2020 11:01:01 +0530 Subject: [PATCH 02/38] fix: empty conf value for ignore_tables_in_backup Co-authored-by: Faris Ansari --- frappe/utils/backups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index d0867e40e6..e475cef276 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -151,7 +151,7 @@ class BackupGenerator: if self.verbose: print("Skipping Tables: {0}\n".format(", ".join(frappe.conf.ignore_tables_in_backup))) - args["skip_tables"] = " ".join(["--ignore-table={0}.{1}".format(frappe.conf.db_name, table) for table in frappe.conf.ignore_tables_in_backup]) + args["skip_tables"] = " ".join(["--ignore-table={0}.{1}".format(frappe.conf.db_name, table) for table in frappe.conf.ignore_tables_in_backup or []]) cmd_string = """mysqldump --single-transaction --quick --lock-tables=false -u %(user)s -p%(password)s %(db_name)s -h %(db_host)s -P %(db_port)s %(skip_tables)s | gzip > %(backup_path_db)s """ % args if self.db_type == 'postgres': From 6cb6a18b48a050efc7ee1e307f180d3e4ba55ec3 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 23 Sep 2020 13:18:04 +0530 Subject: [PATCH 03/38] feat: Ability to exclude/include certain doctypes from conf or via CLI --- frappe/commands/site.py | 18 +++++++- frappe/utils/backups.py | 92 +++++++++++++++++++++++++++++++++-------- 2 files changed, 90 insertions(+), 20 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index c5008b32a5..a727341b73 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -387,10 +387,13 @@ def use(site, sites_path='.'): @click.command('backup') @click.option('--with-files', default=False, is_flag=True, help="Take backup with files") +@click.option('--ignore-backup-conf', default=False, is_flag=True, help="Ignore excludes/includes set in config") +@click.option('--include', default="", type=str, help="Specify the DocTypes to backup seperated by commas") +@click.option('--exclude', default="", type=str, help="Specify the DocTypes to not backup seperated by commas") @click.option('--verbose', default=False, is_flag=True) @pass_context def backup(context, with_files=False, backup_path_db=None, backup_path_files=None, - backup_path_private_files=None, quiet=False, verbose=False): + backup_path_private_files=None, quiet=False, verbose=False, ignore_backup_conf=False, include="", exclude=""): "Backup" from frappe.utils.backups import scheduled_backup verbose = verbose or context.verbose @@ -399,10 +402,21 @@ def backup(context, with_files=False, backup_path_db=None, backup_path_files=Non try: frappe.init(site=site) frappe.connect() - odb = scheduled_backup(ignore_files=not with_files, backup_path_db=backup_path_db, backup_path_files=backup_path_files, backup_path_private_files=backup_path_private_files, force=True, verbose=verbose) + odb = scheduled_backup( + ignore_files=not with_files, + backup_path_db=backup_path_db, + backup_path_files=backup_path_files, + backup_path_private_files=backup_path_private_files, + ignore_conf=ignore_backup_conf, + include_doctypes=include, + exclude_doctypes=exclude, + verbose=verbose, + force=True, + ) except Exception as e: if verbose: print("Backup failed for {0}. Database or site_config.json may be corrupted".format(site)) + print(e) exit_code = 1 continue diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 4d29134abd..d3a063cd89 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -18,6 +18,7 @@ from frappe.utils import cstr, get_url, now_datetime # backup variable for backwards compatibility verbose = False _verbose = verbose +base_tables = ["__Auth", "__global_search", "__UserSettings"] class BackupGenerator: @@ -29,7 +30,7 @@ class BackupGenerator: """ def __init__(self, db_name, user, password, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, db_host="localhost", db_port=None, verbose=False, - db_type='mariadb', backup_path_conf=None): + db_type='mariadb', backup_path_conf=None, ignore_conf=False, include_doctypes="", exclude_doctypes=""): global _verbose self.db_host = db_host self.db_port = db_port @@ -41,6 +42,9 @@ class BackupGenerator: self.backup_path_db = backup_path_db self.backup_path_files = backup_path_files self.backup_path_private_files = backup_path_private_files + self.ignore_conf = ignore_conf + self.include_doctypes = include_doctypes + self.exclude_doctypes = exclude_doctypes if not self.db_type: self.db_type = 'mariadb' @@ -54,6 +58,7 @@ class BackupGenerator: self.site_slug = site.replace('.', '_') self.verbose = verbose self.setup_backup_directory() + self.setup_backup_tables() _verbose = verbose def setup_backup_directory(self): @@ -68,6 +73,35 @@ class BackupGenerator: dir = os.path.dirname(file_path) os.makedirs(dir, exist_ok=True) + def setup_backup_tables(self): + """Sets self.backup_includes, self.backup_excludes based on passed args + """ + def get_tables(doctypes): + tables = [] + for doctype in doctypes: + if doctype: + if doctype.startswith("tab"): + tables.append(doctype) + else: + tables.append("tab" + doctype) + return tables + + passed_tables = { + "include": get_tables(self.include_doctypes.strip().split(",")), + "exclude": get_tables(self.exclude_doctypes.strip().split(",")) + } + conf_tables = { + "include": get_tables(frappe.conf.backup.get("includes", [])) + base_tables, + "exclude": get_tables(frappe.conf.backup.get("excludes", [])) + } + + self.backup_includes = passed_tables["include"] + self.backup_excludes = passed_tables["exclude"] + + if not self.ignore_conf: + self.backup_includes = self.backup_includes or conf_tables["include"] + self.backup_excludes = self.backup_excludes or conf_tables["exclude"] + @property def site_config_backup_path(self): # For backwards compatibility @@ -189,23 +223,42 @@ class BackupGenerator: args = dict([item[0], frappe.utils.esc(str(item[1]), '$ ')] for item in self.__dict__.copy().items()) - if self.verbose: - print("Skipping Tables: {0}\n".format(", ".join(frappe.conf.ignore_tables_in_backup))) - - args["skip_tables"] = " ".join(["--ignore-table={0}.{1}".format(frappe.conf.db_name, table) for table in frappe.conf.ignore_tables_in_backup or []]) - cmd_string = """mysqldump --single-transaction --quick --lock-tables=false -u %(user)s -p%(password)s %(db_name)s -h %(db_host)s -P %(db_port)s %(skip_tables)s | gzip > %(backup_path_db)s """ % args + if self.backup_includes: + print("Backing Up Tables: {0}\n".format(", ".join(self.backup_includes))) + elif self.backup_excludes: + print("Skipping Tables: {0}\n".format(", ".join(self.backup_excludes))) if self.db_type == 'postgres': - cmd_string = "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name} | gzip > {backup_path_db}".format( - user=args.get('user'), - password=args.get('password'), - db_host=args.get('db_host'), - db_port=args.get('db_port'), - db_name=args.get('db_name'), - backup_path_db=args.get('backup_path_db') - ) + if self.backup_includes: + args["include"] = " ".join(["--table='{0}'".format(table) for table in self.backup_includes]) + elif self.backup_excludes: + args["exclude"] = " ".join(["--exclude-table='{0}'".format(table) for table in self.backup_excludes]) - err, out = frappe.utils.execute_in_shell(cmd_string) + cmd_string = "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name} {include} {exclude} | gzip > {backup_path_db}" + + else: + if self.backup_includes: + args["include"] = " ".join(["'{0}'".format(x) for x in self.backup_includes]) + + elif self.backup_excludes: + args["exclude"] = " ".join(["--ignore-table='{0}.{1}'".format(frappe.conf.db_name, table) for table in self.backup_excludes]) + + cmd_string = "mysqldump --single-transaction --quick --lock-tables=false -u {user} -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude} | gzip > {backup_path_db}" + + command = cmd_string.format( + user=args.get('user'), + password=args.get('password'), + db_host=args.get('db_host'), + db_port=args.get('db_port'), + db_name=args.get('db_name'), + backup_path_db=args.get('backup_path_db'), + exclude=args.get('exclude', ''), + include=args.get('include', '') + ) + + print(command) + + err, out = frappe.utils.execute_in_shell(command) def send_email(self): """ @@ -279,14 +332,14 @@ def fetch_latest_backups(): } -def scheduled_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, force=False, verbose=False): +def scheduled_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, force=False, verbose=False, ignore_conf=False, include_doctypes="", exclude_doctypes=""): """this function is called from scheduler deletes backups older than 7 days takes backup""" - odb = new_backup(older_than, ignore_files, backup_path_db=backup_path_db, backup_path_files=backup_path_files, force=force, verbose=verbose) + odb = new_backup(older_than, ignore_files, backup_path_db=backup_path_db, backup_path_files=backup_path_files, force=force, verbose=verbose, ignore_conf=ignore_conf, include_doctypes=include_doctypes, exclude_doctypes=exclude_doctypes) return odb -def new_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, force=False, verbose=False): +def new_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, force=False, verbose=False, ignore_conf=False, include_doctypes="", exclude_doctypes=""): delete_temp_backups(older_than = frappe.conf.keep_backups_for_hours or 24) odb = BackupGenerator(frappe.conf.db_name, frappe.conf.db_name,\ frappe.conf.db_password, @@ -295,6 +348,9 @@ def new_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_pat db_host = frappe.db.host, db_port = frappe.db.port, db_type = frappe.conf.db_type, + ignore_conf=ignore_conf, + include_doctypes=include_doctypes, + exclude_doctypes=exclude_doctypes, verbose=verbose) odb.get_backup(older_than, ignore_files, force=force) return odb From bb2a1910e86c959286f461ededdf7678dffa6dfc Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 24 Sep 2020 11:34:43 +0530 Subject: [PATCH 04/38] fix: Handle excludes or includes explicitly * Take backup only if doctype exists * Show mysql command only if verbose is passed --- frappe/utils/backups.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index d3a063cd89..b8d1da299a 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -76,10 +76,11 @@ class BackupGenerator: def setup_backup_tables(self): """Sets self.backup_includes, self.backup_excludes based on passed args """ + existing_doctypes = set([x.name for x in frappe.get_all("DocType")]) def get_tables(doctypes): tables = [] for doctype in doctypes: - if doctype: + if doctype and doctype in existing_doctypes: if doctype.startswith("tab"): tables.append(doctype) else: @@ -98,7 +99,7 @@ class BackupGenerator: self.backup_includes = passed_tables["include"] self.backup_excludes = passed_tables["exclude"] - if not self.ignore_conf: + if not (self.backup_includes or self.backup_excludes) and not self.ignore_conf: self.backup_includes = self.backup_includes or conf_tables["include"] self.backup_excludes = self.backup_excludes or conf_tables["exclude"] @@ -239,7 +240,6 @@ class BackupGenerator: else: if self.backup_includes: args["include"] = " ".join(["'{0}'".format(x) for x in self.backup_includes]) - elif self.backup_excludes: args["exclude"] = " ".join(["--ignore-table='{0}.{1}'".format(frappe.conf.db_name, table) for table in self.backup_excludes]) @@ -256,7 +256,8 @@ class BackupGenerator: include=args.get('include', '') ) - print(command) + if self.verbose: + print(command) err, out = frappe.utils.execute_in_shell(command) From bde7adeb2f597ef59d7c173ad5265b4b1fab6c27 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 14:06:28 +0530 Subject: [PATCH 05/38] style: Black + flake8 --- frappe/utils/backups.py | 445 ++++++++++++++++++++++++++++------------ 1 file changed, 314 insertions(+), 131 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 25147f2085..b59f5526e9 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -2,7 +2,6 @@ # MIT License. See license.txt # imports - standard imports -import json import os from calendar import timegm from datetime import datetime @@ -25,12 +24,31 @@ base_tables = ["__Auth", "__global_search", "__UserSettings"] class BackupGenerator: """ - This class contains methods to perform On Demand Backup + This class contains methods to perform On Demand Backup - To initialize, specify (db_name, user, password, db_file_name=None, db_host="localhost") - If specifying db_file_name, also append ".sql.gz" + To initialize, specify (db_name, user, password, db_file_name=None, db_host="localhost") + If specifying db_file_name, also append ".sql.gz" """ - def __init__(self, db_name, user, password, backup_path=None, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, db_host="localhost", db_port=None, db_type='mariadb', backup_path_conf=None, ignore_conf=False, compress_files=False, include_doctypes="", exclude_doctypes="", verbose=False): + + def __init__( + self, + db_name, + user, + password, + backup_path=None, + backup_path_db=None, + backup_path_files=None, + backup_path_private_files=None, + db_host="localhost", + db_port=None, + db_type="mariadb", + backup_path_conf=None, + ignore_conf=False, + compress_files=False, + include_doctypes="", + exclude_doctypes="", + verbose=False, + ): global _verbose self.compress_files = compress_files or compress self.db_host = db_host @@ -49,22 +67,29 @@ class BackupGenerator: self.exclude_doctypes = exclude_doctypes if not self.db_type: - self.db_type = 'mariadb' + self.db_type = "mariadb" - if not self.db_port and self.db_type == 'mariadb': - self.db_port = 3306 - elif not self.db_port and self.db_type == 'postgres': - self.db_port = 5432 + if not self.db_port: + if self.db_type == "mariadb": + self.db_port = 3306 + if self.db_type == "postgres": + self.db_port = 5432 site = frappe.local.site or frappe.generate_hash(length=8) - self.site_slug = site.replace('.', '_') + self.site_slug = site.replace(".", "_") self.verbose = verbose self.setup_backup_directory() self.setup_backup_tables() _verbose = verbose def setup_backup_directory(self): - specified = self.backup_path or self.backup_path_db or self.backup_path_files or self.backup_path_private_files or self.backup_path_conf + specified = ( + self.backup_path + or self.backup_path_db + or self.backup_path_files + or self.backup_path_private_files + or self.backup_path_conf + ) if not specified: backups_folder = get_backup_path() @@ -74,15 +99,22 @@ class BackupGenerator: if self.backup_path: os.makedirs(self.backup_path, exist_ok=True) - for file_path in set([self.backup_path_files, self.backup_path_db, self.backup_path_private_files, self.backup_path_conf]): + for file_path in set( + [ + self.backup_path_files, + self.backup_path_db, + self.backup_path_private_files, + self.backup_path_conf, + ] + ): if file_path: dir = os.path.dirname(file_path) os.makedirs(dir, exist_ok=True) def setup_backup_tables(self): - """Sets self.backup_includes, self.backup_excludes based on passed args - """ + """Sets self.backup_includes, self.backup_excludes based on passed args""" existing_doctypes = set([x.name for x in frappe.get_all("DocType")]) + def get_tables(doctypes): tables = [] for doctype in doctypes: @@ -95,11 +127,11 @@ class BackupGenerator: passed_tables = { "include": get_tables(self.include_doctypes.strip().split(",")), - "exclude": get_tables(self.exclude_doctypes.strip().split(",")) + "exclude": get_tables(self.exclude_doctypes.strip().split(",")), } conf_tables = { "include": get_tables(frappe.conf.backup.get("includes", [])) + base_tables, - "exclude": get_tables(frappe.conf.backup.get("excludes", [])) + "exclude": get_tables(frappe.conf.backup.get("excludes", [])), } self.backup_includes = passed_tables["include"] @@ -112,24 +144,43 @@ class BackupGenerator: @property def site_config_backup_path(self): # For backwards compatibility - click.secho("BackupGenerator.site_config_backup_path has been deprecated in favour of BackupGenerator.backup_path_conf", fg="yellow") + click.secho( + "BackupGenerator.site_config_backup_path has been deprecated in favour of" + " BackupGenerator.backup_path_conf", + fg="yellow", + ) return getattr(self, "backup_path_conf", None) def get_backup(self, older_than=24, ignore_files=False, force=False): """ - Takes a new dump if existing file is old - and sends the link to the file as email + Takes a new dump if existing file is old + and sends the link to the file as email """ - #Check if file exists and is less than a day old - #If not Take Dump + # Check if file exists and is less than a day old + # If not Take Dump if not force: - last_db, last_file, last_private_file, site_config_backup_path = self.get_recent_backup(older_than) + ( + last_db, + last_file, + last_private_file, + site_config_backup_path, + ) = self.get_recent_backup(older_than) else: - last_db, last_file, last_private_file, site_config_backup_path = False, False, False, False + last_db, last_file, last_private_file, site_config_backup_path = ( + False, + False, + False, + False, + ) - self.todays_date = now_datetime().strftime('%Y%m%d_%H%M%S') + self.todays_date = now_datetime().strftime("%Y%m%d_%H%M%S") - if not (self.backup_path_conf and self.backup_path_db and self.backup_path_files and self.backup_path_private_files): + if not ( + self.backup_path_conf + and self.backup_path_db + and self.backup_path_files + and self.backup_path_private_files + ): self.set_backup_file_name() if not (last_db and last_file and last_private_file and site_config_backup_path): @@ -145,7 +196,7 @@ class BackupGenerator: self.backup_path_conf = site_config_backup_path def set_backup_file_name(self): - #Generate a random name using today's date and a 8 digit random number + # Generate a random name using today's date and a 8 digit random number for_conf = self.todays_date + "-" + self.site_slug + "-site_config_backup.json" for_db = self.todays_date + "-" + self.site_slug + "-database.sql.gz" ext = "tgz" if self.compress_files else "tar" @@ -191,8 +242,7 @@ class BackupGenerator: return file_path latest_backups = { - file_type: get_latest(pattern) - for file_type, pattern in file_type_slugs.items() + file_type: get_latest(pattern) for file_type, pattern in file_type_slugs.items() } recent_backups = { @@ -208,32 +258,40 @@ class BackupGenerator: def zip_files(self): # For backwards compatibility - pre v13 - click.secho("BackupGenerator.zip_files has been deprecated in favour of BackupGenerator.backup_files", fg="yellow") + click.secho( + "BackupGenerator.zip_files has been deprecated in favour of" + " BackupGenerator.backup_files", + fg="yellow", + ) return self.backup_files() def get_summary(self): summary = { "config": { "path": self.backup_path_conf, - "size": get_file_size(self.backup_path_conf, format=True) + "size": get_file_size(self.backup_path_conf, format=True), }, "database": { "path": self.backup_path_db, - "size": get_file_size(self.backup_path_db, format=True) - } + "size": get_file_size(self.backup_path_db, format=True), + }, } - if os.path.exists(self.backup_path_files) and os.path.exists(self.backup_path_private_files): - summary.update({ - "public": { - "path": self.backup_path_files, - "size": get_file_size(self.backup_path_files, format=True) - }, - "private": { - "path": self.backup_path_private_files, - "size": get_file_size(self.backup_path_private_files, format=True) + if os.path.exists(self.backup_path_files) and os.path.exists( + self.backup_path_private_files + ): + summary.update( + { + "public": { + "path": self.backup_path_files, + "size": get_file_size(self.backup_path_files, format=True), + }, + "private": { + "path": self.backup_path_private_files, + "size": get_file_size(self.backup_path_private_files, format=True), + }, } - }) + ) return summary @@ -249,13 +307,17 @@ class BackupGenerator: for folder in ("public", "private"): files_path = frappe.get_site_path(folder, "files") - backup_path = self.backup_path_files if folder=="public" else self.backup_path_private_files + backup_path = ( + self.backup_path_files if folder == "public" else self.backup_path_private_files + ) if self.compress_files: cmd_string = "tar cf - {1} | gzip > {0}" else: cmd_string = "tar -cf {0} {1}" - output = subprocess.check_output(cmd_string.format(backup_path, files_path), shell=True) + output = subprocess.check_output( + cmd_string.format(backup_path, files_path), shell=True + ) if self.verbose and output: print(output.decode("utf8")) @@ -271,39 +333,57 @@ class BackupGenerator: import frappe.utils # escape reserved characters - args = dict([item[0], frappe.utils.esc(str(item[1]), '$ ')] - for item in self.__dict__.copy().items()) + args = dict( + [item[0], frappe.utils.esc(str(item[1]), "$ ")] + for item in self.__dict__.copy().items() + ) if self.backup_includes: print("Backing Up Tables: {0}\n".format(", ".join(self.backup_includes))) elif self.backup_excludes: print("Skipping Tables: {0}\n".format(", ".join(self.backup_excludes))) - if self.db_type == 'postgres': + if self.db_type == "postgres": if self.backup_includes: - args["include"] = " ".join(["--table='{0}'".format(table) for table in self.backup_includes]) + args["include"] = " ".join( + ["--table='{0}'".format(table) for table in self.backup_includes] + ) elif self.backup_excludes: - args["exclude"] = " ".join(["--exclude-table='{0}'".format(table) for table in self.backup_excludes]) + args["exclude"] = " ".join( + ["--exclude-table='{0}'".format(table) for table in self.backup_excludes] + ) - cmd_string = "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name} {include} {exclude} | gzip > {backup_path_db}" + cmd_string = ( + "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name}" + " {include} {exclude} | gzip > {backup_path_db}" + ) else: if self.backup_includes: args["include"] = " ".join(["'{0}'".format(x) for x in self.backup_includes]) elif self.backup_excludes: - args["exclude"] = " ".join(["--ignore-table='{0}.{1}'".format(frappe.conf.db_name, table) for table in self.backup_excludes]) + args["exclude"] = " ".join( + [ + "--ignore-table='{0}.{1}'".format(frappe.conf.db_name, table) + for table in self.backup_excludes + ] + ) - cmd_string = "mysqldump --single-transaction --quick --lock-tables=false -u {user} -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude} | gzip > {backup_path_db}" + cmd_string = ( + "mysqldump --single-transaction --quick --lock-tables=false -u {user}" + " -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude}" + " | gzip > {backup_path_db}" + ) command = cmd_string.format( - user=args.get('user'), - password=args.get('password'), - db_host=args.get('db_host'), - db_port=args.get('db_port'), - db_name=args.get('db_name'), - backup_path_db=args.get('backup_path_db'), - exclude=args.get('exclude', ''), - include=args.get('include', '') + user=args.get("user"), + password=args.get("password"), + db_host=args.get("db_host"), + db_port=args.get("db_port"), + db_name=args.get("db_name"), + backup_path_db=args.get("backup_path_db"), + exclude=args.get("exclude", ""), + include=args.get("include", ""), ) if self.verbose: @@ -313,13 +393,17 @@ class BackupGenerator: def send_email(self): """ - Sends the link to backup file located at erpnext/backups + Sends the link to backup file located at erpnext/backups """ from frappe.email import get_system_managers recipient_list = get_system_managers() - db_backup_url = get_url(os.path.join('backups', os.path.basename(self.backup_path_db))) - files_backup_url = get_url(os.path.join('backups', os.path.basename(self.backup_path_files))) + db_backup_url = get_url( + os.path.join("backups", os.path.basename(self.backup_path_db)) + ) + files_backup_url = get_url( + os.path.join("backups", os.path.basename(self.backup_path_files)) + ) msg = """Hello, @@ -331,11 +415,13 @@ Your backups are ready to be downloaded. This link will be valid for 24 hours. A new backup will be available for download only after 24 hours.""" % { "db_backup_url": db_backup_url, - "files_backup_url": files_backup_url + "files_backup_url": files_backup_url, } datetime_str = datetime.fromtimestamp(os.stat(self.backup_path_db).st_ctime) - subject = datetime_str.strftime("%d/%m/%Y %H:%M:%S") + """ - Backup ready to be downloaded""" + subject = ( + datetime_str.strftime("%d/%m/%Y %H:%M:%S") + """ - Backup ready to be downloaded""" + ) frappe.sendmail(recipients=recipient_list, msg=msg, subject=subject) return recipient_list @@ -344,16 +430,25 @@ download only after 24 hours.""" % { @frappe.whitelist() def get_backup(): """ - This function is executed when the user clicks on - Toos > Download Backup + This function is executed when the user clicks on + Toos > Download Backup """ delete_temp_backups() - odb = BackupGenerator(frappe.conf.db_name, frappe.conf.db_name,\ - frappe.conf.db_password, db_host = frappe.db.host,\ - db_type=frappe.conf.db_type, db_port=frappe.conf.db_port) + odb = BackupGenerator( + frappe.conf.db_name, + frappe.conf.db_name, + frappe.conf.db_password, + db_host=frappe.db.host, + db_type=frappe.conf.db_type, + db_port=frappe.conf.db_port, + ) odb.get_backup() recipient_list = odb.send_email() - frappe.msgprint(_("Download link for your backup will be emailed on the following email address: {0}").format(', '.join(recipient_list))) + frappe.msgprint( + _( + "Download link for your backup will be emailed on the following email address: {0}" + ).format(", ".join(recipient_list)) + ) @frappe.whitelist() @@ -375,44 +470,86 @@ def fetch_latest_backups(): ) database, public, private, config = odb.get_recent_backup(older_than=24 * 30) - return { - "database": database, - "public": public, - "private": private, - "config": config - } + return {"database": database, "public": public, "private": private, "config": config} -def scheduled_backup(older_than=6, ignore_files=False, backup_path=None, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, backup_path_conf=None, ignore_conf=False, include_doctypes="", exclude_doctypes="", compress=False, force=False, verbose=False): +def scheduled_backup( + older_than=6, + ignore_files=False, + backup_path=None, + backup_path_db=None, + backup_path_files=None, + backup_path_private_files=None, + backup_path_conf=None, + ignore_conf=False, + include_doctypes="", + exclude_doctypes="", + compress=False, + force=False, + verbose=False, +): """this function is called from scheduler - deletes backups older than 7 days - takes backup""" - odb = new_backup(older_than=older_than, ignore_files=ignore_files, backup_path=None, backup_path_db=backup_path_db, backup_path_files=backup_path_files, backup_path_private_files=backup_path_private_files, backup_path_conf=backup_path_conf, ignore_conf=ignore_conf, include_doctypes=include_doctypes, exclude_doctypes=exclude_doctypes, compress=compress, force=force, verbose=verbose) + deletes backups older than 7 days + takes backup""" + odb = new_backup( + older_than=older_than, + ignore_files=ignore_files, + backup_path=None, + backup_path_db=backup_path_db, + backup_path_files=backup_path_files, + backup_path_private_files=backup_path_private_files, + backup_path_conf=backup_path_conf, + ignore_conf=ignore_conf, + include_doctypes=include_doctypes, + exclude_doctypes=exclude_doctypes, + compress=compress, + force=force, + verbose=verbose, + ) return odb -def new_backup(older_than=6, ignore_files=False, backup_path=None, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, backup_path_conf=None, ignore_conf=False, include_doctypes="", exclude_doctypes="", compress=False, force=False, verbose=False): - delete_temp_backups(older_than = frappe.conf.keep_backups_for_hours or 24) - odb = BackupGenerator(frappe.conf.db_name, frappe.conf.db_name,\ - frappe.conf.db_password, - db_host = frappe.db.host, - db_port = frappe.db.port, - db_type = frappe.conf.db_type, - backup_path=backup_path, - backup_path_db=backup_path_db, - backup_path_files=backup_path_files, - backup_path_private_files=backup_path_private_files, - backup_path_conf=backup_path_conf, - ignore_conf=ignore_conf, - include_doctypes=include_doctypes, - exclude_doctypes=exclude_doctypes, - verbose=verbose, - compress_files=compress) + +def new_backup( + older_than=6, + ignore_files=False, + backup_path=None, + backup_path_db=None, + backup_path_files=None, + backup_path_private_files=None, + backup_path_conf=None, + ignore_conf=False, + include_doctypes="", + exclude_doctypes="", + compress=False, + force=False, + verbose=False, +): + delete_temp_backups(older_than=frappe.conf.keep_backups_for_hours or 24) + odb = BackupGenerator( + frappe.conf.db_name, + frappe.conf.db_name, + frappe.conf.db_password, + db_host=frappe.db.host, + db_port=frappe.db.port, + db_type=frappe.conf.db_type, + backup_path=backup_path, + backup_path_db=backup_path_db, + backup_path_files=backup_path_files, + backup_path_private_files=backup_path_private_files, + backup_path_conf=backup_path_conf, + ignore_conf=ignore_conf, + include_doctypes=include_doctypes, + exclude_doctypes=exclude_doctypes, + verbose=verbose, + compress_files=compress, + ) odb.get_backup(older_than, ignore_files, force=force) return odb + def delete_temp_backups(older_than=24): """ - Cleans up the backup_link_path directory by deleting files older than 24 hours + Cleans up the backup_link_path directory by deleting files older than 24 hours """ backup_path = get_backup_path() if os.path.exists(backup_path): @@ -422,54 +559,72 @@ def delete_temp_backups(older_than=24): if is_file_old(this_file_path, older_than): os.remove(this_file_path) + def is_file_old(db_file_name, older_than=24): - """ - Checks if file exists and is older than specified hours - Returns -> - True: file does not exist or file is old - False: file is new - """ - if os.path.isfile(db_file_name): - from datetime import timedelta - #Get timestamp of the file - file_datetime = datetime.fromtimestamp\ - (os.stat(db_file_name).st_ctime) - if datetime.today() - file_datetime >= timedelta(hours = older_than): - if _verbose: - print("File is old") - return True - else: - if _verbose: - print("File is recent") - return False + """ + Checks if file exists and is older than specified hours + Returns -> + True: file does not exist or file is old + False: file is new + """ + if os.path.isfile(db_file_name): + from datetime import timedelta + + # Get timestamp of the file + file_datetime = datetime.fromtimestamp(os.stat(db_file_name).st_ctime) + if datetime.today() - file_datetime >= timedelta(hours=older_than): + if _verbose: + print("File is old") + return True else: if _verbose: - print("File does not exist") - return True + print("File is recent") + return False + else: + if _verbose: + print("File does not exist") + return True + def get_backup_path(): backup_path = frappe.utils.get_site_path(conf.get("backup_path", "private/backups")) return backup_path -def backup(with_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, backup_path_conf=None, quiet=False): + +def backup( + with_files=False, + backup_path_db=None, + backup_path_files=None, + backup_path_private_files=None, + backup_path_conf=None, + quiet=False, +): "Backup" - odb = scheduled_backup(ignore_files=not with_files, backup_path_db=backup_path_db, backup_path_files=backup_path_files, backup_path_private_files=backup_path_private_files, backup_path_conf=backup_path_conf, force=True) + odb = scheduled_backup( + ignore_files=not with_files, + backup_path_db=backup_path_db, + backup_path_files=backup_path_files, + backup_path_private_files=backup_path_private_files, + backup_path_conf=backup_path_conf, + force=True, + ) return { "backup_path_db": odb.backup_path_db, "backup_path_files": odb.backup_path_files, - "backup_path_private_files": odb.backup_path_private_files + "backup_path_private_files": odb.backup_path_private_files, } if __name__ == "__main__": """ - is_file_old db_name user password db_host db_type db_port - get_backup db_name user password db_host db_type db_port + is_file_old db_name user password db_host db_type db_port + get_backup db_name user password db_host db_type db_port """ import sys + cmd = sys.argv[1] - db_type = 'mariadb' + db_type = "mariadb" try: db_type = sys.argv[6] except IndexError: @@ -482,19 +637,47 @@ if __name__ == "__main__": pass if cmd == "is_file_old": - odb = BackupGenerator(sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5] or "localhost", db_type=db_type, db_port=db_port) + odb = BackupGenerator( + sys.argv[2], + sys.argv[3], + sys.argv[4], + sys.argv[5] or "localhost", + db_type=db_type, + db_port=db_port, + ) is_file_old(odb.db_file_name) if cmd == "get_backup": - odb = BackupGenerator(sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5] or "localhost", db_type=db_type, db_port=db_port) + odb = BackupGenerator( + sys.argv[2], + sys.argv[3], + sys.argv[4], + sys.argv[5] or "localhost", + db_type=db_type, + db_port=db_port, + ) odb.get_backup() if cmd == "take_dump": - odb = BackupGenerator(sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5] or "localhost", db_type=db_type, db_port=db_port) + odb = BackupGenerator( + sys.argv[2], + sys.argv[3], + sys.argv[4], + sys.argv[5] or "localhost", + db_type=db_type, + db_port=db_port, + ) odb.take_dump() if cmd == "send_email": - odb = BackupGenerator(sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5] or "localhost", db_type=db_type, db_port=db_port) + odb = BackupGenerator( + sys.argv[2], + sys.argv[3], + sys.argv[4], + sys.argv[5] or "localhost", + db_type=db_type, + db_port=db_port, + ) odb.send_email("abc.sql.gz") if cmd == "delete_temp_backups": From bd6f52305eb1e48f370217de89608a16976e207c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 14:32:18 +0530 Subject: [PATCH 06/38] fix: Show traceback if verbose --- frappe/commands/site.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index dc5d8b838e..446166d44c 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -422,6 +422,8 @@ def backup(context, with_files=False, backup_path=None, backup_path_db=None, bac ) except Exception: click.secho("Backup failed for Site {0}. Database or site_config.json may be corrupted".format(site), fg="red") + if verbose: + print(frappe.get_traceback()) exit_code = 1 continue From 7519ba5e1c81a721bc248762b28f03657b12210a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 14:32:40 +0530 Subject: [PATCH 07/38] fix: Backup tables correctly ? --- frappe/utils/backups.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index b59f5526e9..5a91ad656e 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -129,9 +129,12 @@ class BackupGenerator: "include": get_tables(self.include_doctypes.strip().split(",")), "exclude": get_tables(self.exclude_doctypes.strip().split(",")), } + specified_tables = get_tables(frappe.conf.get("backup", {}).get("includes", [])) + include_tables = (specified_tables + base_tables) if specified_tables else [] + conf_tables = { - "include": get_tables(frappe.conf.backup.get("includes", [])) + base_tables, - "exclude": get_tables(frappe.conf.backup.get("excludes", [])), + "include": include_tables, + "exclude": get_tables(frappe.conf.get("backup", {}).get("excludes", [])), } self.backup_includes = passed_tables["include"] @@ -387,7 +390,7 @@ class BackupGenerator: ) if self.verbose: - print(command) + print(command + "\n") err, out = frappe.utils.execute_in_shell(command) From 7d638d298c098036fcf509d3a6086418644e4b57 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 14:33:02 +0530 Subject: [PATCH 08/38] chore: remove old comment that lied --- frappe/utils/backups.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 5a91ad656e..5b1f6b0616 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -619,10 +619,6 @@ def backup( if __name__ == "__main__": - """ - is_file_old db_name user password db_host db_type db_port - get_backup db_name user password db_host db_type db_port - """ import sys cmd = sys.argv[1] From 26387401b39517f61fd50f965a5bf84c99e9817a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 21:06:23 +0530 Subject: [PATCH 09/38] test: Added tests for bench backup skip tables * Added utils for coloured outputs * Fixed bug during last branch update --- frappe/tests/test_commands.py | 112 +++++++++++++++++++++++++++++++++- frappe/utils/backups.py | 2 +- 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index e7e639c775..39d76aaba5 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -1,10 +1,13 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors # imports - standard imports +import json import os import shlex import subprocess +import sys import unittest +import gzip from glob import glob # imports - module imports @@ -12,12 +15,66 @@ import frappe from frappe.utils.backups import fetch_latest_backups +# TODO: check frappe.cli.coloured_output to set coloured output! + +def supports_color(): + """ + Returns True if the running system's terminal supports color, and False + otherwise. + """ + plat = sys.platform + supported_platform = plat != 'Pocket PC' and (plat != 'win32' or 'ANSICON' in os.environ) + # isatty is not always implemented, #6223. + is_a_tty = hasattr(sys.stdout, 'isatty') and sys.stdout.isatty() + return supported_platform and is_a_tty + + +class color(dict): + nc = '\033[0m' + blue = '\033[94m' + green = '\033[92m' + yellow = '\033[93m' + red = '\033[91m' + silver = '\033[90m' + + def __getattr__(self, key): + if supports_color(): + ret = self.get(key) + else: + ret = "" + return ret + + def clean(value): - if isinstance(value, (bytes, str)): - value = value.decode().strip() + """Strips and converts bytes to str + + Args: + value ([type]): [description] + + Returns: + [type]: [description] + """ + if isinstance(value, bytes): + value = value.decode() + if isinstance(value, str): + value = value.strip() return value +def exists_in_backup(doctypes, file): + """Checks if the list of doctypes exist in the database.sql.gz file supplied + + Args: + doctypes (list): List of DocTypes to be checked + file (str): Path of the database file + + Returns: + bool: True if all tables exist + """ + with gzip.open(file, 'rb') as f: + content = f.read().decode("utf8") + return all(["CREATE TABLE `tab{}`".format(doctype).lower() in content.lower() for doctype in doctypes]) + class BaseTestCommands(unittest.TestCase): def execute(self, command, kwargs=None): site = {"site": frappe.local.site} @@ -25,7 +82,8 @@ class BaseTestCommands(unittest.TestCase): kwargs.update(site) else: kwargs = site - command = command.replace("\n", " ").format(**kwargs) + command = " ".join(command.split()).format(**kwargs) + print("{0}$ {1}{2}".format(color.silver, command, color.nc)) command = shlex.split(command) self._proc = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) self.stdout = clean(self._proc.stdout) @@ -54,6 +112,21 @@ class TestCommands(BaseTestCommands): self.assertEquals(self.stdout[1:-1], frappe.bold(text='DocType')) def test_backup(self): + backup = { + "includes": { + "includes": [ + "ToDo", + "Note", + ] + }, + "excludes": { + "excludes": [ + "Activity Log", + "Access Log", + "Error Log" + ] + } + } home = os.path.expanduser("~") site_backup_path = frappe.utils.get_site_path("private", "backups") @@ -119,3 +192,36 @@ class TestCommands(BaseTestCommands): # test 6: take a backup with --verbose self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) + + # test 7: take a backup with frappe.conf.backup.includes + self.execute("bench --site {site} set-config backup '{includes}' --as-dict", {"includes": json.dumps(backup["includes"])}) + self.execute("bench --site {site} backup --verbose") + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) + + # test 8: take a backup with frappe.conf.backup.excludes + self.execute("bench --site {site} set-config backup '{excludes}' --as-dict", {"excludes": json.dumps(backup["excludes"])}) + self.execute("bench --site {site} backup --verbose") + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) + self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) + + # test 9: take a backup with --include (with frappe.conf.excludes still set) + self.execute("bench --site {site} backup --include '{include}'", {"include": ",".join(backup["includes"]["includes"])}) + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) + + # test 10: take a backup with --exclude + self.execute("bench --site {site} backup --exclude '{exclude}'", {"exclude": ",".join(backup["excludes"]["excludes"])}) + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) + + # test 11: take a backup with --ignore-backup-conf + self.execute("bench --site {site} backup --ignore-backup-conf") + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 5b1f6b0616..43dd7c17f1 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -497,7 +497,7 @@ def scheduled_backup( odb = new_backup( older_than=older_than, ignore_files=ignore_files, - backup_path=None, + backup_path=backup_path, backup_path_db=backup_path_db, backup_path_files=backup_path_files, backup_path_private_files=backup_path_private_files, From ff1f1e755da30f028ff52e7b878a3ee48905e4ca Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 8 Oct 2020 15:45:15 +0530 Subject: [PATCH 10/38] fix: Dynamic summary spacing --- frappe/utils/backups.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 43dd7c17f1..1ae976807a 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -302,8 +302,12 @@ class BackupGenerator: backup_summary = self.get_summary() print("Backup Summary for {0} at {1}".format(frappe.local.site, now())) + title = max([len(x) for x in backup_summary]) + path = max([len(x["path"]) for x in backup_summary.values()]) + for _type, info in backup_summary.items(): - print("{0:8}: {1:85} {2}".format(_type.title(), info["path"], info["size"])) + template = "{{0:{0}}}: {{1:{1}}} {{2}}".format(title, path) + print(template.format(_type.title(), info["path"], info["size"])) def backup_files(self): import subprocess From 104186906f3ecf8abe130c35e34c4c4ba9b21e36 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 8 Oct 2020 16:23:14 +0530 Subject: [PATCH 11/38] feat: Show command execution summary in message format --- frappe/tests/test_commands.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 39d76aaba5..b63a886a2f 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -82,14 +82,25 @@ class BaseTestCommands(unittest.TestCase): kwargs.update(site) else: kwargs = site - command = " ".join(command.split()).format(**kwargs) - print("{0}$ {1}{2}".format(color.silver, command, color.nc)) - command = shlex.split(command) + self.command = " ".join(command.split()).format(**kwargs) + print("{0}$ {1}{2}".format(color.silver, self.command, color.nc)) + command = shlex.split(self.command) self._proc = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) self.stdout = clean(self._proc.stdout) self.stderr = clean(self._proc.stderr) self.returncode = clean(self._proc.returncode) + def _formatMessage(self, msg, standardMsg): + output = super(BaseTestCommands, self)._formatMessage(msg, standardMsg) + cmd_execution_summary = "\n".join([ + "-" * 70, + "Last Command Execution Summary:", + "Command: {}".format(self.command) if self.command else "", + "Standard Output: {}".format(self.stdout) if self.stdout else "", + "Standard Error: {}".format(self.stderr) if self.stderr else "", + "Return Code: {}".format(self.returncode) if self.returncode else "", + ]).strip() + return "{}\n\n{}".format(output, cmd_execution_summary) class TestCommands(BaseTestCommands): def test_execute(self): From 44a09191b38f13682ef6f8a0ce21058ce3c25756 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 8 Oct 2020 18:56:29 +0530 Subject: [PATCH 12/38] fix: Use public schema for postgres site tx @surajshetty3416 --- frappe/tests/test_commands.py | 9 +++++++-- frappe/utils/backups.py | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index b63a886a2f..e7cb9f5d40 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -71,9 +71,14 @@ def exists_in_backup(doctypes, file): Returns: bool: True if all tables exist """ - with gzip.open(file, 'rb') as f: + predicate = ( + 'CREATE TABLE public."tab{}"' + if frappe.conf.db_type == "postgres" + else "CREATE TABLE `tab{}`" + ) + with gzip.open(file, "rb") as f: content = f.read().decode("utf8") - return all(["CREATE TABLE `tab{}`".format(doctype).lower() in content.lower() for doctype in doctypes]) + return all([predicate.format(doctype).lower() in content.lower() for doctype in doctypes]) class BaseTestCommands(unittest.TestCase): def execute(self, command, kwargs=None): diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 1ae976807a..ab5783006c 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -353,7 +353,7 @@ class BackupGenerator: if self.db_type == "postgres": if self.backup_includes: args["include"] = " ".join( - ["--table='{0}'".format(table) for table in self.backup_includes] + ["--table='public.\"{0}\"'".format(table) for table in self.backup_includes] ) elif self.backup_excludes: args["exclude"] = " ".join( From 368a031da3b389bb65b859a3165cc9640bf56e0b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 9 Oct 2020 11:19:16 +0530 Subject: [PATCH 13/38] fix: Update postgres exlcude table data --- frappe/tests/test_commands.py | 2 +- frappe/utils/backups.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index e7cb9f5d40..19fd90e99c 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -72,7 +72,7 @@ def exists_in_backup(doctypes, file): bool: True if all tables exist """ predicate = ( - 'CREATE TABLE public."tab{}"' + 'COPY public."tab{}"' if frappe.conf.db_type == "postgres" else "CREATE TABLE `tab{}`" ) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index ab5783006c..552debbaa1 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -357,7 +357,7 @@ class BackupGenerator: ) elif self.backup_excludes: args["exclude"] = " ".join( - ["--exclude-table='{0}'".format(table) for table in self.backup_excludes] + ["--exclude-table-data='public.\"{0}\"'".format(table) for table in self.backup_excludes] ) cmd_string = ( From 044da8a5757be170cce29a7f986ef8a1af919fac Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 30 Oct 2020 14:04:03 +0530 Subject: [PATCH 14/38] fix: create auto_repeat field if docfield/custom field does not exist --- frappe/core/doctype/doctype/doctype.py | 3 ++- frappe/custom/doctype/customize_form/customize_form.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 8a9c130fbe..1d15de9ab5 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -570,7 +570,8 @@ class DocType(Document): def make_repeatable(self): """If allow_auto_repeat is set, add auto_repeat custom field.""" if self.allow_auto_repeat: - if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', 'dt': self.name}): + if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', 'dt': self.name}) and \ + not frappe.db.exists('DocField', {'fieldname': 'auto_repeat', 'parent': self.name}): insert_after = self.fields[len(self.fields) - 1].fieldname df = dict(fieldname='auto_repeat', label='Auto Repeat', fieldtype='Link', options='Auto Repeat', insert_after=insert_after, read_only=1, no_copy=1, print_hide=1) create_custom_field(self.name, df) diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 61ecdd88b9..f3e67bba7b 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -76,8 +76,8 @@ class CustomizeForm(Document): def create_auto_repeat_custom_field_if_requried(self, meta): if self.allow_auto_repeat: - if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', - 'dt': self.doc_type}): + if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', 'dt': self.doc_type}) and \ + not frappe.db.exists('DocField', {'fieldname': 'auto_repeat', 'parent': self.name}): insert_after = self.fields[len(self.fields) - 1].fieldname df = dict( fieldname='auto_repeat', From af1ed2f0bcd964fb60381fc802092b9466ce5a20 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 9 Nov 2020 18:50:03 +0530 Subject: [PATCH 15/38] refactor: Move _new_site and extract_sql_from_archive from frappe.commands.site module to frappe.installer --- frappe/commands/site.py | 73 ++----------------------- frappe/installer.py | 115 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 71 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 8af0b422ba..5305502b17 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -9,7 +9,7 @@ import click import frappe from frappe.commands import get_site, pass_context from frappe.exceptions import SiteNotSpecifiedError -from frappe.utils import get_site_path, touch_file +from frappe.installer import _new_site @click.command('new-site') @@ -42,57 +42,6 @@ def new_site(site, mariadb_root_username=None, mariadb_root_password=None, admin if len(frappe.utils.get_sites()) == 1: use(site) -def _new_site(db_name, site, mariadb_root_username=None, mariadb_root_password=None, - admin_password=None, verbose=False, install_apps=None, source_sql=None, force=False, - no_mariadb_socket=False, reinstall=False, db_password=None, db_type=None, db_host=None, - db_port=None, new_site=False): - """Install a new Frappe site""" - - if not force and os.path.exists(site): - print('Site {0} already exists'.format(site)) - sys.exit(1) - - if no_mariadb_socket and not db_type == "mariadb": - print('--no-mariadb-socket requires db_type to be set to mariadb.') - sys.exit(1) - - if not db_name: - import hashlib - db_name = '_' + hashlib.sha1(site.encode()).hexdigest()[:16] - - from frappe.commands.scheduler import _is_scheduler_enabled - from frappe.installer import install_db, make_site_dirs - from frappe.installer import install_app as _install_app - import frappe.utils.scheduler - - frappe.init(site=site) - - try: - - # enable scheduler post install? - enable_scheduler = _is_scheduler_enabled() - except Exception: - enable_scheduler = False - - make_site_dirs() - - installing = touch_file(get_site_path('locks', 'installing.lock')) - - install_db(root_login=mariadb_root_username, root_password=mariadb_root_password, db_name=db_name, - admin_password=admin_password, verbose=verbose, source_sql=source_sql, force=force, reinstall=reinstall, - db_password=db_password, db_type=db_type, db_host=db_host, db_port=db_port, no_mariadb_socket=no_mariadb_socket) - apps_to_install = ['frappe'] + (frappe.conf.get("install_apps") or []) + (list(install_apps) or []) - for app in apps_to_install: - _install_app(app, verbose=verbose, set_as_patched=not source_sql) - - os.remove(installing) - - frappe.utils.scheduler.toggle_scheduler(enable_scheduler) - frappe.db.commit() - - scheduler_status = "disabled" if frappe.utils.scheduler.is_scheduler_disabled() else "enabled" - print("*** Scheduler is", scheduler_status, "***") - @click.command('restore') @click.argument('sql-file-path') @@ -107,25 +56,9 @@ def _new_site(db_name, site, mariadb_root_username=None, mariadb_root_password=N @pass_context def restore(context, sql_file_path, mariadb_root_username=None, mariadb_root_password=None, db_name=None, verbose=None, install_app=None, admin_password=None, force=None, with_public_files=None, with_private_files=None): "Restore site database from an sql file" - from frappe.installer import extract_sql_gzip, extract_files, is_downgrade + from frappe.installer import extract_sql_from_archive, extract_files, is_downgrade force = context.force or force - - # Extract the gzip file if user has passed *.sql.gz file instead of *.sql file - if not os.path.exists(sql_file_path): - base_path = '..' - sql_file_path = os.path.join(base_path, sql_file_path) - if not os.path.exists(sql_file_path): - print('Invalid path {0}'.format(sql_file_path[3:])) - sys.exit(1) - elif sql_file_path.startswith(os.sep): - base_path = os.sep - else: - base_path = '.' - - if sql_file_path.endswith('sql.gz'): - decompressed_file_name = extract_sql_gzip(os.path.abspath(sql_file_path)) - else: - decompressed_file_name = sql_file_path + decompressed_file_name = extract_sql_from_archive(sql_file_path) site = get_site(context) frappe.init(site=site) diff --git a/frappe/installer.py b/frappe/installer.py index df767a3294..b1420e5d9f 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -3,8 +3,90 @@ import json import os -from frappe.defaults import _clear_cache +import sys + import frappe +from frappe.defaults import _clear_cache + + +def _new_site( + db_name, + site, + mariadb_root_username=None, + mariadb_root_password=None, + admin_password=None, + verbose=False, + install_apps=None, + source_sql=None, + force=False, + no_mariadb_socket=False, + reinstall=False, + db_password=None, + db_type=None, + db_host=None, + db_port=None, + new_site=False, +): + """Install a new Frappe site""" + + if not force and os.path.exists(site): + print("Site {0} already exists".format(site)) + sys.exit(1) + + if no_mariadb_socket and not db_type == "mariadb": + print("--no-mariadb-socket requires db_type to be set to mariadb.") + sys.exit(1) + + if not db_name: + import hashlib + db_name = "_" + hashlib.sha1(site.encode()).hexdigest()[:16] + + frappe.init(site=site) + + from frappe.commands.scheduler import _is_scheduler_enabled + from frappe.utils import get_site_path, scheduler, touch_file + + try: + # enable scheduler post install? + enable_scheduler = _is_scheduler_enabled() + except Exception: + enable_scheduler = False + + make_site_dirs() + + installing = touch_file(get_site_path("locks", "installing.lock")) + + install_db( + root_login=mariadb_root_username, + root_password=mariadb_root_password, + db_name=db_name, + admin_password=admin_password, + verbose=verbose, + source_sql=source_sql, + force=force, + reinstall=reinstall, + db_password=db_password, + db_type=db_type, + db_host=db_host, + db_port=db_port, + no_mariadb_socket=no_mariadb_socket, + ) + apps_to_install = ( + ["frappe"] + (frappe.conf.get("install_apps") or []) + (list(install_apps) or []) + ) + + for app in apps_to_install: + install_app(app, verbose=verbose, set_as_patched=not source_sql) + + os.remove(installing) + + scheduler.toggle_scheduler(enable_scheduler) + frappe.db.commit() + + scheduler_status = ( + "disabled" if frappe.utils.scheduler.is_scheduler_disabled() else "enabled" + ) + print("*** Scheduler is", scheduler_status, "***") def install_db(root_login="root", root_password=None, db_name=None, source_sql=None, @@ -331,6 +413,37 @@ 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 + + Returns: + str: Path of the decompressed SQL file + """ + # Extract the gzip file if user has passed *.sql.gz file instead of *.sql file + if not os.path.exists(sql_file_path): + base_path = '..' + sql_file_path = os.path.join(base_path, sql_file_path) + if not os.path.exists(sql_file_path): + print('Invalid path {0}'.format(sql_file_path[3:])) + sys.exit(1) + elif sql_file_path.startswith(os.sep): + base_path = os.sep + else: + base_path = '.' + + if sql_file_path.endswith('sql.gz'): + decompressed_file_name = extract_sql_gzip(os.path.abspath(sql_file_path)) + else: + decompressed_file_name = sql_file_path + + return decompressed_file_name + + def extract_sql_gzip(sql_gz_path): import subprocess From 7b1fa59a29ec59535976a53bd10528fe80f6e18c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 9 Nov 2020 18:51:33 +0530 Subject: [PATCH 16/38] feat: Restore partial backups via bench partial-restore --- frappe/commands/site.py | 17 ++++++++++++++++- frappe/database/db_manager.py | 1 - frappe/installer.py | 23 +++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 5305502b17..fd247b4182 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -91,6 +91,20 @@ def restore(context, sql_file_path, mariadb_root_username=None, mariadb_root_pas success_message = "Site {0} has been restored{1}".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') +@click.option("--verbose", "-v", is_flag=True) +@pass_context +def partial_restore(context, sql_file_path, verbose): + from frappe.installer import partial_restore + verbose = context.verbose or verbose + + site = get_site(context) + frappe.init(site=site) + frappe.connect(site=site) + partial_restore(sql_file_path, verbose) + frappe.destroy() + @click.command('reinstall') @click.option('--admin-password', help='Administrator Password for reinstalled site') @click.option('--mariadb-root-username', help='Root username for MariaDB') @@ -650,5 +664,6 @@ commands = [ stop_recording, add_to_hosts, start_ngrok, - build_search_index + build_search_index, + partial_restore ] diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index 3345fce735..da1f584f57 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -3,7 +3,6 @@ import frappe class DbManager: - def __init__(self, db): """ Pass root_conn here for access to all databases. diff --git a/frappe/installer.py b/frappe/installer.py index b1420e5d9f..2bbf0421e3 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -519,3 +519,26 @@ def is_downgrade(sql_file_path, verbose=False): print("Your site will be downgraded from Frappe {0} to {1}".format(current_version, backup_version)) return downgrade + + +def partial_restore(sql_file_path, verbose=False): + sql_file = extract_sql_from_archive(sql_file_path) + + if frappe.conf.db_type in (None, "mariadb"): + from frappe.database.mariadb.setup_db import import_db_from_sql + elif frappe.conf.db_type == "postgres": + from frappe.database.postgres.setup_db import import_db_from_sql + import warnings + from click import style + warn = style( + "Delete the tables you want to restore manually before attempting" + " partial restore operation for PostreSQL databases", + fg="yellow" + ) + warnings.warn(warn) + + import_db_from_sql(source_sql=sql_file, verbose=verbose) + + # Removing temporarily created file + if sql_file != sql_file_path: + os.remove(sql_file) From ce45343355a4f429e772c83636fe7f2c438345e1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 9 Nov 2020 18:53:14 +0530 Subject: [PATCH 17/38] feat: Add aliases for bench backup * --include is the same as --only, -i * --exclude is the same as -e --- 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 fd247b4182..646984dc8b 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -325,8 +325,8 @@ def use(site, sites_path='.'): @click.command('backup') @click.option('--with-files', default=False, is_flag=True, help="Take backup with files") -@click.option('--include', default="", type=str, help="Specify the DocTypes to backup seperated by commas") -@click.option('--exclude', default="", type=str, help="Specify the DocTypes to not backup seperated by commas") +@click.option('--include', '--only', '-i', default="", type=str, help="Specify the DocTypes to backup seperated by commas") +@click.option('--exclude', '-e', default="", type=str, help="Specify the DocTypes to not backup seperated by commas") @click.option('--backup-path', default=None, help="Set path for saving all the files in this operation") @click.option('--backup-path-db', default=None, help="Set path for saving database file") @click.option('--backup-path-files', default=None, help="Set path for saving public file") From b371c7005359e202c68adc4a2abd09030f5366e2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 11:11:19 +0530 Subject: [PATCH 18/38] refactor: Meaningful variable names --- frappe/database/db_manager.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index da1f584f57..b8ffae519b 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -65,10 +65,10 @@ class DbManager: esc = make_esc('$ ') from distutils.spawn import find_executable - pipe = find_executable('pv') - if pipe: - pipe = '{pipe} {source} |'.format( - pipe=pipe, + pv = find_executable('pv') + if pv: + pipe = '{pv} {source} |'.format( + pv=pv, source=source ) source = '' @@ -77,7 +77,7 @@ class DbManager: source = '< {source}'.format(source=source) if pipe: - print('Creating Database...') + print('Restoring Database file...') command = '{pipe} mysql -u {user} -p{password} -h{host} ' + ('-P{port}' if frappe.db.port else '') + ' {target} {source}' command = command.format( From e8fdaa195bf3fe276be58c455d4fc3bad56970a8 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 09:11:19 +0530 Subject: [PATCH 19/38] refactor: PostgreSQL module methods correspond to MariaDB * Added bootstrap_database, import_db_from_sql function APIs similar to MariaDB implementations --- frappe/database/postgres/setup_db.py | 29 +++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index f53872db82..6f2ba7a1b7 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -10,6 +10,23 @@ def setup_database(force, source_sql=None, verbose=False): root_conn.sql("CREATE user {0} password '{1}'".format(frappe.conf.db_name, frappe.conf.db_password)) root_conn.sql("GRANT ALL PRIVILEGES ON DATABASE `{0}` TO {0}".format(frappe.conf.db_name)) + root_conn.close() + + bootstrap_database(frappe.conf.db_name, verbose, source_sql=source_sql) + frappe.connect() + +def bootstrap_database(db_name, verbose, source_sql=None): + frappe.connect(db_name=db_name) + import_db_from_sql(source_sql, verbose) + frappe.connect(db_name=db_name) + if not 'tabDefaultValue' in frappe.db.get_tables(): + print('''Database not installed, this can due to lack of permission, or that the database name exists. + Check your mysql root password, or use --force to reinstall''') + sys.exit(1) + +def import_db_from_sql(source_sql=None, verbose=False): + if verbose: + print("Starting Database Import...") # we can't pass psql password in arguments in postgresql as mysql. So # set password connection parameter in environment variable @@ -19,15 +36,21 @@ def setup_database(force, source_sql=None, verbose=False): if not source_sql: source_sql = os.path.join(os.path.dirname(__file__), 'framework_postgres.sql') - subprocess.check_output([ + command = [ 'psql', frappe.conf.db_name, '-h', frappe.conf.db_host or 'localhost', '-p', str(frappe.conf.db_port or '5432'), '-U', frappe.conf.db_name, '-f', source_sql - ], env=subprocess_env) + ] - frappe.connect() + if verbose: + print(" ".join(command)) + + subprocess.check_output(command, env=subprocess_env) + + if verbose: + print(f"Imported from Database File: {source_sql}") def setup_help_database(help_db_name): root_conn = get_root_connection() From 19f87c36e51d57c2d7b96d20b43e919c7d4289e1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 9 Nov 2020 18:57:21 +0530 Subject: [PATCH 20/38] fix: Marked is_downgrade function as only MariaDB compatible --- frappe/installer.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 2bbf0421e3..b73f3f1d6e 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -118,9 +118,9 @@ def install_db(root_login="root", root_password=None, db_name=None, source_sql=N def install_app(name, verbose=False, set_as_patched=True): from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs - from frappe.utils.fixtures import sync_fixtures from frappe.model.sync import sync_for from frappe.modules.utils import sync_customizations + from frappe.utils.fixtures import sync_fixtures frappe.flags.in_install = name frappe.flags.ignore_in_install = False @@ -458,9 +458,10 @@ def extract_sql_gzip(sql_gz_path): return decompressed_file + def extract_files(site_name, file_path, folder_name): - import subprocess import shutil + import subprocess # Need to do frappe.init to maintain the site locals frappe.init(site=site_name) @@ -488,6 +489,12 @@ def extract_files(site_name, file_path, folder_name): def is_downgrade(sql_file_path, verbose=False): """checks if input db backup will get downgraded on current bench""" + + # This function is only tested with mariadb + # TODO: Add postgres support + if frappe.conf.db_type not in (None, "mariadb"): + return False + from semantic_version import Version head = "INSERT INTO `tabInstalled Application` VALUES" From 142b6009fe0d2b609a9f0c40a4404a7dfb16ac72 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 10:42:03 +0530 Subject: [PATCH 21/38] fix: Better error message on missing table --- frappe/database/mariadb/setup_db.py | 18 ++++++++++++++---- frappe/database/postgres/setup_db.py | 13 ++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/frappe/database/mariadb/setup_db.py b/frappe/database/mariadb/setup_db.py index a4e4d624ae..9b73d77171 100644 --- a/frappe/database/mariadb/setup_db.py +++ b/frappe/database/mariadb/setup_db.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals import frappe -import os, sys +import os from frappe.database.db_manager import DbManager expected_settings_10_2_earlier = { @@ -86,6 +86,8 @@ def drop_user_and_database(db_name, root_login, root_password): dbman.drop_database(db_name) def bootstrap_database(db_name, verbose, source_sql=None): + import sys + frappe.connect(db_name=db_name) if not check_database_settings(): print('Database settings do not match expected values; stopping database setup.') @@ -94,9 +96,17 @@ def bootstrap_database(db_name, verbose, source_sql=None): import_db_from_sql(source_sql, verbose) frappe.connect(db_name=db_name) - if not 'tabDefaultValue' in frappe.db.get_tables(): - print('''Database not installed, this can due to lack of permission, or that the database name exists. - Check your mysql root password, or use --force to reinstall''') + if 'tabDefaultValue' not in frappe.db.get_tables(): + from click import secho + + secho( + "Table 'tabDefaultValue' missing in the restored site. " + "Database not installed correctly, this can due to lack of " + "permission, or that the database name exists. Check your mysql" + " root password, validity of the backup file or use --force to" + " reinstall", + fg="red" + ) sys.exit(1) def import_db_from_sql(source_sql=None, verbose=False): diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 6f2ba7a1b7..109ed0469e 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -19,9 +19,16 @@ def bootstrap_database(db_name, verbose, source_sql=None): frappe.connect(db_name=db_name) import_db_from_sql(source_sql, verbose) frappe.connect(db_name=db_name) - if not 'tabDefaultValue' in frappe.db.get_tables(): - print('''Database not installed, this can due to lack of permission, or that the database name exists. - Check your mysql root password, or use --force to reinstall''') + if 'tabDefaultValue' not in frappe.db.get_tables(): + import sys + from click import secho + + secho( + "Table 'tabDefaultValue' missing in the restored site. " + "This may be due to incorrect permissions or the result of a restore from a bad backup file. " + "Database not installed correctly.", + fg="red" + ) sys.exit(1) def import_db_from_sql(source_sql=None, verbose=False): From 853acf6dd05176729ed033ec36658557ea31ce76 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 10:51:23 +0530 Subject: [PATCH 22/38] fix: Add "partial" tag in the backup file following site name to indicate its nature --- frappe/utils/backups.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 552debbaa1..7856c8d953 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -65,6 +65,7 @@ class BackupGenerator: self.ignore_conf = ignore_conf self.include_doctypes = include_doctypes self.exclude_doctypes = exclude_doctypes + self.partial = False if not self.db_type: self.db_type = "mariadb" @@ -144,6 +145,8 @@ class BackupGenerator: self.backup_includes = self.backup_includes or conf_tables["include"] self.backup_excludes = self.backup_excludes or conf_tables["exclude"] + self.partial = (self.backup_includes or self.backup_excludes) and not self.ignore_conf + @property def site_config_backup_path(self): # For backwards compatibility @@ -199,7 +202,10 @@ class BackupGenerator: self.backup_path_conf = site_config_backup_path def set_backup_file_name(self): - # Generate a random name using today's date and a 8 digit random number + if self.partial: + # slugs postfixed with partial won't get returned by get_recent_backup + self.site_slug = self.site_slug + "-partial" + for_conf = self.todays_date + "-" + self.site_slug + "-site_config_backup.json" for_db = self.todays_date + "-" + self.site_slug + "-database.sql.gz" ext = "tgz" if self.compress_files else "tar" From b4e17b9f95f65900b9c287d8a14985fba114065b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 10:52:10 +0530 Subject: [PATCH 23/38] fix: Give more information about file to match verbosity --- frappe/utils/backups.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 7856c8d953..f44bcd40e8 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -573,29 +573,29 @@ def delete_temp_backups(older_than=24): os.remove(this_file_path) -def is_file_old(db_file_name, older_than=24): +def is_file_old(file_path, older_than=24): """ Checks if file exists and is older than specified hours Returns -> True: file does not exist or file is old False: file is new """ - if os.path.isfile(db_file_name): + if os.path.isfile(file_path): from datetime import timedelta # Get timestamp of the file - file_datetime = datetime.fromtimestamp(os.stat(db_file_name).st_ctime) + file_datetime = datetime.fromtimestamp(os.stat(file_path).st_ctime) if datetime.today() - file_datetime >= timedelta(hours=older_than): if _verbose: - print("File is old") + print(f"File {file_path} is older than {older_than} hours") return True else: if _verbose: - print("File is recent") + print(f"File {file_path} is recent") return False else: if _verbose: - print("File does not exist") + print(f"File {file_path} does not exist") return True From a073a595448a2459e3d220e6406a46291f462daa Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 11 Nov 2020 09:11:19 +0530 Subject: [PATCH 24/38] fix: Add partial slug only for database file --- frappe/utils/backups.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index f44bcd40e8..e2fe4c559a 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -202,16 +202,14 @@ class BackupGenerator: self.backup_path_conf = site_config_backup_path def set_backup_file_name(self): - if self.partial: - # slugs postfixed with partial won't get returned by get_recent_backup - self.site_slug = self.site_slug + "-partial" - - for_conf = self.todays_date + "-" + self.site_slug + "-site_config_backup.json" - for_db = self.todays_date + "-" + self.site_slug + "-database.sql.gz" + # backups with the partial tag won't get returned by get_recent_backup + partial = "-partial" if self.partial else "" ext = "tgz" if self.compress_files else "tar" - for_public_files = self.todays_date + "-" + self.site_slug + "-files." + ext - for_private_files = self.todays_date + "-" + self.site_slug + "-private-files." + ext + for_conf = f"{self.todays_date}-{self.site_slug}-site_config_backup.json" + for_db = f"{self.todays_date}-{self.site_slug}{partial}-database.sql.gz" + for_public_files = f"{self.todays_date}-{self.site_slug}-files.{ext}" + for_private_files = f"{self.todays_date}-{self.site_slug}-private-files.{ext}" backup_path = self.backup_path or get_backup_path() if not self.backup_path_conf: From 3fed5c72553e2404123beb63ba10405cbf934536 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 13:11:34 +0530 Subject: [PATCH 25/38] test: Add tests for bench partial-restore --- frappe/tests/test_commands.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 283f9c01a7..f5cdd6b775 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -12,6 +12,7 @@ from glob import glob # imports - module imports import frappe +from frappe.utils import add_to_date, now from frappe.utils.backups import fetch_latest_backups import frappe.recorder @@ -243,6 +244,30 @@ class TestCommands(BaseTestCommands): database = fetch_latest_backups()["database"] self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) + def test_partial_restore(self): + _now = now() + for num in range(10): + frappe.get_doc({ + "doctype": "ToDo", + "date": add_to_date(_now, days=num), + "description": frappe.mock("paragraph") + }).insert() + todo_count = frappe.db.count("ToDo") + + # check if todos exist, create a partial backup and see if the state is the same after restore + self.assertIsNot(todo_count, 0) + self.execute("bench --site {site} backup --only 'ToDo'") + db_path = fetch_latest_backups(partial=True)["database"] + self.assertTrue("partial" in db_path) + + frappe.db.sql_ddl("DROP TABLE IF EXISTS `tabToDo`") + frappe.db.commit() + + self.execute("bench --site {site} partial-restore {path}", {"path": db_path}) + self.assertEquals(self.returncode, 0) + frappe.db.commit() + self.assertEquals(frappe.db.count("ToDo"), todo_count) + def test_recorder(self): frappe.recorder.stop() From 34af8cb32601993c069a862f1902aac53d848e69 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 12 Nov 2020 09:11:19 +0530 Subject: [PATCH 26/38] fix: Show partial backups when flag set * in fetch_latest_backups whitelisted API * BackupGenerator.get_recent_backup --- frappe/tests/test_commands.py | 8 ++++---- frappe/utils/backups.py | 9 ++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index f5cdd6b775..1063307b76 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -215,27 +215,27 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {site} set-config backup '{includes}' --as-dict", {"includes": json.dumps(backup["includes"])}) self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) - database = fetch_latest_backups()["database"] + database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 8: take a backup with frappe.conf.backup.excludes self.execute("bench --site {site} set-config backup '{excludes}' --as-dict", {"excludes": json.dumps(backup["excludes"])}) self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) - database = fetch_latest_backups()["database"] + database = fetch_latest_backups(partial=True)["database"] self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 9: take a backup with --include (with frappe.conf.excludes still set) self.execute("bench --site {site} backup --include '{include}'", {"include": ",".join(backup["includes"]["includes"])}) self.assertEquals(self.returncode, 0) - database = fetch_latest_backups()["database"] + database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 10: take a backup with --exclude self.execute("bench --site {site} backup --exclude '{exclude}'", {"exclude": ",".join(backup["excludes"]["excludes"])}) self.assertEquals(self.returncode, 0) - database = fetch_latest_backups()["database"] + database = fetch_latest_backups(partial=True)["database"] self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) # test 11: take a backup with --ignore-backup-conf diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index e2fe4c559a..178d213e59 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -202,7 +202,6 @@ class BackupGenerator: self.backup_path_conf = site_config_backup_path def set_backup_file_name(self): - # backups with the partial tag won't get returned by get_recent_backup partial = "-partial" if self.partial else "" ext = "tgz" if self.compress_files else "tar" @@ -221,11 +220,11 @@ class BackupGenerator: if not self.backup_path_private_files: self.backup_path_private_files = os.path.join(backup_path, for_private_files) - def get_recent_backup(self, older_than): + def get_recent_backup(self, older_than, partial=False): backup_path = get_backup_path() file_type_slugs = { - "database": "*-{}-database.sql.gz", + "database": "*-{{}}-{}database.sql.gz".format('*' if partial else ''), "public": "*-{}-files.tar", "private": "*-{}-private-files.tar", "config": "*-{}-site_config_backup.json", @@ -463,7 +462,7 @@ def get_backup(): @frappe.whitelist() -def fetch_latest_backups(): +def fetch_latest_backups(partial=False): """Fetches paths of the latest backup taken in the last 30 days Only for: System Managers @@ -479,7 +478,7 @@ def fetch_latest_backups(): db_type=frappe.conf.db_type, db_port=frappe.conf.db_port, ) - database, public, private, config = odb.get_recent_backup(older_than=24 * 30) + database, public, private, config = odb.get_recent_backup(older_than=24 * 30, partial=partial) return {"database": database, "public": public, "private": private, "config": config} From a22cd461ac85764477a39dc1e5ed613125b4e0bf Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 13:34:48 +0530 Subject: [PATCH 27/38] test: Commit after insert, not before count check --- frappe/tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 1063307b76..d19a62d788 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -252,6 +252,7 @@ class TestCommands(BaseTestCommands): "date": add_to_date(_now, days=num), "description": frappe.mock("paragraph") }).insert() + frappe.db.commit() todo_count = frappe.db.count("ToDo") # check if todos exist, create a partial backup and see if the state is the same after restore @@ -265,7 +266,6 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {site} partial-restore {path}", {"path": db_path}) self.assertEquals(self.returncode, 0) - frappe.db.commit() self.assertEquals(frappe.db.count("ToDo"), todo_count) def test_recorder(self): From 0f13fe7da5e7158830ab77896a76116e71b02b64 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 13 Nov 2020 15:13:55 +0530 Subject: [PATCH 28/38] fix: Validate SQL files properly --- frappe/installer.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 6745a92345..3edea185cf 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -415,23 +415,29 @@ def validate_database_sql(path, _raise=True): path (str): Path of the decompressed SQL file _raise (bool, optional): Raise exception if invalid file. Defaults to True. """ - to_raise = False + empty_file = False + missing_table = True + error_message = "" if not os.path.getsize(path): error_message = f"{path} is an empty file!" - to_raise = True + empty_file = True - if not _raise: + # dont bother checking if empty file + if not empty_file: with open(path, "r") as f: for line in f: if 'tabDefaultValue' in line: - error_message = "Table `tabDefaultValue` not found in file." - to_raise = True + missing_table = False + break + + if missing_table: + error_message = "Table `tabDefaultValue` not found in file." if error_message: import click click.secho(error_message, fg="red") - if _raise and to_raise: + if _raise and (missing_table or empty_file): raise frappe.InvalidDatabaseFile From 14fe6ccfa3e12afbe94ead781aa3179f79f20a2f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 13 Nov 2020 19:42:38 +0530 Subject: [PATCH 29/38] fix: Add is_partial check in bench restore --- frappe/commands/site.py | 35 ++++++++++++++++++++++++++++++++--- frappe/installer.py | 8 ++++++++ frappe/utils/backups.py | 1 + 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index c12afcb7ef..b55e7b7253 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -56,17 +56,41 @@ def new_site(site, mariadb_root_username=None, mariadb_root_password=None, admin @pass_context def restore(context, sql_file_path, mariadb_root_username=None, mariadb_root_password=None, db_name=None, verbose=None, install_app=None, admin_password=None, force=None, with_public_files=None, with_private_files=None): "Restore site database from an sql file" - from frappe.installer import extract_sql_from_archive, extract_files, is_downgrade, validate_database_sql + from frappe.installer import ( + extract_sql_from_archive, + extract_files, + is_downgrade, + is_partial, + validate_database_sql + ) + force = context.force or force decompressed_file_name = extract_sql_from_archive(sql_file_path) + # check if partial backup + if is_partial(decompressed_file_name): + 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) + + # check if valid SQL file validate_database_sql(decompressed_file_name, _raise=force) + site = get_site(context) frappe.init(site=site) # dont allow downgrading to older versions of frappe without force if not force and is_downgrade(decompressed_file_name, verbose=True): - warn_message = "This is not recommended and may lead to unexpected behaviour. Do you want to continue anyway?" + warn_message = ( + "This is not recommended and may lead to unexpected behaviour. " + "Do you want to continue anyway?" + ) click.confirm(warn_message, abort=True) _new_site(frappe.conf.db_name, site, mariadb_root_username=mariadb_root_username, @@ -89,9 +113,13 @@ def restore(context, sql_file_path, mariadb_root_username=None, mariadb_root_pas if decompressed_file_name != sql_file_path: os.remove(decompressed_file_name) - success_message = "Site {0} has been restored{1}".format(site, " with files" if (with_public_files or with_private_files) else "") + success_message = "Site {0} has been restored{1}".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') @click.option("--verbose", "-v", is_flag=True) @@ -106,6 +134,7 @@ def partial_restore(context, sql_file_path, verbose): partial_restore(sql_file_path, verbose) frappe.destroy() + @click.command('reinstall') @click.option('--admin-password', help='Administrator Password for reinstalled site') @click.option('--mariadb-root-username', help='Root username for MariaDB') diff --git a/frappe/installer.py b/frappe/installer.py index 2285d0011a..32a61f35fb 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -528,6 +528,14 @@ def is_downgrade(sql_file_path, verbose=False): 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 partial_restore(sql_file_path, verbose=False): sql_file = extract_sql_from_archive(sql_file_path) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 178d213e59..fe045be81f 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -6,6 +6,7 @@ import os from calendar import timegm from datetime import datetime from glob import glob +import gzip # imports - third party imports import click From 2fd5a82bbdd970089e209229eaa93304493c8ab5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 13 Nov 2020 19:43:07 +0530 Subject: [PATCH 30/38] fix: Add header content in database backups --- frappe/utils/backups.py | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index fe045be81f..b11c73c2b2 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -342,17 +342,36 @@ class BackupGenerator: def take_dump(self): import frappe.utils + from frappe.utils.change_log import get_app_branch + + database_header_content = [ + f"Backup generated by Frappe {frappe.__version__} on branch {get_app_branch('frappe') or 'N/A'}", + "", + ] # escape reserved characters - args = dict( + args = frappe._dict( [item[0], frappe.utils.esc(str(item[1]), "$ ")] for item in self.__dict__.copy().items() ) if self.backup_includes: - print("Backing Up Tables: {0}\n".format(", ".join(self.backup_includes))) + backup_info = ("Backing Up Tables: ", ", ".join(self.backup_includes)) elif self.backup_excludes: - print("Skipping Tables: {0}\n".format(", ".join(self.backup_excludes))) + backup_info = ("Skipping Tables: ", ", ".join(self.backup_excludes)) + + if self.partial: + print(''.join(backup_info), "\n") + database_header_content.extend([ + f"Partial Backup of Frappe Site {frappe.local.site}", + ("Backup contains: " if self.backup_includes else "Backup excludes: ") + backup_info[1], + "", + ]) + + generated_header = "\n".join([f"-- {x}" for x in database_header_content]) + "\n" + + with gzip.open(args.backup_path_db, "wt") as f: + f.write(generated_header) if self.db_type == "postgres": if self.backup_includes: @@ -366,7 +385,7 @@ class BackupGenerator: cmd_string = ( "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name}" - " {include} {exclude} | gzip > {backup_path_db}" + " {include} {exclude} | gzip >> {backup_path_db}" ) else: @@ -383,16 +402,16 @@ class BackupGenerator: cmd_string = ( "mysqldump --single-transaction --quick --lock-tables=false -u {user}" " -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude}" - " | gzip > {backup_path_db}" + " | gzip >> {backup_path_db}" ) command = cmd_string.format( - user=args.get("user"), - password=args.get("password"), - db_host=args.get("db_host"), - db_port=args.get("db_port"), - db_name=args.get("db_name"), - backup_path_db=args.get("backup_path_db"), + user=args.user, + password=args.password, + db_host=args.db_host, + db_port=args.db_port, + db_name=args.db_name, + backup_path_db=args.backup_path_db, exclude=args.get("exclude", ""), include=args.get("include", ""), ) From 22b752ac21c69cf2f50df9bfae472c1dacf0128a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Sat, 14 Nov 2020 11:11:19 +0530 Subject: [PATCH 31/38] test: Add tests for bench restore --- frappe/tests/test_commands.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 3f19abdae4..422dbf2930 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -201,9 +201,7 @@ class TestCommands(BaseTestCommands): # test 5: take a backup with --compress self.execute("bench --site {site} backup --with-files --compress") - self.assertEquals(self.returncode, 0) - compressed_files = glob(site_backup_path + "/*.tgz") self.assertGreater(len(compressed_files), 0) @@ -244,6 +242,18 @@ class TestCommands(BaseTestCommands): database = fetch_latest_backups()["database"] self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) + def test_restore(self): + # test 1: bench restore from full backup + self.execute("bench --site {site} backup --ignore-backup-conf") + database = fetch_latest_backups()["database"] + self.execute("bench --site {site} restore {database}", {"database": database}) + + # test 2: restore from partial backup + self.execute("bench --site {site} backup --exclude 'ToDo") + database = fetch_latest_backups(partial=True)["database"] + self.execute("bench --site {site} restore {database}", {"database": database}) + self.assertEquals(self.returncode, 1) + def test_partial_restore(self): _now = now() for num in range(10): From c4e57926f9899a3c0d1a0922aea651f66a3032a9 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 12:33:21 +0530 Subject: [PATCH 32/38] feat(postgres): Show restore progress if pv is available --- frappe/database/postgres/setup_db.py | 50 +++++++++++++++++----------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 109ed0469e..bf344c3aa5 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -1,5 +1,7 @@ -import frappe, subprocess, os -from six.moves import input +import os + +import frappe + def setup_database(force, source_sql=None, verbose=False): root_conn = get_root_connection() @@ -19,6 +21,7 @@ def bootstrap_database(db_name, verbose, source_sql=None): frappe.connect(db_name=db_name) import_db_from_sql(source_sql, verbose) frappe.connect(db_name=db_name) + if 'tabDefaultValue' not in frappe.db.get_tables(): import sys from click import secho @@ -32,32 +35,40 @@ def bootstrap_database(db_name, verbose, source_sql=None): sys.exit(1) def import_db_from_sql(source_sql=None, verbose=False): - if verbose: - print("Starting Database Import...") + from shlex import split + from shutil import which + from subprocess import run, PIPE # we can't pass psql password in arguments in postgresql as mysql. So # set password connection parameter in environment variable subprocess_env = os.environ.copy() subprocess_env['PGPASSWORD'] = str(frappe.conf.db_password) + # bootstrap db if not source_sql: source_sql = os.path.join(os.path.dirname(__file__), 'framework_postgres.sql') - command = [ - 'psql', frappe.conf.db_name, - '-h', frappe.conf.db_host or 'localhost', - '-p', str(frappe.conf.db_port or '5432'), - '-U', frappe.conf.db_name, - '-f', source_sql - ] + pv = which('pv') + + _command = ( + f"psql {frappe.conf.db_name} " + f"-h {frappe.conf.db_host or 'localhost'} -p {str(frappe.conf.db_port or '5432')} " + f"-U {frappe.conf.db_name}" + ) + + if pv: + command = f"{pv} {source_sql} | " + _command + else: + command = _command + f"-f {source_sql}" + + print("Restoring Database file...") + if verbose: + print(command) + + restore_proc = run(command, env=subprocess_env, shell=True, stdout=PIPE) if verbose: - print(" ".join(command)) - - subprocess.check_output(command, env=subprocess_env) - - if verbose: - print(f"Imported from Database File: {source_sql}") + print(f"\nSTDOUT by psql:\n{restore_proc.stdout.decode()}\nImported from Database File: {source_sql}") def setup_help_database(help_db_name): root_conn = get_root_connection() @@ -68,19 +79,20 @@ def setup_help_database(help_db_name): root_conn.sql("GRANT ALL PRIVILEGES ON DATABASE `{0}` TO {0}".format(help_db_name)) def get_root_connection(root_login=None, root_password=None): - import getpass if not frappe.local.flags.root_connection: if not root_login: root_login = frappe.conf.get("root_login") or None if not root_login: + from six.moves import input root_login = input("Enter postgres super user: ") if not root_password: root_password = frappe.conf.get("root_password") or None if not root_password: - root_password = getpass.getpass("Postgres super user password: ") + from getpass import getpass + root_password = getpass("Postgres super user password: ") frappe.local.flags.root_connection = frappe.database.get_db(user=root_login, password=root_password) From 84e2fcd6a8b7bdc0ec6f6eaabe1b4b6d7582cf3c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 12:33:59 +0530 Subject: [PATCH 33/38] fix: Add missing quote --- frappe/tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 422dbf2930..3c5fddeef0 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -249,7 +249,7 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {site} restore {database}", {"database": database}) # test 2: restore from partial backup - self.execute("bench --site {site} backup --exclude 'ToDo") + self.execute("bench --site {site} backup --exclude 'ToDo'") database = fetch_latest_backups(partial=True)["database"] self.execute("bench --site {site} restore {database}", {"database": database}) self.assertEquals(self.returncode, 1) From 3b5ec6aa88fa94a83aa00358fb2f85f7e72e0ab5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 12:45:55 +0530 Subject: [PATCH 34/38] fix: Sider + improper command ending --- frappe/database/postgres/setup_db.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index bf344c3aa5..3ee6b6a286 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -35,7 +35,6 @@ def bootstrap_database(db_name, verbose, source_sql=None): sys.exit(1) def import_db_from_sql(source_sql=None, verbose=False): - from shlex import split from shutil import which from subprocess import run, PIPE @@ -59,7 +58,7 @@ def import_db_from_sql(source_sql=None, verbose=False): if pv: command = f"{pv} {source_sql} | " + _command else: - command = _command + f"-f {source_sql}" + command = _command + f" -f {source_sql}" print("Restoring Database file...") if verbose: From 291179904615480bcaaa623e9b2bc055188b8ed9 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 15:14:23 +0530 Subject: [PATCH 35/38] fix: Re-write restore tests to process another site --- frappe/tests/test_commands.py | 36 +++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 3c5fddeef0..e282f1027d 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -243,15 +243,39 @@ class TestCommands(BaseTestCommands): self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) def test_restore(self): + # step 0: create a site to run the test on + global_config = { + "admin_password": frappe.conf.admin_password, + "root_login": frappe.conf.root_login, + "root_password": frappe.conf.root_password, + "db_type": frappe.conf.db_type + } + site_data = { + "another_site": f"{frappe.local.site}-restore.test", + **global_config + } + for key, value in global_config.items(): + if value: + self.execute(f"bench set-config {key} {value} -g") + self.execute("bench new-site {another_site} --admin-password {admin_password} --db-type {db_type} --force", site_data) + # test 1: bench restore from full backup - self.execute("bench --site {site} backup --ignore-backup-conf") - database = fetch_latest_backups()["database"] - self.execute("bench --site {site} restore {database}", {"database": database}) + self.execute("bench --site {another_site} backup --ignore-backup-conf", site_data) + self.execute("bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups", site_data) + site_data.update({"database": json.loads(self.stdout)["database"]}) + self.execute("bench --site {another_site} restore {database}", site_data) # test 2: restore from partial backup - self.execute("bench --site {site} backup --exclude 'ToDo'") - database = fetch_latest_backups(partial=True)["database"] - self.execute("bench --site {site} restore {database}", {"database": database}) + self.execute("bench --site {another_site} backup --exclude 'ToDo'", site_data) + site_data.update({"kw": "\"{'partial':True}\""}) + self.execute( + "bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups --kwargs {kw}", + site_data + ) + site_data.update({ + "database": json.loads(self.stdout)["database"] + }) + self.execute("bench --site {another_site} restore {database}", site_data) self.assertEquals(self.returncode, 1) def test_partial_restore(self): From f1cd3388ba0b71a52cf87bd7e1af16f4e834b35f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 16:28:15 +0530 Subject: [PATCH 36/38] style: Black-ish + fixed typos + Optimized imports --- frappe/build.py | 2 +- .../frappe_providers/frappecloud.py | 2 +- frappe/tests/test_commands.py | 83 ++++++++++++------- 3 files changed, 53 insertions(+), 34 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index f14b250a92..f47a7cb32b 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -105,7 +105,7 @@ def download_frappe_assets(verbose=True): if frappe_head: try: url = get_assets_link(frappe_head) - click.secho("Retreiving assets...", fg="yellow") + click.secho("Retrieving assets...", fg="yellow") prefix = mkdtemp(prefix="frappe-assets-", suffix=frappe_head) assets_archive = download_file(url, prefix) print("\n{0} Downloaded Frappe assets from {1}".format(green('✔'), url)) diff --git a/frappe/integrations/frappe_providers/frappecloud.py b/frappe/integrations/frappe_providers/frappecloud.py index e09f09a44b..f60344ee8f 100644 --- a/frappe/integrations/frappe_providers/frappecloud.py +++ b/frappe/integrations/frappe_providers/frappecloud.py @@ -6,7 +6,7 @@ import frappe def frappecloud_migrator(local_site): - print("Retreiving Site Migrator...") + print("Retrieving Site Migrator...") remote_site = frappe.conf.frappecloud_url or "frappecloud.com" request_url = "https://{}/api/method/press.api.script".format(remote_site) request = requests.get(request_url) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index e282f1027d..8c76ce2f48 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -1,24 +1,24 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors # imports - standard imports +import gzip import json import os import shlex import subprocess import sys import unittest -import gzip -from glob import glob +import glob # imports - module imports import frappe +import frappe.recorder +from frappe.installer import add_to_installed_apps from frappe.utils import add_to_date, now from frappe.utils.backups import fetch_latest_backups -import frappe.recorder # TODO: check frappe.cli.coloured_output to set coloured output! - def supports_color(): """ Returns True if the running system's terminal supports color, and False @@ -32,12 +32,12 @@ def supports_color(): class color(dict): - nc = '\033[0m' - blue = '\033[94m' - green = '\033[92m' - yellow = '\033[93m' - red = '\033[91m' - silver = '\033[90m' + nc = "\033[0m" + blue = "\033[94m" + green = "\033[92m" + yellow = "\033[93m" + red = "\033[91m" + silver = "\033[90m" def __getattr__(self, key): if supports_color(): @@ -82,6 +82,7 @@ def exists_in_backup(doctypes, file): content = f.read().decode("utf8") return all([predicate.format(doctype).lower() in content.lower() for doctype in doctypes]) + class BaseTestCommands(unittest.TestCase): def execute(self, command, kwargs=None): site = {"site": frappe.local.site} @@ -109,6 +110,7 @@ class BaseTestCommands(unittest.TestCase): ]).strip() return "{}\n\n{}".format(output, cmd_execution_summary) + class TestCommands(BaseTestCommands): def test_execute(self): # test 1: execute a command expecting a numeric output @@ -127,7 +129,7 @@ class TestCommands(BaseTestCommands): # The returned value has quotes which have been trimmed for the test self.execute("""bench --site {site} execute frappe.bold --kwargs '{{"text": "DocType"}}'""") self.assertEquals(self.returncode, 0) - self.assertEquals(self.stdout[1:-1], frappe.bold(text='DocType')) + self.assertEquals(self.stdout[1:-1], frappe.bold(text="DocType")) def test_backup(self): backup = { @@ -184,16 +186,19 @@ class TestCommands(BaseTestCommands): "db_path": "database.sql.gz", "files_path": "public.tar", "private_path": "private.tar", - "conf_path": "config.json" + "conf_path": "config.json", }.items() } - self.execute("""bench + self.execute( + """bench --site {site} backup --with-files --backup-path-db {db_path} --backup-path-files {files_path} --backup-path-private-files {private_path} - --backup-path-conf {conf_path}""", kwargs) + --backup-path-conf {conf_path}""", + kwargs, + ) self.assertEquals(self.returncode, 0) for path in kwargs.values(): @@ -202,7 +207,7 @@ class TestCommands(BaseTestCommands): # test 5: take a backup with --compress self.execute("bench --site {site} backup --with-files --compress") self.assertEquals(self.returncode, 0) - compressed_files = glob(site_backup_path + "/*.tgz") + compressed_files = glob.glob(site_backup_path + "/*.tgz") self.assertGreater(len(compressed_files), 0) # test 6: take a backup with --verbose @@ -210,14 +215,20 @@ class TestCommands(BaseTestCommands): self.assertEquals(self.returncode, 0) # test 7: take a backup with frappe.conf.backup.includes - self.execute("bench --site {site} set-config backup '{includes}' --as-dict", {"includes": json.dumps(backup["includes"])}) + self.execute( + "bench --site {site} set-config backup '{includes}' --as-dict", + {"includes": json.dumps(backup["includes"])}, + ) self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 8: take a backup with frappe.conf.backup.excludes - self.execute("bench --site {site} set-config backup '{excludes}' --as-dict", {"excludes": json.dumps(backup["excludes"])}) + self.execute( + "bench --site {site} set-config backup '{excludes}' --as-dict", + {"excludes": json.dumps(backup["excludes"])}, + ) self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] @@ -225,13 +236,19 @@ class TestCommands(BaseTestCommands): self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 9: take a backup with --include (with frappe.conf.excludes still set) - self.execute("bench --site {site} backup --include '{include}'", {"include": ",".join(backup["includes"]["includes"])}) + self.execute( + "bench --site {site} backup --include '{include}'", + {"include": ",".join(backup["includes"]["includes"])}, + ) self.assertEquals(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 10: take a backup with --exclude - self.execute("bench --site {site} backup --exclude '{exclude}'", {"exclude": ",".join(backup["excludes"]["excludes"])}) + self.execute( + "bench --site {site} backup --exclude '{exclude}'", + {"exclude": ",".join(backup["excludes"]["excludes"])}, + ) self.assertEquals(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) @@ -248,20 +265,24 @@ class TestCommands(BaseTestCommands): "admin_password": frappe.conf.admin_password, "root_login": frappe.conf.root_login, "root_password": frappe.conf.root_password, - "db_type": frappe.conf.db_type - } - site_data = { - "another_site": f"{frappe.local.site}-restore.test", - **global_config + "db_type": frappe.conf.db_type, } + site_data = {"another_site": f"{frappe.local.site}-restore.test", **global_config} for key, value in global_config.items(): if value: self.execute(f"bench set-config {key} {value} -g") - self.execute("bench new-site {another_site} --admin-password {admin_password} --db-type {db_type} --force", site_data) + self.execute( + "bench new-site {another_site} --admin-password {admin_password} --db-type" + " {db_type}", + site_data, + ) # test 1: bench restore from full backup self.execute("bench --site {another_site} backup --ignore-backup-conf", site_data) - self.execute("bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups", site_data) + self.execute( + "bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups", + site_data, + ) site_data.update({"database": json.loads(self.stdout)["database"]}) self.execute("bench --site {another_site} restore {database}", site_data) @@ -269,12 +290,11 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {another_site} backup --exclude 'ToDo'", site_data) site_data.update({"kw": "\"{'partial':True}\""}) self.execute( - "bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups --kwargs {kw}", - site_data + "bench --site {another_site} execute" + " frappe.utils.backups.fetch_latest_backups --kwargs {kw}", + site_data, ) - site_data.update({ - "database": json.loads(self.stdout)["database"] - }) + site_data.update({"database": json.loads(self.stdout)["database"]}) self.execute("bench --site {another_site} restore {database}", site_data) self.assertEquals(self.returncode, 1) @@ -314,7 +334,6 @@ class TestCommands(BaseTestCommands): self.assertEqual(frappe.recorder.status(), False) def test_remove_from_installed_apps(self): - from frappe.installer import add_to_installed_apps app = "test_remove_app" add_to_installed_apps(app) From 39da1e0363b0d15d4f2c9eb779ea3eb093d960fc Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 18 Nov 2020 01:31:18 +0530 Subject: [PATCH 37/38] fix: Raise error if executable not found in PATH --- frappe/exceptions.py | 3 ++- frappe/utils/backups.py | 29 +++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/frappe/exceptions.py b/frappe/exceptions.py index 267f5410af..9b63b23787 100644 --- a/frappe/exceptions.py +++ b/frappe/exceptions.py @@ -110,4 +110,5 @@ class DocumentAlreadyRestored(Exception): pass class InvalidAuthorizationHeader(CSRFTokenError): pass class InvalidAuthorizationPrefix(CSRFTokenError): pass class InvalidAuthorizationToken(CSRFTokenError): pass -class InvalidDatabaseFile(ValidationError): pass \ No newline at end of file +class InvalidDatabaseFile(ValidationError): pass +class ExecutableNotFound(FileNotFoundError): pass diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index b11c73c2b2..3ae300d3c4 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -2,11 +2,12 @@ # MIT License. See license.txt # imports - standard imports +import gzip import os from calendar import timegm from datetime import datetime from glob import glob -import gzip +from shutil import which # imports - third party imports import click @@ -14,7 +15,7 @@ import click # imports - module imports import frappe from frappe import _, conf -from frappe.utils import get_url, now, now_datetime, get_file_size +from frappe.utils import get_file_size, get_url, now, now_datetime # backup variable for backwards compatibility verbose = False @@ -344,6 +345,20 @@ class BackupGenerator: import frappe.utils from frappe.utils.change_log import get_app_branch + db_exc = { + "mariadb": ("mysqldump", which("mysqldump")), + "postgres": ("pg_dump", which("pg_dump")), + }[self.db_type] + gzip_exc = which("gzip") + + if not (gzip_exc and db_exc[1]): + _exc = "gzip" if not gzip_exc else db_exc[0] + frappe.throw( + f"{_exc} not found in PATH! This is required to take a backup.", + exc=frappe.ExecutableNotFound + ) + db_exc = db_exc[0] + database_header_content = [ f"Backup generated by Frappe {frappe.__version__} on branch {get_app_branch('frappe') or 'N/A'}", "", @@ -384,8 +399,8 @@ class BackupGenerator: ) cmd_string = ( - "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name}" - " {include} {exclude} | gzip >> {backup_path_db}" + "{db_exc} postgres://{user}:{password}@{db_host}:{db_port}/{db_name}" + " {include} {exclude} | {gzip} >> {backup_path_db}" ) else: @@ -400,20 +415,22 @@ class BackupGenerator: ) cmd_string = ( - "mysqldump --single-transaction --quick --lock-tables=false -u {user}" + "{db_exc} --single-transaction --quick --lock-tables=false -u {user}" " -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude}" - " | gzip >> {backup_path_db}" + " | {gzip} >> {backup_path_db}" ) command = cmd_string.format( user=args.user, password=args.password, + db_exc=db_exc, db_host=args.db_host, db_port=args.db_port, db_name=args.db_name, backup_path_db=args.backup_path_db, exclude=args.get("exclude", ""), include=args.get("include", ""), + gzip=gzip_exc, ) if self.verbose: From f37789280b9f75fffc7e392da28da00c635a9ab9 Mon Sep 17 00:00:00 2001 From: Himanshu Date: Wed, 18 Nov 2020 10:47:33 +0530 Subject: [PATCH 38/38] fix(user defaults): Set user selected timezone in user defaults (#11902) Co-authored-by: Sahil Khan Co-authored-by: Sahil Khan --- frappe/core/doctype/user/user.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 2c5865fb69..7309528da6 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -13,7 +13,7 @@ from frappe.utils.user import get_system_managers from bs4 import BeautifulSoup import frappe.permissions import frappe.share - +import frappe.defaults from frappe.website.utils import is_signup_enabled from frappe.utils.background_jobs import enqueue @@ -107,6 +107,10 @@ class User(Document): ) if self.name not in ('Administrator', 'Guest') and not self.user_image: frappe.enqueue('frappe.core.doctype.user.user.update_gravatar', name=self.name) + + # Set user selected timezone + if self.time_zone: + frappe.defaults.set_default("time_zone", self.time_zone, self.name) def has_website_permission(self, ptype, user, verbose=False): """Returns true if current user is the session user""" @@ -1129,4 +1133,4 @@ def check_password_reset_limit(user, rate_limit): frappe.throw(_("You have reached the hourly limit for generating password reset links. Please try again later.")) def get_generated_link_count(user): - return cint(frappe.cache().hget("password_reset_link_count", user)) or 0 \ No newline at end of file + return cint(frappe.cache().hget("password_reset_link_count", user)) or 0