From fd227d38f421a34358b8bf190ac1da1445f0d3e5 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 26 Jan 2022 11:21:30 +0530 Subject: [PATCH] feat: post model-sync patches (#15351) Ability to run a few patches after the doctype model schema is synced. Read module-level docstring of patch_handler.py for more info. --- frappe/installer.py | 17 +-- frappe/migrate.py | 13 +- frappe/modules/patch_handler.py | 129 ++++++++++++++---- frappe/patches.txt | 14 +- frappe/patches/v14_0/copy_mail_data.py | 3 - ...date_color_names_in_kanban_board_column.py | 1 - frappe/tests/test_patches.py | 15 ++ frappe/utils/boilerplate.py | 7 +- 8 files changed, 142 insertions(+), 57 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index b24dd7db41..d892ff4ddc 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -346,14 +346,15 @@ def post_install(rebuild_website=False): def set_all_patches_as_completed(app): - patch_path = os.path.join(frappe.get_pymodule_path(app), "patches.txt") - if os.path.exists(patch_path): - for patch in frappe.get_file_items(patch_path): - frappe.get_doc({ - "doctype": "Patch Log", - "patch": patch - }).insert(ignore_permissions=True) - frappe.db.commit() + from frappe.modules.patch_handler import get_patches_from_app + + patches = get_patches_from_app(app) + for patch in patches: + frappe.get_doc({ + "doctype": "Patch Log", + "patch": patch + }).insert(ignore_permissions=True) + frappe.db.commit() def init_singles(): diff --git a/frappe/migrate.py b/frappe/migrate.py index 6abc38796f..d13fe858f7 100644 --- a/frappe/migrate.py +++ b/frappe/migrate.py @@ -19,6 +19,8 @@ from frappe.modules.utils import sync_customizations from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs from frappe.search.website_search import build_index_for_all_routes from frappe.database.schema import add_column +from frappe.modules.patch_handler import PatchType + def migrate(verbose=True, skip_failing=False, skip_search_index=False): @@ -59,16 +61,13 @@ Otherwise, check the server logs and ensure that all the required services are r clear_global_cache() - #run before_migrate hooks for app in frappe.get_installed_apps(): for fn in frappe.get_hooks('before_migrate', app_name=app): frappe.get_attr(fn)() - # run patches - frappe.modules.patch_handler.run_all(skip_failing) - - # sync + frappe.modules.patch_handler.run_all(skip_failing=skip_failing, patch_type=PatchType.pre_model_sync) frappe.model.sync.sync_all() + frappe.modules.patch_handler.run_all(skip_failing=skip_failing, patch_type=PatchType.post_model_sync) frappe.translate.clear_cache() sync_jobs() sync_fixtures() @@ -78,18 +77,16 @@ Otherwise, check the server logs and ensure that all the required services are r frappe.get_doc('Portal Settings', 'Portal Settings').sync_menu() - # syncs statics + # syncs static files clear_website_cache() # updating installed applications data frappe.get_single('Installed Applications').update_versions() - #run after_migrate hooks for app in frappe.get_installed_apps(): for fn in frappe.get_hooks('after_migrate', app_name=app): frappe.get_attr(fn)() - # build web_routes index if not skip_search_index: # Run this last as it updates the current session print('Building search index for {}'.format(frappe.local.site)) diff --git a/frappe/modules/patch_handler.py b/frappe/modules/patch_handler.py index 8dfb27c0b8..75d4972152 100644 --- a/frappe/modules/patch_handler.py +++ b/frappe/modules/patch_handler.py @@ -1,37 +1,76 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -""" - Execute Patch Files +""" Patch Handler. + +This file manages execution of manaully written patches. Patches are script +that apply changes in database schema or data to accomodate for changes in the +code. + +Ways to specify patches: + +1. patches.txt file specifies patches that run before doctype schema +migration. Each line represents one patch (old format). +2. patches.txt can alternatively also separate pre and post model sync +patches by using INI like file format: + ```patches.txt + [pre_model_sync] + app.module.patch1 + app.module.patch2 - To run directly - python lib/wnf.py patch patch1, patch2 etc - python lib/wnf.py patch -f patch1, patch2 etc + [post_model_sync] + app.module.patch3 + ``` - where patch1, patch2 is module name + When different sections are specified patches are executed in this order: + 1. Run pre_model_sync patches + 2. Reload/resync all doctype schema + 3. Run post_model_sync patches + + Hence any patch that just needs to modify data but doesn't depend on + old schema should be added to post_model_sync section of file. + +3. simple python commands can be added by starting line with `execute:` +`execute:` example: `execute:print("hello world")` """ -import frappe, frappe.permissions, time -class PatchError(Exception): pass +import configparser +import time +from enum import Enum +from typing import List, Optional + +import frappe -def run_all(skip_failing=False): + +class PatchError(Exception): + pass + + +class PatchType(Enum): + pre_model_sync = "pre_model_sync" + post_model_sync = "post_model_sync" + + +def run_all(skip_failing: bool = False, patch_type: Optional[PatchType] = None) -> None: """run all pending patches""" - executed = [p[0] for p in frappe.db.sql("""select patch from `tabPatch Log`""")] + executed = set(frappe.get_all("Patch Log", fields="patch", pluck="patch")) frappe.flags.final_patches = [] def run_patch(patch): try: if not run_single(patchmodule = patch): - log(patch + ': failed: STOPPED') + print(patch + ': failed: STOPPED') raise PatchError(patch) except Exception: if not skip_failing: raise else: - log('Failed to execute patch') + print('Failed to execute patch') + + patches = get_all_patches(patch_type=patch_type) - for patch in get_all_patches(): + for patch in patches: if patch and (patch not in executed): run_patch(patch) @@ -40,18 +79,54 @@ def run_all(skip_failing=False): patch = patch.replace('finally:', '') run_patch(patch) -def get_all_patches(): +def get_all_patches(patch_type: Optional[PatchType] = None) -> List[str]: + + if patch_type and not isinstance(patch_type, PatchType): + frappe.throw(f"Unsupported patch type specified: {patch_type}") + patches = [] for app in frappe.get_installed_apps(): - if app == "shopping_cart": - continue - # 3-to-4 fix - if app=="webnotes": - app="frappe" - patches.extend(frappe.get_file_items(frappe.get_pymodule_path(app, "patches.txt"))) + patches.extend(get_patches_from_app(app, patch_type=patch_type)) return patches +def get_patches_from_app(app: str, patch_type: Optional[PatchType] = None) -> List[str]: + """ Get patches from an app's patches.txt + + patches.txt can be: + 1. ini like file with section for different patch_type + 2. plain text file with each line representing a patch. + """ + + patches_txt = frappe.get_pymodule_path(app, "patches.txt") + + try: + # Attempt to parse as ini file with pre/post patches + # allow_no_value: patches are not key value pairs + # delimiters = '\n' to avoid treating default `:` and `=` in execute as k:v delimiter + parser = configparser.ConfigParser(allow_no_value=True, delimiters="\n") + # preserve case + parser.optionxform = str + parser.read(patches_txt) + + + if not patch_type: + return [patch for patch in parser[PatchType.pre_model_sync.value]] + \ + [patch for patch in parser[PatchType.post_model_sync.value]] + + if patch_type.value in parser.sections(): + return [patch for patch in parser[patch_type.value]] + else: + frappe.throw(frappe._("Patch type {} not found in patches.txt").format(patch_type)) + + except configparser.MissingSectionHeaderError: + # treat as old format with each line representing a single patch + # backward compatbility with old patches.txt format + if not patch_type or patch_type == PatchType.pre_model_sync: + return frappe.get_file_items(patches_txt) + + return [] + def reload_doc(args): import frappe.modules run_single(method = frappe.modules.reload_doc, methodargs = args) @@ -73,7 +148,7 @@ def execute_patch(patchmodule, method=None, methodargs=None): frappe.db.begin() start_time = time.time() try: - log('Executing {patch} in {site} ({db})'.format(patch=patchmodule or str(methodargs), + print('Executing {patch} in {site} ({db})'.format(patch=patchmodule or str(methodargs), site=frappe.local.site, db=frappe.db.cur_db_name)) if patchmodule: if patchmodule.startswith("finally:"): @@ -96,7 +171,7 @@ def execute_patch(patchmodule, method=None, methodargs=None): frappe.db.commit() end_time = time.time() block_user(False) - log('Success: Done in {time}s'.format(time = round(end_time - start_time, 3))) + print('Success: Done in {time}s'.format(time = round(end_time - start_time, 3))) return True @@ -109,10 +184,7 @@ def executed(patchmodule): if patchmodule.startswith('finally:'): # patches are saved without the finally: tag patchmodule = patchmodule.replace('finally:', '') - done = frappe.db.get_value("Patch Log", {"patch": patchmodule}) - # if done: - # print "Patch %s already executed in %s" % (patchmodule, frappe.db.cur_db_name) - return done + return frappe.db.get_value("Patch Log", {"patch": patchmodule}) def block_user(block, msg=None): """stop/start execution till patch is run""" @@ -128,6 +200,3 @@ def check_session_stopped(): if frappe.db.get_global("__session_status")=='stop': frappe.msgprint(frappe.db.get_global("__session_status_message")) raise frappe.SessionStopped('Session Stopped') - -def log(msg): - print (msg) diff --git a/frappe/patches.txt b/frappe/patches.txt index 16ae349941..c393b456e3 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -1,3 +1,4 @@ +[pre_model_sync] frappe.patches.v12_0.remove_deprecated_fields_from_doctype #3 execute:frappe.utils.global_search.setup_global_search_table() execute:frappe.reload_doc('core', 'doctype', 'doctype_action', force=True) #2019-09-23 @@ -87,7 +88,6 @@ frappe.patches.v11_0.set_missing_creation_and_modified_value_for_user_permission frappe.patches.v11_0.set_default_letter_head_source frappe.patches.v12_0.set_primary_key_in_series execute:frappe.delete_doc("Page", "modules", ignore_missing=True) -frappe.patches.v11_0.set_default_letter_head_source frappe.patches.v12_0.setup_comments_from_communications frappe.patches.v12_0.replace_null_values_in_tables frappe.patches.v12_0.reset_home_settings @@ -123,7 +123,7 @@ frappe.patches.v12_0.remove_parent_and_parenttype_from_print_formats frappe.patches.v12_0.remove_example_email_thread_notify execute:from frappe.desk.page.setup_wizard.install_fixtures import update_genders;update_genders() frappe.patches.v12_0.set_correct_url_in_files -execute:frappe.reload_doc('core', 'doctype', 'doctype', force=True) +execute:frappe.reload_doc('core', 'doctype', 'doctype') execute:frappe.reload_doc('custom', 'doctype', 'property_setter') frappe.patches.v13_0.remove_invalid_options_for_data_fields frappe.patches.v13_0.website_theme_custom_scss @@ -184,12 +184,14 @@ frappe.patches.v13_0.queryreport_columns frappe.patches.v13_0.jinja_hook frappe.patches.v13_0.update_notification_channel_if_empty frappe.patches.v13_0.set_first_day_of_the_week -frappe.patches.v14_0.drop_data_import_legacy frappe.patches.v14_0.rename_cancelled_documents -frappe.patches.v14_0.copy_mail_data #08.03.21 frappe.patches.v14_0.update_workspace2 # 20.09.2021 +frappe.patches.v14_0.save_ratings_in_fraction #23-12-2021 +frappe.patches.v14_0.transform_todo_schema + +[post_model_sync] +frappe.patches.v14_0.drop_data_import_legacy +frappe.patches.v14_0.copy_mail_data #08.03.21 frappe.patches.v14_0.update_github_endpoints #08-11-2021 frappe.patches.v14_0.remove_db_aggregation -frappe.patches.v14_0.save_ratings_in_fraction #23-12-2021 frappe.patches.v14_0.update_color_names_in_kanban_board_column -frappe.patches.v14_0.transform_todo_schema diff --git a/frappe/patches/v14_0/copy_mail_data.py b/frappe/patches/v14_0/copy_mail_data.py index d3a5c59209..8ef9cfaf1f 100644 --- a/frappe/patches/v14_0/copy_mail_data.py +++ b/frappe/patches/v14_0/copy_mail_data.py @@ -3,9 +3,6 @@ import frappe def execute(): - frappe.reload_doc("email", "doctype", "imap_folder") - frappe.reload_doc("email", "doctype", "email_account") - # patch for all Email Account with the flag use_imap for email_account in frappe.get_list("Email Account", filters={"enable_incoming": 1, "use_imap": 1}): # get all data from Email Account diff --git a/frappe/patches/v14_0/update_color_names_in_kanban_board_column.py b/frappe/patches/v14_0/update_color_names_in_kanban_board_column.py index ea8a10e43a..ff03604754 100644 --- a/frappe/patches/v14_0/update_color_names_in_kanban_board_column.py +++ b/frappe/patches/v14_0/update_color_names_in_kanban_board_column.py @@ -5,7 +5,6 @@ from __future__ import unicode_literals import frappe def execute(): - frappe.reload_doc("desk", "doctype", "kanban_board_column") indicator_map = { 'blue': 'Blue', 'orange': 'Orange', diff --git a/frappe/tests/test_patches.py b/frappe/tests/test_patches.py index 7f4efc700c..8e6d97bd0d 100644 --- a/frappe/tests/test_patches.py +++ b/frappe/tests/test_patches.py @@ -15,3 +15,18 @@ class TestPatches(unittest.TestCase): self.assertTrue(frappe.get_attr(patchmodule.split()[0] + ".execute")) frappe.flags.in_install = False + + def test_get_patch_list(self): + pre = patch_handler.get_patches_from_app("frappe", patch_handler.PatchType.pre_model_sync) + post = patch_handler.get_patches_from_app("frappe", patch_handler.PatchType.post_model_sync) + all_patches = patch_handler.get_patches_from_app("frappe") + self.assertGreater(len(pre), 0) + self.assertGreater(len(post), 0) + + self.assertEqual(len(all_patches), len(pre) + len(post)) + + def test_all_patches_are_marked_completed(self): + all_patches = patch_handler.get_patches_from_app("frappe") + finished_patches = frappe.db.count("Patch Log") + + self.assertGreaterEqual(finished_patches, len(all_patches)) diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index 6c405ce467..6fa2aee0d4 100755 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -1,6 +1,11 @@ # Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -import frappe, os, re, git + +import git +import os +import re + +import frappe from frappe.utils import touch_file, cstr def make_boilerplate(dest, app_name, no_git=False):