From a926e64ec9956e2f6b9a32c5e29d9c36d230d338 Mon Sep 17 00:00:00 2001 From: David Arnold Date: Mon, 24 Jul 2023 00:18:29 -0500 Subject: [PATCH] fix: procure db config from single authority (#21578) * fix: procure db config from single authority ensures that configuration is uniformely procured from local.conf instead of making use of hard to audit multilevel fallback logic Implementation Note: - `get_db(host, port, user, password)` was stripped of any optional argument and therefrom all errors where fixed. - All occurances of `grep 'frappe.db.db_'` where changed to `frappe.conf.db_` * fix: revert unnecessary breaking changes --- frappe/__init__.py | 7 ++++++- frappe/database/db_manager.py | 4 ++-- frappe/database/mariadb/setup_db.py | 5 ++++- frappe/database/postgres/setup_db.py | 5 ++++- frappe/integrations/offsite_backup_utils.py | 8 ++++---- frappe/tests/test_commands.py | 4 ++-- frappe/utils/backups.py | 19 +++++++------------ 7 files changed, 29 insertions(+), 23 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 44e1a24137..b3a5c54fa5 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -270,7 +270,12 @@ def connect( if site: init(site) - local.db = get_db(user=db_name or local.conf.db_name) + local.db = get_db( + host=local.conf.db_host, + port=local.conf.db_port, + user=db_name or local.conf.db_name, + password=None, + ) if set_admin_as_user: set_user("Administrator") diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index 381abdc3d4..a1761b5995 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -73,9 +73,9 @@ class DbManager: pipe=pipe, user=esc(user), password=esc(password), - host=esc(frappe.db.host), + host=esc(frappe.conf.db_host), target=esc(target), source=source, - port=frappe.db.port, + port=frappe.conf.db_port, ) os.system(command) diff --git a/frappe/database/mariadb/setup_db.py b/frappe/database/mariadb/setup_db.py index add7fa373f..87a7f6d343 100644 --- a/frappe/database/mariadb/setup_db.py +++ b/frappe/database/mariadb/setup_db.py @@ -166,7 +166,10 @@ def get_root_connection(root_login, root_password): root_password = getpass.getpass("MySQL root password: ") frappe.local.flags.root_connection = frappe.database.get_db( - user=root_login, password=root_password + host=frappe.conf.db_host, + port=frappe.conf.db_port, + user=root_login, + password=root_password, ) return frappe.local.flags.root_connection diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 200bae892f..9d1d045b04 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -92,7 +92,10 @@ def get_root_connection(root_login=None, root_password=None): root_password = getpass("Postgres super user password: ") frappe.local.flags.root_connection = frappe.database.get_db( - user=root_login, password=root_password + host=frappe.conf.db_host, + port=frappe.conf.db_port, + user=root_login, + password=root_password, ) return frappe.local.flags.root_connection diff --git a/frappe/integrations/offsite_backup_utils.py b/frappe/integrations/offsite_backup_utils.py index 620f692ad0..0a5cd80ea6 100644 --- a/frappe/integrations/offsite_backup_utils.py +++ b/frappe/integrations/offsite_backup_utils.py @@ -54,9 +54,9 @@ def get_latest_backup_file(with_files=False): frappe.conf.db_name, frappe.conf.db_name, frappe.conf.db_password, - db_host=frappe.db.host, - db_type=frappe.conf.db_type, + db_host=frappe.conf.db_host, db_port=frappe.conf.db_port, + db_type=frappe.conf.db_type, ) database, public, private, config = odb.get_recent_backup(older_than=24 * 30) @@ -112,9 +112,9 @@ def generate_files_backup(): frappe.conf.db_name, frappe.conf.db_name, frappe.conf.db_password, - db_host=frappe.db.host, - db_type=frappe.conf.db_type, + db_host=frappe.conf.db_host, db_port=frappe.conf.db_port, + db_type=frappe.conf.db_type, ) backup.set_backup_file_name() diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index ed31e945e5..e090d860d7 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -535,8 +535,8 @@ class TestBackups(BaseTestCommands): frappe.conf.db_name, frappe.conf.db_name, frappe.conf.db_password + "INCORRECT PASSWORD", - db_host=frappe.db.host, - db_port=frappe.db.port, + db_host=frappe.conf.db_host, + db_port=frappe.conf.db_port, db_type=frappe.conf.db_type, ) with self.assertRaises(Exception): diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 10e7cbc1a5..30a7353758 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -44,9 +44,9 @@ class BackupGenerator: backup_path_db=None, backup_path_files=None, backup_path_private_files=None, - db_host="localhost", + db_host=None, db_port=None, - db_type="mariadb", + db_type=None, backup_path_conf=None, ignore_conf=False, compress_files=False, @@ -72,11 +72,6 @@ class BackupGenerator: self.exclude_doctypes = exclude_doctypes self.partial = False - if not self.db_type: - self.db_type = "mariadb" - - self.db_port = self.db_port or frappe.db.default_port - site = frappe.local.site or frappe.generate_hash(length=8) self.site_slug = site.replace(".", "_") self.verbose = verbose @@ -430,7 +425,7 @@ class BackupGenerator: args["include"] = " ".join([f"'{x}'" for x in self.backup_includes]) elif self.backup_excludes: args["exclude"] = " ".join( - [f"--ignore-table='{frappe.conf.db_name}.{table}'" for table in self.backup_excludes] + [f"--ignore-table='{self.db_name}.{table}'" for table in self.backup_excludes] ) cmd_string = ( @@ -502,9 +497,9 @@ def fetch_latest_backups(partial=False): frappe.conf.db_name, frappe.conf.db_name, frappe.conf.db_password, - db_host=frappe.db.host, - db_type=frappe.conf.db_type, + db_host=frappe.conf.db_host, db_port=frappe.conf.db_port, + db_type=frappe.conf.db_type, ) database, public, private, config = odb.get_recent_backup(older_than=24 * 30, partial=partial) @@ -567,8 +562,8 @@ def new_backup( frappe.conf.db_name, frappe.conf.db_name, frappe.conf.db_password, - db_host=frappe.db.host, - db_port=frappe.db.port, + db_host=frappe.conf.db_host, + db_port=frappe.conf.db_port, db_type=frappe.conf.db_type, backup_path=backup_path, backup_path_db=backup_path_db,