diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index 367b576204..6e6fa32650 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -51,7 +51,7 @@ jobs: python $GITHUB_WORKSPACE/.github/helper/documentation.py $PR_NUMBER linter: - name: 'Frappe Linter' + name: 'Semgrep Rules' runs-on: ubuntu-latest if: github.event_name == 'pull_request' @@ -61,7 +61,6 @@ jobs: with: python-version: '3.10' cache: pip - - uses: pre-commit/action@v3.0.0 - name: Download Semgrep rules run: git clone --depth 1 https://github.com/frappe/semgrep-rules.git frappe-semgrep-rules diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml new file mode 100644 index 0000000000..32ececc78a --- /dev/null +++ b/.github/workflows/pre-commit.yml @@ -0,0 +1,26 @@ +name: Pre-commit + +on: + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: precommit-frappe-${{ github.event_name }}-${{ github.event.number }} + cancel-in-progress: true + +jobs: + linter: + name: 'precommit' + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.10' + cache: pip + - uses: pre-commit/action@v3.0.0 diff --git a/frappe/__init__.py b/frappe/__init__.py index 30e848129d..289e5c6e3d 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -279,13 +279,19 @@ def connect( ) -> None: """Connect to site database instance. - :param site: If site is given, calls `frappe.init`. + :param site: (Deprecated) If site is given, calls `frappe.init`. :param db_name: Optional. Will use from `site_config.json`. :param set_admin_as_user: Set Administrator as current user. """ from frappe.database import get_db if site: + from frappe.utils.deprecations import deprecation_warning + + deprecation_warning( + "Calling frappe.connect with the site argument is deprecated and will be removed in next major version. " + "Instead, explicitly invoke frappe.init(site) prior to calling frappe.connect(), if initializing the site is necessary." + ) init(site) local.db = get_db( diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 73724b8df8..3b6fcb32f1 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -342,7 +342,7 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): site = get_site(context) verbose = context.verbose or verbose frappe.init(site=site) - frappe.connect(site=site) + frappe.connect() err, out = frappe.utils.execute_in_shell(f"file {sql_file_path}", check_exit_code=True) if err: click.secho("Failed to detect type of backup file", fg="red") @@ -538,7 +538,8 @@ def add_db_index(context, doctype, column): columns = column # correct naming for site in context.sites: - frappe.connect(site=site) + frappe.init(site=site) + frappe.connect() try: frappe.db.add_index(doctype, columns) if len(columns) == 1: @@ -580,7 +581,8 @@ def describe_database_table(context, doctype, column): import json for site in context.sites: - frappe.connect(site=site) + frappe.init(site=site) + frappe.connect() try: data = _extract_table_stats(doctype, column) # NOTE: Do not print anything else in this to avoid clobbering the output. @@ -666,7 +668,8 @@ def add_system_manager(context, email, first_name, last_name, send_welcome_email import frappe.utils.user for site in context.sites: - frappe.connect(site=site) + frappe.init(site=site) + frappe.connect() try: frappe.utils.user.add_system_manager(email, first_name, last_name, send_welcome_email, password) frappe.db.commit() @@ -692,7 +695,8 @@ def add_user_for_sites( import frappe.utils.user for site in context.sites: - frappe.connect(site=site) + frappe.init(site=site) + frappe.connect() try: add_new_user(email, first_name, last_name, user_type, send_welcome_email, password, add_role) frappe.db.commit() diff --git a/frappe/commands/translate.py b/frappe/commands/translate.py index 5042843405..247d4a77d5 100644 --- a/frappe/commands/translate.py +++ b/frappe/commands/translate.py @@ -34,7 +34,8 @@ def new_language(context, lang_code, app): raise Exception("--site is required") # init site - frappe.connect(site=context["sites"][0]) + frappe.init(site=context["sites"][0]) + frappe.connect() frappe.translate.write_translations_file(app, lang_code) print( diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 8f6cb70d17..5d27fe6e6e 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -108,7 +108,8 @@ def clear_cache(context): for site in context.sites: try: - frappe.connect(site) + frappe.init(site=site) + frappe.connect() frappe.clear_cache() clear_website_cache() finally: @@ -601,7 +602,7 @@ def console(context, autoreload=False): all_apps = frappe.get_installed_apps() failed_to_import = [] - for app in all_apps: + for app in list(all_apps): try: locals()[app] = __import__(app) except ModuleNotFoundError: diff --git a/frappe/core/doctype/comment/comment.py b/frappe/core/doctype/comment/comment.py index 946f9833e1..b33b7e6f63 100644 --- a/frappe/core/doctype/comment/comment.py +++ b/frappe/core/doctype/comment/comment.py @@ -93,7 +93,7 @@ class Comment(Document): def remove_comment_from_cache(self): _comments = get_comments_from_parent(self) - for c in _comments: + for c in list(_comments): if c.get("name") == self.name: _comments.remove(c) diff --git a/frappe/core/utils.py b/frappe/core/utils.py index 3e7fb8f350..13b912b3aa 100644 --- a/frappe/core/utils.py +++ b/frappe/core/utils.py @@ -8,7 +8,7 @@ import frappe def get_parent_doc(doc): """Return document of `reference_doctype`, `reference_doctype`.""" - if not hasattr(doc, "parent_doc"): + if not getattr(doc, "parent_doc", None): if doc.reference_doctype and doc.reference_name: doc.parent_doc = frappe.get_doc(doc.reference_doctype, doc.reference_name) else: diff --git a/frappe/database/mariadb/setup_db.py b/frappe/database/mariadb/setup_db.py index e1d5644283..7cf567c504 100644 --- a/frappe/database/mariadb/setup_db.py +++ b/frappe/database/mariadb/setup_db.py @@ -162,16 +162,17 @@ def check_compatible_versions(): def get_root_connection(): if not frappe.local.flags.root_connection: + from getpass import getpass + if not frappe.flags.root_login: - frappe.flags.root_login = "root" + frappe.flags.root_login = ( + frappe.conf.get("root_login") or input("Enter mysql super user [root]: ") or "root" + ) if not frappe.flags.root_password: - frappe.flags.root_password = frappe.conf.get("root_password") or None - - if not frappe.flags.root_password: - import getpass - - frappe.flags.root_password = getpass.getpass("MySQL root password: ") + frappe.flags.root_password = frappe.conf.get("root_password") or getpass( + "MySQL root password: " + ) frappe.local.flags.root_connection = frappe.database.get_db( host=frappe.conf.db_host, diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 50c35a08ba..855f31b89c 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -63,19 +63,17 @@ def import_db_from_sql(source_sql=None, verbose=False): def get_root_connection(): if not frappe.local.flags.root_connection: - if not frappe.flags.root_login: - frappe.flags.root_login = frappe.conf.get("root_login") or None + from getpass import getpass if not frappe.flags.root_login: - frappe.flags.root_login = input("Enter postgres super user: ") + frappe.flags.root_login = ( + frappe.conf.get("root_login") or input("Enter postgres super user [postgres]: ") or "postgres" + ) if not frappe.flags.root_password: - frappe.flags.root_password = frappe.conf.get("root_password") or None - - if not frappe.flags.root_password: - from getpass import getpass - - frappe.flags.root_password = getpass("Postgres super user password: ") + frappe.flags.root_password = frappe.conf.get("root_password") or getpass( + "Postgres super user password: " + ) frappe.local.flags.root_connection = frappe.database.get_db( host=frappe.conf.db_host, diff --git a/frappe/desk/doctype/desktop_icon/desktop_icon.py b/frappe/desk/doctype/desktop_icon/desktop_icon.py index fda9eed7bb..1c3c2d1c86 100644 --- a/frappe/desk/doctype/desktop_icon/desktop_icon.py +++ b/frappe/desk/doctype/desktop_icon/desktop_icon.py @@ -271,7 +271,7 @@ def set_desktop_icons(visible_list, ignore_duplicate=True): frappe.db.sql("update `tabDesktop Icon` set blocked=0, hidden=1 where standard=1") # set as visible if present, or add icon - for module_name in visible_list: + for module_name in list(visible_list): name = frappe.db.get_value("Desktop Icon", {"module_name": module_name}) if name: frappe.db.set_value("Desktop Icon", name, "hidden", 0) diff --git a/frappe/desk/doctype/kanban_board/kanban_board.py b/frappe/desk/doctype/kanban_board/kanban_board.py index 4ab9e66f9d..49604f6cd0 100644 --- a/frappe/desk/doctype/kanban_board/kanban_board.py +++ b/frappe/desk/doctype/kanban_board/kanban_board.py @@ -238,7 +238,7 @@ def update_column_order(board_name, order): new_columns = [] for col in order: - for column in old_columns: + for column in list(old_columns): if col == column.column_name: new_columns.append(column) old_columns.remove(column) diff --git a/frappe/desk/page/setup_wizard/setup_wizard.js b/frappe/desk/page/setup_wizard/setup_wizard.js index 44960915b1..dde665bce5 100644 --- a/frappe/desk/page/setup_wizard/setup_wizard.js +++ b/frappe/desk/page/setup_wizard/setup_wizard.js @@ -476,18 +476,15 @@ frappe.setup.slides_settings = [ onload: function (slide) { if (frappe.session.user !== "Administrator") { - slide.form.fields_dict.email.$wrapper.toggle(false); - slide.form.fields_dict.password.$wrapper.toggle(false); - - // remove password field - delete slide.form.fields_dict.password; - - if (frappe.boot.user.first_name || frappe.boot.user.last_name) { + const { first_name, last_name, email } = frappe.boot.user; + if (first_name || last_name) { slide.form.fields_dict.full_name.set_input( - [frappe.boot.user.first_name, frappe.boot.user.last_name].join(" ").trim() + [first_name, last_name].join(" ").trim() ); } - delete slide.form.fields_dict.email; + slide.form.fields_dict.email.set_input(email); + slide.form.fields_dict.email.df.read_only = 1; + slide.form.fields_dict.email.refresh(); } else { slide.form.fields_dict.email.df.reqd = 1; slide.form.fields_dict.email.refresh(); diff --git a/frappe/desk/page/setup_wizard/setup_wizard.py b/frappe/desk/page/setup_wizard/setup_wizard.py index 22aa4245d8..e5601a195c 100755 --- a/frappe/desk/page/setup_wizard/setup_wizard.py +++ b/frappe/desk/page/setup_wizard/setup_wizard.py @@ -14,7 +14,7 @@ from frappe.utils.password import update_password from . import install_fixtures -def get_setup_stages(args): +def get_setup_stages(args): # nosemgrep # App setup stage functions should not include frappe.db.commit # That is done by frappe after successful completion of all stages @@ -104,18 +104,18 @@ def process_setup_stages(stages, user_input, is_background_task=False): frappe.flags.in_setup_wizard = False -def update_global_settings(args): +def update_global_settings(args): # nosemgrep if args.language and args.language != "English": set_default_language(get_language_code(args.lang)) frappe.db.commit() frappe.clear_cache() update_system_settings(args) - update_user_name(args) + create_or_update_user(args) set_timezone(args) -def run_post_setup_complete(args): +def run_post_setup_complete(args): # nosemgrep disable_future_access() frappe.db.commit() frappe.clear_cache() @@ -124,20 +124,20 @@ def run_post_setup_complete(args): frappe.get_cached_doc("System Settings") and frappe.get_doc("System Settings") -def run_setup_success(args): +def run_setup_success(args): # nosemgrep for hook in frappe.get_hooks("setup_wizard_success"): frappe.get_attr(hook)(args) install_fixtures.install() -def get_stages_hooks(args): +def get_stages_hooks(args): # nosemgrep stages = [] for method in frappe.get_hooks("setup_wizard_stages"): stages += frappe.get_attr(method)(args) return stages -def get_setup_complete_hooks(args): +def get_setup_complete_hooks(args): # nosemgrep return [ { "status": "Executing method", @@ -154,7 +154,7 @@ def get_setup_complete_hooks(args): ] -def handle_setup_exception(args): +def handle_setup_exception(args): # nosemgrep frappe.db.rollback() if args: traceback = frappe.get_traceback(with_context=True) @@ -163,7 +163,7 @@ def handle_setup_exception(args): frappe.get_attr(hook)(traceback, args) -def update_system_settings(args): +def update_system_settings(args): # nosemgrep number_format = get_country_info(args.get("country")).get("number_format", "#,###.##") # replace these as float number formats, as they have 0 precision @@ -194,72 +194,48 @@ def update_system_settings(args): frappe.db.set_default("session_recording_start", now()) -def update_user_name(args): +def create_or_update_user(args): # nosemgrep + email = args.get("email") first_name, last_name = args.get("full_name", ""), "" if " " in first_name: first_name, last_name = first_name.split(" ", 1) - if args.get("email"): - if frappe.db.exists("User", args.get("email")): - # running again - return - - args["name"] = args.get("email") - + if user := frappe.db.get_value("User", email, ["first_name", "last_name"], as_dict=True): + if user.first_name != first_name or user.last_name != last_name: + ( + frappe.qb.update("User") + .set("first_name", first_name) + .set("last_name", last_name) + .set("full_name", args.get("full_name")) + ).run() + else: _mute_emails, frappe.flags.mute_emails = frappe.flags.mute_emails, True - doc = frappe.get_doc( + + user = frappe.new_doc("User") + user.update( { - "doctype": "User", - "email": args.get("email"), + "email": email, "first_name": first_name, "last_name": last_name, } ) + user.append_roles(*_get_default_roles()) + user.flags.no_welcome_mail = True + user.insert() - doc.append_roles(*_get_default_roles()) - doc.flags.no_welcome_mail = True - doc.insert() frappe.flags.mute_emails = _mute_emails - update_password(args.get("email"), args.get("password")) - elif first_name: - args.update({"name": frappe.session.user, "first_name": first_name, "last_name": last_name}) - - frappe.db.sql( - """update `tabUser` SET first_name=%(first_name)s, - last_name=%(last_name)s WHERE name=%(name)s""", - args, - ) - - if args.get("attach_user"): - attach_user = args.get("attach_user").split(",") - if len(attach_user) == 3: - filename, filetype, content = attach_user - _file = frappe.get_doc( - { - "doctype": "File", - "file_name": filename, - "attached_to_doctype": "User", - "attached_to_name": args.get("name"), - "content": content, - "decode": True, - } - ) - _file.save() - fileurl = _file.file_url - frappe.db.set_value("User", args.get("name"), "user_image", fileurl) - - if args.get("name"): - add_all_roles_to(args.get("name")) + if args.get("password"): + update_password(email, args.get("password")) -def set_timezone(args): +def set_timezone(args): # nosemgrep if args.get("timezone"): for name in frappe.STANDARD_USERS: frappe.db.set_value("User", name, "time_zone", args.get("timezone")) -def parse_args(args): +def parse_args(args): # nosemgrep if not args: args = frappe.local.form_dict if isinstance(args, str): @@ -344,7 +320,7 @@ def load_user_details(): } -def prettify_args(args): +def prettify_args(args): # nosemgrep # remove attachments for key, val in args.items(): if isinstance(val, str) and "data:image" in val: @@ -357,7 +333,7 @@ def prettify_args(args): return pretty_args -def email_setup_wizard_exception(traceback, args): +def email_setup_wizard_exception(traceback, args): # nosemgrep if not frappe.conf.setup_wizard_exception_email: return @@ -402,7 +378,7 @@ def email_setup_wizard_exception(traceback, args): ) -def log_setup_wizard_exception(traceback, args): +def log_setup_wizard_exception(traceback, args): # nosemgrep with open("../logs/setup-wizard.log", "w+") as setup_log: setup_log.write(traceback) setup_log.write(json.dumps(args)) diff --git a/frappe/geo/doctype/currency/currency.json b/frappe/geo/doctype/currency/currency.json index c51ab7f063..00dfe248c9 100644 --- a/frappe/geo/doctype/currency/currency.json +++ b/frappe/geo/doctype/currency/currency.json @@ -82,7 +82,7 @@ "idx": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2022-07-04 09:42:52.425440", + "modified": "2024-01-17 15:37:31.605278", "modified_by": "Administrator", "module": "Geo", "name": "Currency", @@ -102,6 +102,10 @@ "share": 1, "write": 1 }, + { + "read": 1, + "role": "Accounts Manager" + }, { "read": 1, "role": "Accounts User" diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 241bfdee31..f83a971082 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import datetime import json +import weakref from functools import cached_property from typing import TYPE_CHECKING, TypeVar @@ -163,6 +164,7 @@ class BaseDocument: state.pop("meta", None) state.pop("permitted_fieldnames", None) + state.pop("_parent_doc", None) def update(self, d): """Update multiple fields of a doctype using a dictionary of key-value pairs. @@ -261,11 +263,28 @@ class BaseDocument: ret_value = self._init_child(value, key) table.append(ret_value) - # reference parent document - ret_value.parent_doc = self + # reference parent document but with weak reference, parent_doc will be deleted if self is garbage collected. + ret_value.parent_doc = weakref.ref(self) return ret_value + @property + def parent_doc(self): + parent_doc_ref = getattr(self, "_parent_doc", None) + + if isinstance(parent_doc_ref, BaseDocument): + return parent_doc_ref + elif isinstance(parent_doc_ref, weakref.ReferenceType): + return parent_doc_ref() + + @parent_doc.setter + def parent_doc(self, value): + self._parent_doc = value + + @parent_doc.deleter + def parent_doc(self): + self._parent_doc = None + def extend(self, key, value): try: value = iter(value) @@ -1231,7 +1250,7 @@ class BaseDocument: ref_doc = frappe.new_doc(self.doctype) else: # get values from old doc - if self.get("parent_doc"): + if self.parent_doc: parent_doc = self.parent_doc.get_latest() child_docs = [d for d in parent_doc.get(self.parentfield) if d.name == self.name] if not child_docs: diff --git a/frappe/public/js/frappe/form/controls/base_control.js b/frappe/public/js/frappe/form/controls/base_control.js index 88aceb56a3..7817db95a9 100644 --- a/frappe/public/js/frappe/form/controls/base_control.js +++ b/frappe/public/js/frappe/form/controls/base_control.js @@ -49,9 +49,6 @@ frappe.ui.form.Control = class BaseControl { if (this.df.get_status) { return this.df.get_status(this); } - if (this.df.is_virtual) { - return "Read"; - } if ( (!this.doctype && !this.docname) || diff --git a/frappe/public/js/frappe/form/controls/select.js b/frappe/public/js/frappe/form/controls/select.js index e3bdfa1e38..95a3ed36df 100644 --- a/frappe/public/js/frappe/form/controls/select.js +++ b/frappe/public/js/frappe/form/controls/select.js @@ -111,7 +111,7 @@ frappe.ui.form.add_options = function (input, options_list, sort) { let options = options_list.map((raw_option) => parse_option(raw_option)); if (sort) { - options = options.sort((a, b) => a.label.localeCompare(b.label)); + options = options.sort((a, b) => cstr(a.label).localeCompare(cstr(b.label))); } options diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index 22be220af2..77b92b046e 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -193,7 +193,7 @@ $.extend(frappe.perm, { if (!perm) { let is_hidden = df && (cint(df.hidden) || cint(df.hidden_due_to_dependency)); - let is_read_only = df && cint(df.read_only); + let is_read_only = df && (cint(df.read_only) || cint(df.is_virtual)); return is_hidden ? "None" : is_read_only ? "Read" : "Write"; } diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index ad42e355c1..7eff954054 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -461,12 +461,17 @@ class TestCommands(BaseTestCommands): self.execute( f"bench new-site {site} --force --verbose " f"--admin-password {frappe.conf.admin_password} " - f"--mariadb-root-password {frappe.conf.root_password} " + f"--db-root-username {frappe.conf.root_login} " + f"--db-root-password {frappe.conf.root_password} " f"--db-type {frappe.conf.db_type} " ) self.assertEqual(self.returncode, 0) - self.execute(f"bench drop-site {site} --force --root-password {frappe.conf.root_password}") + self.execute( + f"bench drop-site {site} --force " + f"--db-root-username {frappe.conf.root_login} " + f"--db-root-password {frappe.conf.root_password} " + ) self.assertEqual(self.returncode, 0) bench_path = get_bench_path() @@ -486,7 +491,8 @@ class TestCommands(BaseTestCommands): self.execute( f"bench new-site {TEST_SITE} --verbose " f"--admin-password {frappe.conf.admin_password} " - f"--mariadb-root-password {frappe.conf.root_password} " + f"--db-root-username {frappe.conf.root_login} " + f"--db-root-password {frappe.conf.root_password} " f"--db-type {frappe.conf.db_type} " ) @@ -520,16 +526,17 @@ class TestCommands(BaseTestCommands): kwargs = { "new_site": site, "admin_password": frappe.conf.admin_password, - "root_password": frappe.conf.root_password or "", "db_type": frappe.conf.db_type, "db_user": user, "db_password": password, "db_root_username": frappe.conf.root_login, + "db_root_password": frappe.conf.root_password or "", } self.execute( "bench new-site {new_site} --force --verbose " "--admin-password {admin_password} " - "--db-root-password {root_password} " + "--db-root-username {db_root_username} " + "--db-root-password {db_root_password} " "--db-type {db_type} " "--db-user {db_user} " "--db-password {db_password}", @@ -542,7 +549,9 @@ class TestCommands(BaseTestCommands): self.assertEqual(config[site]["db_user"], user) self.assertEqual(config[site]["db_password"], password) self.execute( - "bench drop-site {new_site} --force --db-root-username {db_root_username} --db-root-password {root_password}", + "bench drop-site {new_site} --force " + "--db-root-username {db_root_username} " + "--db-root-password {db_root_password} ", kwargs, ) self.assertEqual(self.returncode, 0) @@ -564,19 +573,20 @@ class TestCommands(BaseTestCommands): kwargs = { "new_site": site, "admin_password": frappe.conf.admin_password, - "root_password": frappe.conf.root_password, "db_type": frappe.conf.db_type, "db_user": user, "db_password": password, "db_root_username": frappe.conf.root_login, + "db_root_password": frappe.conf.root_password, } self.execute( "bench new-site {new_site} --force --verbose " "--admin-password {admin_password} " - "--db-root-password {root_password} " "--db-type {db_type} " "--db-user {db_user} " - "--db-password {db_password}", + "--db-password {db_password} " + "--db-root-username {db_root_username} " + "--db-root-password {db_root_password} ", kwargs, ) self.assertEqual(self.returncode, 0) @@ -586,7 +596,9 @@ class TestCommands(BaseTestCommands): self.assertEqual(config[site]["db_user"], user) self.assertEqual(config[site]["db_password"], password) self.execute( - "bench drop-site {new_site} --force --db-root-username {db_root_username} --db-root-password {root_password}", + "bench drop-site {new_site} --force " + "--db-root-username {db_root_username} " + "--db-root-password {db_root_password} ", kwargs, ) self.assertEqual(self.returncode, 0) diff --git a/frappe/tests/test_perf.py b/frappe/tests/test_perf.py index e05fb03f82..fc736543a8 100644 --- a/frappe/tests/test_perf.py +++ b/frappe/tests/test_perf.py @@ -193,3 +193,7 @@ class TestPerformance(FrappeTestCase): result = frappe.db.sql(query, **kwargs) self.assertEqual(sys.getrefcount(result), 2) # Note: This always returns +1 self.assertFalse(gc.get_referrers(result)) + + def test_no_cyclic_references(self): + doc = frappe.get_doc("User", "Administrator") + self.assertEqual(sys.getrefcount(doc), 2) # Note: This always returns +1 diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index 36b4a75c96..1283df7e7b 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -188,7 +188,8 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, retval = None if is_async: - frappe.connect(site) + frappe.init(site=site) + frappe.connect() if os.environ.get("CI"): frappe.flags.in_test = True @@ -272,6 +273,7 @@ def start_worker( if not strategy: strategy = DequeueStrategy.DEFAULT + _start_sentry() _freeze_gc() with frappe.init_site(): @@ -292,49 +294,6 @@ def start_worker( if quiet: logging_level = "WARNING" - # Always initialize sentry SDK if the DSN is sent - if sentry_dsn := os.getenv("FRAPPE_SENTRY_DSN"): - import sentry_sdk - from sentry_sdk.integrations.argv import ArgvIntegration - from sentry_sdk.integrations.atexit import AtexitIntegration - from sentry_sdk.integrations.dedupe import DedupeIntegration - from sentry_sdk.integrations.excepthook import ExcepthookIntegration - from sentry_sdk.integrations.modules import ModulesIntegration - from sentry_sdk.integrations.rq import RqIntegration - - from frappe.utils.sentry import FrappeIntegration, before_send - - integrations = [ - AtexitIntegration(), - ExcepthookIntegration(), - DedupeIntegration(), - ModulesIntegration(), - ArgvIntegration(), - RqIntegration(), - ] - - experiments = {} - kwargs = {} - - if os.getenv("ENABLE_SENTRY_DB_MONITORING"): - integrations.append(FrappeIntegration()) - experiments["record_sql_params"] = True - - if tracing_sample_rate := os.getenv("SENTRY_TRACING_SAMPLE_RATE"): - kwargs["traces_sample_rate"] = float(tracing_sample_rate) - - sentry_sdk.init( - dsn=sentry_dsn, - before_send=before_send, - attach_stacktrace=True, - release=frappe.__version__, - auto_enabling_integrations=False, - default_integrations=False, - integrations=integrations, - _experiments=experiments, - **kwargs, - ) - worker = Worker(queues, name=get_worker_name(queue_name), connection=redis_connection) worker.work( logging_level=logging_level, @@ -356,6 +315,7 @@ def start_worker_pool( WARNING: This feature is considered "EXPERIMENTAL". """ + _start_sentry() _freeze_gc() with frappe.init_site(): @@ -622,3 +582,50 @@ def truncate_failed_registry(job, connection, type, value, traceback): for job_ids in create_batch(failed_jobs, 100): for job_obj in Job.fetch_many(job_ids=job_ids, connection=connection): job_obj and fail_registry.remove(job_obj, delete_job=True) + + +def _start_sentry(): + sentry_dsn = os.getenv("FRAPPE_SENTRY_DSN") + if not sentry_dsn: + return + + import sentry_sdk + from sentry_sdk.integrations.argv import ArgvIntegration + from sentry_sdk.integrations.atexit import AtexitIntegration + from sentry_sdk.integrations.dedupe import DedupeIntegration + from sentry_sdk.integrations.excepthook import ExcepthookIntegration + from sentry_sdk.integrations.modules import ModulesIntegration + from sentry_sdk.integrations.rq import RqIntegration + + from frappe.utils.sentry import FrappeIntegration, before_send + + integrations = [ + AtexitIntegration(), + ExcepthookIntegration(), + DedupeIntegration(), + ModulesIntegration(), + ArgvIntegration(), + RqIntegration(), + ] + + experiments = {} + kwargs = {} + + if os.getenv("ENABLE_SENTRY_DB_MONITORING"): + integrations.append(FrappeIntegration()) + experiments["record_sql_params"] = True + + if tracing_sample_rate := os.getenv("SENTRY_TRACING_SAMPLE_RATE"): + kwargs["traces_sample_rate"] = float(tracing_sample_rate) + + sentry_sdk.init( + dsn=sentry_dsn, + before_send=before_send, + attach_stacktrace=True, + release=frappe.__version__, + auto_enabling_integrations=False, + default_integrations=False, + integrations=integrations, + _experiments=experiments, + **kwargs, + ) diff --git a/frappe/utils/response.py b/frappe/utils/response.py index 448d43a456..9fe10545d1 100644 --- a/frappe/utils/response.py +++ b/frappe/utils/response.py @@ -61,7 +61,7 @@ def report_error(status_code): def _link_error_with_message_log(error_log, exception, message_logs): - for message in message_logs: + for message in list(message_logs): if message.get("__frappe_exc_id") == getattr(exception, "__frappe_exc_id", None): error_log.update(message) message_logs.remove(message)