From f110b6eea3dffc6da45152b08c98c5a2659e64b0 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 18 Dec 2023 10:56:51 +0530 Subject: [PATCH] refactor(data_import): handle RQ timeouts better (#23811) * refactor(data_import): handle RQ timeouts better Signed-off-by: Akhil Narang * refactor(data_import): display count of documents even for timed out jobs Also handle "0" cases better - should be plural there Signed-off-by: Akhil Narang --------- Signed-off-by: Akhil Narang --- .../core/doctype/data_import/data_import.js | 59 ++- .../core/doctype/data_import/data_import.json | 394 +++++++++--------- .../core/doctype/data_import/data_import.py | 12 +- .../doctype/data_import/data_import_list.js | 3 +- 4 files changed, 236 insertions(+), 232 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 7db3aa9629..b3fa136eb4 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -136,47 +136,40 @@ frappe.ui.form.on("Data Import", { let total_records = cint(r.message.total_records); if (!total_records) return; + let action, message; + if (frm.doc.import_type === "Insert New Records") { + action = "imported"; + } else { + action = "updated"; + } - let message; if (failed_records === 0) { - let message_args = [successful_records]; - if (frm.doc.import_type === "Insert New Records") { - message = - successful_records > 1 - ? __("Successfully imported {0} records.", message_args) - : __("Successfully imported {0} record.", message_args); + let message_args = [action, successful_records]; + if (successful_records === 1) { + message = __("Successfully {0} 1 record.", message_args); } else { - message = - successful_records > 1 - ? __("Successfully updated {0} records.", message_args) - : __("Successfully updated {0} record.", message_args); + message = __("Successfully {0} {1} records.", message_args); } } else { - let message_args = [successful_records, total_records]; - if (frm.doc.import_type === "Insert New Records") { - message = - successful_records > 1 - ? __( - "Successfully imported {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.", - message_args - ) - : __( - "Successfully imported {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.", - message_args - ); + let message_args = [action, successful_records, total_records]; + if (successful_records === 1) { + message = __( + "Successfully {0} {1} record out of {2}. Click on Export Errored Rows, fix the errors and import again.", + message_args + ); } else { - message = - successful_records > 1 - ? __( - "Successfully updated {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.", - message_args - ) - : __( - "Successfully updated {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.", - message_args - ); + message = __( + "Successfully {0} {1} records out of {2}. Click on Export Errored Rows, fix the errors and import again.", + message_args + ); } } + + // If the job timed out, display an extra hint + if (r.message.status === "Timed Out") { + message += "
" + __("Import timed out, please re-try."); + } + frm.dashboard.set_headline(message); }, }); diff --git a/frappe/core/doctype/data_import/data_import.json b/frappe/core/doctype/data_import/data_import.json index faa9a33bf1..97716219a2 100644 --- a/frappe/core/doctype/data_import/data_import.json +++ b/frappe/core/doctype/data_import/data_import.json @@ -1,198 +1,198 @@ { - "actions": [], - "autoname": "format:{reference_doctype} Import on {creation}", - "beta": 1, - "creation": "2019-08-04 14:16:08.318714", - "doctype": "DocType", - "editable_grid": 1, - "engine": "InnoDB", - "field_order": [ - "reference_doctype", - "import_type", - "download_template", - "import_file", - "payload_count", - "html_5", - "google_sheets_url", - "refresh_google_sheet", - "column_break_5", - "status", - "submit_after_import", - "mute_emails", - "template_options", - "import_warnings_section", - "template_warnings", - "import_warnings", - "section_import_preview", - "import_preview", - "import_log_section", - "show_failed_logs", - "import_log_preview" - ], - "fields": [ - { - "fieldname": "reference_doctype", - "fieldtype": "Link", - "in_list_view": 1, - "label": "Document Type", - "options": "DocType", - "reqd": 1, - "set_only_once": 1 - }, - { - "fieldname": "import_type", - "fieldtype": "Select", - "in_list_view": 1, - "label": "Import Type", - "options": "\nInsert New Records\nUpdate Existing Records", - "reqd": 1, - "set_only_once": 1 - }, - { - "depends_on": "eval:!doc.__islocal", - "fieldname": "import_file", - "fieldtype": "Attach", - "in_list_view": 1, - "label": "Import File", - "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)" - }, - { - "fieldname": "import_preview", - "fieldtype": "HTML", - "label": "Import Preview" - }, - { - "fieldname": "section_import_preview", - "fieldtype": "Section Break", - "label": "Preview" - }, - { - "fieldname": "column_break_5", - "fieldtype": "Column Break" - }, - { - "fieldname": "template_options", - "fieldtype": "Code", - "hidden": 1, - "label": "Template Options", - "options": "JSON", - "read_only": 1 - }, - { - "fieldname": "import_log_section", - "fieldtype": "Section Break", - "label": "Import Log" - }, - { - "fieldname": "import_log_preview", - "fieldtype": "HTML", - "label": "Import Log Preview" - }, - { - "default": "Pending", - "fieldname": "status", - "fieldtype": "Select", - "hidden": 1, - "label": "Status", - "no_copy": 1, - "options": "Pending\nSuccess\nPartial Success\nError", - "read_only": 1 - }, - { - "fieldname": "template_warnings", - "fieldtype": "Code", - "hidden": 1, - "label": "Template Warnings", - "options": "JSON" - }, - { - "default": "0", - "fieldname": "submit_after_import", - "fieldtype": "Check", - "label": "Submit After Import", - "set_only_once": 1 - }, - { - "fieldname": "import_warnings_section", - "fieldtype": "Section Break", - "label": "Import File Errors and Warnings" - }, - { - "fieldname": "import_warnings", - "fieldtype": "HTML", - "label": "Import Warnings" - }, - { - "depends_on": "eval:!doc.__islocal", - "fieldname": "download_template", - "fieldtype": "Button", - "label": "Download Template" - }, - { - "default": "1", - "fieldname": "mute_emails", - "fieldtype": "Check", - "label": "Don't Send Emails", - "set_only_once": 1 - }, - { - "default": "0", - "fieldname": "show_failed_logs", - "fieldtype": "Check", - "label": "Show Failed Logs" - }, - { - "depends_on": "eval:!doc.__islocal && !doc.import_file", - "fieldname": "html_5", - "fieldtype": "HTML", - "options": "
Or
" - }, - { - "depends_on": "eval:!doc.__islocal && !doc.import_file\n", - "description": "Must be a publicly accessible Google Sheets URL", - "fieldname": "google_sheets_url", - "fieldtype": "Data", - "label": "Import from Google Sheets", - "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)" - }, - { - "depends_on": "eval:doc.google_sheets_url && !doc.__unsaved && ['Success', 'Partial Success'].includes(doc.status)", - "fieldname": "refresh_google_sheet", - "fieldtype": "Button", - "label": "Refresh Google Sheet" - }, - { - "fieldname": "payload_count", - "fieldtype": "Int", - "hidden": 1, - "label": "Payload Count", - "read_only": 1 - } - ], - "hide_toolbar": 1, - "links": [], - "modified": "2022-02-14 10:08:37.624914", - "modified_by": "Administrator", - "module": "Core", - "name": "Data Import", - "naming_rule": "Expression", - "owner": "Administrator", - "permissions": [ - { - "create": 1, - "delete": 1, - "email": 1, - "export": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "System Manager", - "share": 1, - "write": 1 - } - ], - "sort_field": "modified", - "sort_order": "DESC", - "states": [], - "track_changes": 1 -} + "actions": [], + "autoname": "format:{reference_doctype} Import on {creation}", + "beta": 1, + "creation": "2019-08-04 14:16:08.318714", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "reference_doctype", + "import_type", + "download_template", + "import_file", + "payload_count", + "html_5", + "google_sheets_url", + "refresh_google_sheet", + "column_break_5", + "status", + "submit_after_import", + "mute_emails", + "template_options", + "import_warnings_section", + "template_warnings", + "import_warnings", + "section_import_preview", + "import_preview", + "import_log_section", + "show_failed_logs", + "import_log_preview" + ], + "fields": [ + { + "fieldname": "reference_doctype", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Document Type", + "options": "DocType", + "reqd": 1, + "set_only_once": 1 + }, + { + "fieldname": "import_type", + "fieldtype": "Select", + "in_list_view": 1, + "label": "Import Type", + "options": "\nInsert New Records\nUpdate Existing Records", + "reqd": 1, + "set_only_once": 1 + }, + { + "depends_on": "eval:!doc.__islocal", + "fieldname": "import_file", + "fieldtype": "Attach", + "in_list_view": 1, + "label": "Import File", + "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)" + }, + { + "fieldname": "import_preview", + "fieldtype": "HTML", + "label": "Import Preview" + }, + { + "fieldname": "section_import_preview", + "fieldtype": "Section Break", + "label": "Preview" + }, + { + "fieldname": "column_break_5", + "fieldtype": "Column Break" + }, + { + "fieldname": "template_options", + "fieldtype": "Code", + "hidden": 1, + "label": "Template Options", + "options": "JSON", + "read_only": 1 + }, + { + "fieldname": "import_log_section", + "fieldtype": "Section Break", + "label": "Import Log" + }, + { + "fieldname": "import_log_preview", + "fieldtype": "HTML", + "label": "Import Log Preview" + }, + { + "default": "Pending", + "fieldname": "status", + "fieldtype": "Select", + "hidden": 1, + "label": "Status", + "no_copy": 1, + "options": "Pending\nSuccess\nPartial Success\nError\nTimed Out", + "read_only": 1 + }, + { + "fieldname": "template_warnings", + "fieldtype": "Code", + "hidden": 1, + "label": "Template Warnings", + "options": "JSON" + }, + { + "default": "0", + "fieldname": "submit_after_import", + "fieldtype": "Check", + "label": "Submit After Import", + "set_only_once": 1 + }, + { + "fieldname": "import_warnings_section", + "fieldtype": "Section Break", + "label": "Import File Errors and Warnings" + }, + { + "fieldname": "import_warnings", + "fieldtype": "HTML", + "label": "Import Warnings" + }, + { + "depends_on": "eval:!doc.__islocal", + "fieldname": "download_template", + "fieldtype": "Button", + "label": "Download Template" + }, + { + "default": "1", + "fieldname": "mute_emails", + "fieldtype": "Check", + "label": "Don't Send Emails", + "set_only_once": 1 + }, + { + "default": "0", + "fieldname": "show_failed_logs", + "fieldtype": "Check", + "label": "Show Failed Logs" + }, + { + "depends_on": "eval:!doc.__islocal && !doc.import_file", + "fieldname": "html_5", + "fieldtype": "HTML", + "options": "
Or
" + }, + { + "depends_on": "eval:!doc.__islocal && !doc.import_file\n", + "description": "Must be a publicly accessible Google Sheets URL", + "fieldname": "google_sheets_url", + "fieldtype": "Data", + "label": "Import from Google Sheets", + "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)" + }, + { + "depends_on": "eval:doc.google_sheets_url && !doc.__unsaved && ['Success', 'Partial Success'].includes(doc.status)", + "fieldname": "refresh_google_sheet", + "fieldtype": "Button", + "label": "Refresh Google Sheet" + }, + { + "fieldname": "payload_count", + "fieldtype": "Int", + "hidden": 1, + "label": "Payload Count", + "read_only": 1 + } + ], + "hide_toolbar": 1, + "links": [], + "modified": "2023-12-15 12:45:49.452834", + "modified_by": "Administrator", + "module": "Core", + "name": "Data Import", + "naming_rule": "Expression", + "owner": "Administrator", + "permissions": [ + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, + "write": 1 + } + ], + "sort_field": "modified", + "sort_order": "DESC", + "states": [], + "track_changes": 1 +} \ No newline at end of file diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index bd6c6efe4f..f3dca2d5af 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -3,6 +3,8 @@ import os +from rq.timeouts import JobTimeoutException + import frappe from frappe import _ from frappe.core.doctype.data_import.exporter import Exporter @@ -32,11 +34,13 @@ class DataImport(Document): payload_count: DF.Int reference_doctype: DF.Link show_failed_logs: DF.Check - status: DF.Literal["Pending", "Success", "Partial Success", "Error"] + status: DF.Literal["Pending", "Success", "Partial Success", "Error", "Timed Out"] submit_after_import: DF.Check template_options: DF.Code | None template_warnings: DF.Code | None + # end: auto-generated types + def validate(self): doc_before_save = self.get_doc_before_save() if ( @@ -136,6 +140,9 @@ def start_import(data_import): try: i = Importer(data_import.reference_doctype, data_import=data_import) i.import_data() + except JobTimeoutException: + frappe.db.rollback() + data_import.db_set("status", "Timed Out") except Exception: frappe.db.rollback() data_import.db_set("status", "Error") @@ -190,6 +197,9 @@ def download_import_log(data_import_name): def get_import_status(data_import_name): import_status = {} + data_import = frappe.get_doc("Data Import", data_import_name) + import_status["status"] = data_import.status + logs = frappe.get_all( "Data Import Log", fields=["count(*) as count", "success"], diff --git a/frappe/core/doctype/data_import/data_import_list.js b/frappe/core/doctype/data_import/data_import_list.js index c054655e62..a16478cdb1 100644 --- a/frappe/core/doctype/data_import/data_import_list.js +++ b/frappe/core/doctype/data_import/data_import_list.js @@ -20,13 +20,14 @@ frappe.listview_settings["Data Import"] = { Success: "green", "In Progress": "orange", Error: "red", + "Timed Out": "orange", }; let status = doc.status; if (imports_in_progress.includes(doc.name)) { status = "In Progress"; } - if (status == "Pending") { + if (status === "Pending") { status = "Not Started"; }