From 94378b6566e4dd738ebcb9b7f3718c130ccb0e61 Mon Sep 17 00:00:00 2001 From: David Arnold Date: Wed, 17 Jan 2024 16:55:48 +0100 Subject: [PATCH 1/8] chore: simplify frappe.connect and use site config for bootstrapping --- frappe/__init__.py | 9 ++++++++- frappe/database/__init__.py | 6 +++--- frappe/database/mariadb/setup_db.py | 6 +++--- frappe/database/postgres/setup_db.py | 9 +++++---- frappe/installer.py | 1 - 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 289e5c6e3d..e930b8d500 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -280,7 +280,7 @@ def connect( """Connect to site database instance. :param site: (Deprecated) If site is given, calls `frappe.init`. - :param db_name: Optional. Will use from `site_config.json`. + :param db_name: (Deprecated) Optional. Will use from `site_config.json`. :param set_admin_as_user: Set Administrator as current user. """ from frappe.database import get_db @@ -293,6 +293,13 @@ def connect( "Instead, explicitly invoke frappe.init(site) prior to calling frappe.connect(), if initializing the site is necessary." ) init(site) + if db_name: + from frappe.utils.deprecations import deprecation_warning + + deprecation_warning( + "Calling frappe.connect with the db_name argument is deprecated and will be removed in next major version. " + "Instead, explicitly invoke frappe.init(site) with the right config prior to calling frappe.connect(), if necessary." + ) local.db = get_db( host=local.conf.db_host, diff --git a/frappe/database/__init__.py b/frappe/database/__init__.py index 530bd4c700..93241d6793 100644 --- a/frappe/database/__init__.py +++ b/frappe/database/__init__.py @@ -23,17 +23,17 @@ def setup_database(force, verbose=None, no_mariadb_socket=False): ) -def bootstrap_database(db_name, verbose=None, source_sql=None): +def bootstrap_database(verbose=None, source_sql=None): import frappe if frappe.conf.db_type == "postgres": import frappe.database.postgres.setup_db - return frappe.database.postgres.setup_db.bootstrap_database(db_name, verbose, source_sql) + return frappe.database.postgres.setup_db.bootstrap_database(verbose, source_sql) else: import frappe.database.mariadb.setup_db - return frappe.database.mariadb.setup_db.bootstrap_database(db_name, verbose, source_sql) + return frappe.database.mariadb.setup_db.bootstrap_database(verbose, source_sql) def drop_user_and_database(db_name, db_user): diff --git a/frappe/database/mariadb/setup_db.py b/frappe/database/mariadb/setup_db.py index 7cf567c504..348c16529e 100644 --- a/frappe/database/mariadb/setup_db.py +++ b/frappe/database/mariadb/setup_db.py @@ -73,17 +73,17 @@ def drop_user_and_database( dbman.delete_user(db_user) -def bootstrap_database(db_name, verbose, source_sql=None): +def bootstrap_database(verbose, source_sql=None): import sys - frappe.connect(db_name=db_name) + frappe.connect() if not check_database_settings(): print("Database settings do not match expected values; stopping database setup.") sys.exit(1) import_db_from_sql(source_sql, verbose) - frappe.connect(db_name=db_name) + frappe.connect() if "tabDefaultValue" not in frappe.db.get_tables(cached=False): from click import secho diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 855f31b89c..ba45af4cad 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -29,11 +29,12 @@ def setup_database(): root_conn.close() -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) +def bootstrap_database(verbose, source_sql=None): + frappe.connect() + import_db_from_sql(source_sql, verbose) + + frappe.connect() if "tabDefaultValue" not in frappe.db.get_tables(): import sys diff --git a/frappe/installer.py b/frappe/installer.py index 2f7138cb48..fce104ad99 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -170,7 +170,6 @@ def install_db( setup_database(force, verbose, no_mariadb_socket) bootstrap_database( - db_name=frappe.conf.db_name, verbose=verbose, source_sql=source_sql, ) From ef1ee77c99136c7dbc612c13f09c0e4d94e74e4a Mon Sep 17 00:00:00 2001 From: David Arnold Date: Sat, 20 Jan 2024 07:30:44 +0100 Subject: [PATCH 2/8] test: fully spec test command line --- frappe/tests/test_commands.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 7eff954054..fc39adc9a3 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -173,14 +173,18 @@ class BaseTestCommands(FrappeTestCase): cmd_config = { "test_site": TEST_SITE, "admin_password": frappe.conf.admin_password, - "root_login": frappe.conf.root_login, - "root_password": frappe.conf.root_password, "db_type": frappe.conf.db_type, + "db_root_username": frappe.conf.root_login, + "db_root_password": frappe.conf.root_password, } if not os.path.exists(os.path.join(TEST_SITE, "site_config.json")): cls.execute( - "bench new-site {test_site} --admin-password {admin_password} --db-type" " {db_type}", + "bench new-site {test_site} " + "--admin-password {admin_password} " + "--db-root-username {db_root_username} " + "--db-root-password {db_root_password} " + "--db-type {db_type}", cmd_config, ) From f1bcdabb6ce46ede102129fef6b37b5e9248170f Mon Sep 17 00:00:00 2001 From: David Arnold Date: Sun, 21 Jan 2024 18:24:18 +0100 Subject: [PATCH 3/8] fix: assert minimum contract for frappe.connect --- frappe/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/__init__.py b/frappe/__init__.py index e930b8d500..51ee459b4c 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -301,6 +301,10 @@ def connect( "Instead, explicitly invoke frappe.init(site) with the right config prior to calling frappe.connect(), if necessary." ) + assert db_name or local.conf.db_user, "site must be fully initialized, db_user missing" + assert db_name or local.conf.db_name, "site must be fully initialized, db_name missing" + assert local.conf.db_password, "site must be fully initialized, db_password missing" + local.db = get_db( host=local.conf.db_host, port=local.conf.db_port, From 78aad52b2c1e9af012444a95dda4949aad9f58c8 Mon Sep 17 00:00:00 2001 From: David Arnold Date: Sun, 21 Jan 2024 17:36:22 +0100 Subject: [PATCH 4/8] fix: default db_user where config defaults are set --- frappe/__init__.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 51ee459b4c..c33cb64f08 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -269,10 +269,6 @@ def init(site: str, sites_path: str = ".", new_site: bool = False, force=False) local.initialised = True - # Set the user as database name if not set in config - if local.conf and local.conf.db_name is not None and local.conf.db_user is None: - local.conf.db_user = local.conf.db_name - def connect( site: str | None = None, db_name: str | None = None, set_admin_as_user: bool = True @@ -398,6 +394,9 @@ def get_site_config(sites_path: str | None = None, site_path: str | None = None) os.environ.get("FRAPPE_DB_PORT") or config.get("db_port") or db_default_ports(config["db_type"]) ) + # Set the user as database name if not set in config + config["db_user"] = config.get("db_user") or config.get("db_name") + return config From 25168b858fa4b9fa5913898b2c49153d7ecc3f11 Mon Sep 17 00:00:00 2001 From: David Arnold Date: Wed, 24 Jan 2024 11:39:50 +0100 Subject: [PATCH 5/8] feat: db_user can be set by env varbiable Co-authored-by: Akhil Narang --- frappe/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index c33cb64f08..65591afa89 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -395,7 +395,9 @@ def get_site_config(sites_path: str | None = None, site_path: str | None = None) ) # Set the user as database name if not set in config - config["db_user"] = config.get("db_user") or config.get("db_name") + config["db_user"] = ( + os.environ.get("FRAPPE_DB_USER") or config.get("db_user") or config.get("db_name") + ) return config From e7284be5a343d30456565417b74e05f02cd47154 Mon Sep 17 00:00:00 2001 From: David Arnold Date: Sun, 21 Jan 2024 18:21:25 +0100 Subject: [PATCH 6/8] fix: remove db name 'hack'; now the code path is clean --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 65591afa89..39ebed222d 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -304,7 +304,7 @@ def connect( local.db = get_db( host=local.conf.db_host, port=local.conf.db_port, - user=local.conf.db_user or db_name or local.conf.db_name, + user=db_name or local.conf.db_user, password=local.conf.db_password, cur_db_name=db_name or local.conf.db_name, ) From c57bc94eadd143c6bb8994f0ef6715213c1d2d8f Mon Sep 17 00:00:00 2001 From: David Arnold Date: Sat, 20 Jan 2024 07:36:35 +0100 Subject: [PATCH 7/8] chore: cleanup frappe.connect invocations --- frappe/commands/utils.py | 3 +-- frappe/desk/form/test_form.py | 7 ------- frappe/installer.py | 5 +++-- frappe/integrations/offsite_backup_utils.py | 3 --- frappe/test_runner.py | 11 +++++------ frappe/tests/test_fmt_money.py | 7 ------- frappe/translate.py | 6 ------ 7 files changed, 9 insertions(+), 33 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index a839566130..a6eedf102a 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -772,12 +772,11 @@ def run_tests( click.secho(f"bench --site {site} set-config allow_tests true", fg="green") return - frappe.init(site=site) - frappe.flags.skip_before_tests = skip_before_tests frappe.flags.skip_test_records = skip_test_records ret = frappe.test_runner.main( + site, app, module, doctype, diff --git a/frappe/desk/form/test_form.py b/frappe/desk/form/test_form.py index f256b03d27..5028fedf96 100644 --- a/frappe/desk/form/test_form.py +++ b/frappe/desk/form/test_form.py @@ -11,10 +11,3 @@ class TestForm(FrappeTestCase): results = get_linked_docs("Role", "System Manager", linkinfo=get_linked_doctypes("Role")) self.assertTrue("User" in results) self.assertTrue("DocType" in results) - - -if __name__ == "__main__": - import unittest - - frappe.connect() - unittest.main() diff --git a/frappe/installer.py b/frappe/installer.py index fce104ad99..68fd4d87d2 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -20,9 +20,10 @@ from frappe.utils.dashboard import sync_dashboards from frappe.utils.synchronization import filelock -def _is_scheduler_enabled() -> bool: +def _is_scheduler_enabled(site) -> bool: enable_scheduler = False try: + frappe.init(site=site) frappe.connect() enable_scheduler = cint(frappe.db.get_single_value("System Settings", "enable_scheduler")) except Exception: @@ -78,7 +79,7 @@ def _new_site( try: # enable scheduler post install? - enable_scheduler = _is_scheduler_enabled() + enable_scheduler = _is_scheduler_enabled(site) except Exception: enable_scheduler = False diff --git a/frappe/integrations/offsite_backup_utils.py b/frappe/integrations/offsite_backup_utils.py index e2e2fed40f..a71fe0e28d 100644 --- a/frappe/integrations/offsite_backup_utils.py +++ b/frappe/integrations/offsite_backup_utils.py @@ -41,9 +41,6 @@ def send_email(success, service_name, doctype, email_field, error_status=None): def get_recipients(doctype, email_field): - if not frappe.db: - frappe.connect() - return split_emails(frappe.db.get_value(doctype, None, email_field)) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 35911269cb..0b81aa75bd 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -38,6 +38,7 @@ def xmlrunner_wrapper(output): def main( + site=None, app=None, module=None, doctype=None, @@ -53,6 +54,10 @@ def main( ): global unittest_runner + frappe.init(site=site) + if not frappe.db: + frappe.connect() + if doctype_list_path: app, doctype_list_path = doctype_list_path.split(os.path.sep, 1) with open(frappe.get_app_path(app, doctype_list_path)) as f: @@ -69,9 +74,6 @@ def main( frappe.flags.print_messages = verbose frappe.flags.in_test = True - if not frappe.db: - frappe.connect() - # workaround! since there is no separate test db frappe.clear_cache() scheduler_disabled_by_user = frappe.utils.scheduler.is_scheduler_disabled(verbose=False) @@ -329,9 +331,6 @@ def _add_test(app, path, filename, verbose, test_suite=None): def make_test_records(doctype, verbose=0, force=False, commit=False): - if not frappe.db: - frappe.connect() - if frappe.flags.skip_test_records: return diff --git a/frappe/tests/test_fmt_money.py b/frappe/tests/test_fmt_money.py index fff5011189..0fbd38cbcc 100644 --- a/frappe/tests/test_fmt_money.py +++ b/frappe/tests/test_fmt_money.py @@ -100,10 +100,3 @@ class TestFmtMoney(FrappeTestCase): frappe.db.set_value("Currency", "JPY", "symbol_on_right", 1) self.assertEqual(fmt_money(100.0, format="#,###.##", currency="JPY"), "100.00 ¥") self.assertEqual(fmt_money(100.0, format="#,###.##", currency="USD"), "$ 100.00") - - -if __name__ == "__main__": - import unittest - - frappe.connect() - unittest.main() diff --git a/frappe/translate.py b/frappe/translate.py index fc4461e102..2e08494df7 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -239,9 +239,6 @@ def get_translation_dict_from_file(path, lang, app, throw=False) -> dict[str, st def get_user_translations(lang): - if not frappe.db: - frappe.connect() - def _read_from_db(): user_translations = {} translations = frappe.get_all( @@ -1054,9 +1051,6 @@ def get_all_languages(with_language_name: bool = False) -> list: def get_all_language_with_name(): return frappe.get_all("Language", ["language_code", "language_name"], {"enabled": 1}) - if not frappe.db: - frappe.connect() - if with_language_name: return frappe.cache.get_value("languages_with_name", get_all_language_with_name) else: From 161bb4250871d23c346b40f1640562b054089c8b Mon Sep 17 00:00:00 2001 From: David Arnold Date: Wed, 31 Jan 2024 09:58:56 +0100 Subject: [PATCH 8/8] fix: deprioritize precedence of db_name argument to connect --- frappe/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 39ebed222d..f5d72fa6ba 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -304,9 +304,9 @@ def connect( local.db = get_db( host=local.conf.db_host, port=local.conf.db_port, - user=db_name or local.conf.db_user, + user=local.conf.db_user or db_name, password=local.conf.db_password, - cur_db_name=db_name or local.conf.db_name, + cur_db_name=local.conf.db_name or db_name, ) if set_admin_as_user: set_user("Administrator")