From b1d2e24892d3bf28034f00fcc4bf8c829b932530 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 6 May 2021 19:18:23 +0530 Subject: [PATCH 01/19] refactor(minor): Move logging code in seperate function --- frappe/app.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/frappe/app.py b/frappe/app.py index a72f343532..64befdf531 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -99,17 +99,7 @@ def application(request): frappe.monitor.stop(response) frappe.recorder.dump() - if hasattr(frappe.local, 'conf') and frappe.local.conf.enable_frappe_logger: - frappe.logger("frappe.web", allow_site=frappe.local.site).info({ - "site": get_site_name(request.host), - "remote_addr": getattr(request, "remote_addr", "NOTFOUND"), - "base_url": getattr(request, "base_url", "NOTFOUND"), - "full_path": getattr(request, "full_path", "NOTFOUND"), - "method": getattr(request, "method", "NOTFOUND"), - "scheme": getattr(request, "scheme", "NOTFOUND"), - "http_status_code": getattr(response, "status_code", "NOTFOUND") - }) - + log_request(request, response) process_response(response) frappe.destroy() @@ -137,6 +127,19 @@ def init_request(request): if request.method != "OPTIONS": frappe.local.http_request = frappe.auth.HTTPRequest() +def log_request(request, response): + if hasattr(frappe.local, 'conf') and frappe.local.conf.enable_frappe_logger: + frappe.logger("frappe.web", allow_site=frappe.local.site).info({ + "site": get_site_name(request.host), + "remote_addr": getattr(request, "remote_addr", "NOTFOUND"), + "base_url": getattr(request, "base_url", "NOTFOUND"), + "full_path": getattr(request, "full_path", "NOTFOUND"), + "method": getattr(request, "method", "NOTFOUND"), + "scheme": getattr(request, "scheme", "NOTFOUND"), + "http_status_code": getattr(response, "status_code", "NOTFOUND") + }) + + def process_response(response): if not response: return From 7a9611a6c532f9780bff649f30c1b618af26bba1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 6 May 2021 19:20:56 +0530 Subject: [PATCH 02/19] fix: Py3 Compatible code Avoid "DeprecationWarning: invalid escape sequence \W" by using raw strings instead, If avoided for longer, these lines will throw a SyntaxError in later Python versions --- frappe/core/doctype/doctype/doctype.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 3588cc553a..4a4001610c 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -622,12 +622,12 @@ class DocType(Document): flags = {"flags": re.ASCII} if six.PY3 else {} # a DocType name should not start or end with an empty space - if re.search("^[ \t\n\r]+|[ \t\n\r]+$", name, **flags): + if re.search(r"^[ \t\n\r]+|[ \t\n\r]+$", name, **flags): frappe.throw(_("DocType's name should not start or end with whitespace"), frappe.NameError) # a DocType's name should not start with a number or underscore # and should only contain letters, numbers and underscore - if not re.match("^(?![\W])[^\d_\s][\w ]+$", name, **flags): + if not re.match(r"^(?![\W])[^\d_\s][\w ]+$", name, **flags): frappe.throw(_("DocType's name should start with a letter and it can only consist of letters, numbers, spaces and underscores"), frappe.NameError) validate_route_conflict(self.doctype, self.name) @@ -915,7 +915,7 @@ def validate_fields(meta): for field in depends_on_fields: depends_on = docfield.get(field, None) if depends_on and ("=" in depends_on) and \ - re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", depends_on): + re.match(r"""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", depends_on): frappe.throw(_("Invalid {0} condition").format(frappe.unscrub(field)), frappe.ValidationError) def check_table_multiselect_option(docfield): From 393696fa4aef2e64dd0ff7c5fe1ac2d7b6a973f6 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 6 May 2021 19:22:31 +0530 Subject: [PATCH 03/19] chore: Drop dead code * Don't create task-logs folder during site creation * Remove old unused async code for tracking tasks --- frappe/hooks.py | 1 - frappe/installer.py | 1 - frappe/realtime.py | 82 +++------------------------------------------ 3 files changed, 4 insertions(+), 80 deletions(-) diff --git a/frappe/hooks.py b/frappe/hooks.py index 1c78d47755..b999d2cd95 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -226,7 +226,6 @@ scheduler_events = { "frappe.desk.doctype.event.event.send_event_digest", "frappe.sessions.clear_expired_sessions", "frappe.email.doctype.notification.notification.trigger_daily_alerts", - "frappe.realtime.remove_old_task_logs", "frappe.utils.scheduler.restrict_scheduler_events_if_dormant", "frappe.email.doctype.auto_email_report.auto_email_report.send_daily", "frappe.website.doctype.personal_data_deletion_request.personal_data_deletion_request.remove_unverified_record", diff --git a/frappe/installer.py b/frappe/installer.py index 0cd5b136ae..e3fd66228d 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -397,7 +397,6 @@ def make_site_dirs(): os.path.join(site_public_path, 'files'), os.path.join(site_private_path, 'files'), os.path.join(frappe.local.site_path, 'logs'), - os.path.join(frappe.local.site_path, 'task-logs')): if not os.path.exists(dir_path): os.makedirs(dir_path) locks_dir = frappe.get_site_path('locks') diff --git a/frappe/realtime.py b/frappe/realtime.py index f546703e58..6c812e8868 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -1,56 +1,23 @@ -# -*- coding: utf-8 -*- # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt -from __future__ import unicode_literals - - import frappe import os -import time import redis -from io import FileIO -from frappe.utils import get_site_path -from frappe import conf -END_LINE = '' -TASK_LOG_MAX_AGE = 86400 # 1 day in seconds redis_server = None + @frappe.whitelist() def get_pending_tasks_for_doc(doctype, docname): return frappe.db.sql_list("select name from `tabAsync Task` where status in ('Queued', 'Running') and reference_doctype=%s and reference_name=%s", (doctype, docname)) -def set_task_status(task_id, status, response=None): - if not response: - response = {} - response.update({ - "status": status, - "task_id": task_id - }) - emit_via_redis("task_status_change", response, room="task:" + task_id) - - -def remove_old_task_logs(): - logs_path = get_site_path('task-logs') - - def full_path(_file): - return os.path.join(logs_path, _file) - - files_to_remove = [full_path(_file) for _file in os.listdir(logs_path)] - files_to_remove = [_file for _file in files_to_remove if is_file_old(_file) and os.path.isfile(_file)] - for _file in files_to_remove: - os.remove(_file) - - -def is_file_old(file_path): - return ((time.time() - os.stat(file_path).st_mtime) > TASK_LOG_MAX_AGE) - def publish_progress(percent, title=None, doctype=None, docname=None, description=None): publish_realtime('progress', {'percent': percent, 'title': title, 'description': description}, user=frappe.session.user, doctype=doctype, docname=docname) + def publish_realtime(event=None, message=None, room=None, user=None, doctype=None, docname=None, task_id=None, after_commit=False): @@ -103,6 +70,7 @@ def publish_realtime(event=None, message=None, room=None, else: emit_via_redis(event, message, room) + def emit_via_redis(event, message, room): """Publish real-time updates via redis @@ -117,57 +85,17 @@ def emit_via_redis(event, message, room): # print(frappe.get_traceback()) pass -def put_log(line_no, line, task_id=None): - r = get_redis_server() - if not task_id: - task_id = frappe.local.task_id - task_progress_room = get_task_progress_room(task_id) - task_log_key = "task_log:" + task_id - publish_realtime('task_progress', { - "message": { - "lines": {line_no: line} - }, - "task_id": task_id - }, room=task_progress_room) - r.hset(task_log_key, line_no, line) - r.expire(task_log_key, 3600) - def get_redis_server(): """returns redis_socketio connection.""" global redis_server if not redis_server: from redis import Redis - redis_server = Redis.from_url(conf.get("redis_socketio") + redis_server = Redis.from_url(frappe.conf.redis_socketio or "redis://localhost:12311") return redis_server -class FileAndRedisStream(FileIO): - def __init__(self, *args, **kwargs): - ret = super(FileAndRedisStream, self).__init__(*args, **kwargs) - self.count = 0 - return ret - - def write(self, data): - ret = super(FileAndRedisStream, self).write(data) - if frappe.local.task_id: - put_log(self.count, data, task_id=frappe.local.task_id) - self.count += 1 - return ret - - -def get_std_streams(task_id): - stdout = FileAndRedisStream(get_task_log_file_path(task_id, 'stdout'), 'w') - # stderr = FileAndRedisStream(get_task_log_file_path(task_id, 'stderr'), 'w') - return stdout, stdout - - -def get_task_log_file_path(task_id, stream_type): - logs_dir = frappe.utils.get_site_path('task-logs') - return os.path.join(logs_dir, task_id + '.' + stream_type) - - @frappe.whitelist(allow_guest=True) def can_subscribe_doc(doctype, docname): if os.environ.get('CI'): @@ -201,9 +129,7 @@ def get_site_room(): def get_task_progress_room(task_id): return "".join([frappe.local.site, ":task_progress:", task_id]) -# frappe.chat def get_chat_room(room): room = ''.join([frappe.local.site, ":room:", room]) return room -# end frappe.chat room \ No newline at end of file From 411075424eef9794e2ea98ee1a4c1e0a215f7148 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 6 May 2021 19:31:15 +0530 Subject: [PATCH 04/19] refactor(minor): Simplify site directory generation --- frappe/installer.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index e3fd66228d..d7d885d60e 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -390,18 +390,16 @@ def get_conf_params(db_name=None, db_password=None): def make_site_dirs(): - site_public_path = os.path.join(frappe.local.site_path, 'public') - site_private_path = os.path.join(frappe.local.site_path, 'private') - for dir_path in ( - os.path.join(site_private_path, 'backups'), - os.path.join(site_public_path, 'files'), - os.path.join(site_private_path, 'files'), - os.path.join(frappe.local.site_path, 'logs'), - if not os.path.exists(dir_path): - os.makedirs(dir_path) - locks_dir = frappe.get_site_path('locks') - if not os.path.exists(locks_dir): - os.makedirs(locks_dir) + for dir_path in [ + os.path.join("public", "files"), + os.path.join("private", "backups"), + os.path.join("private", "files"), + "error-snapshots", + "locks", + "logs", + ]: + path = frappe.get_site_path(dir_path) + os.makedirs(path, exist_ok=True) def add_module_defs(app): From 71fe80430727f2b619a0f6a1720f72e6916a9086 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 7 May 2021 12:11:37 +0530 Subject: [PATCH 05/19] fix(cli): Handle incorrect site names Show "Site {site} does not exist!" and exit with code 1 instead of traceback --- frappe/commands/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/commands/__init__.py b/frappe/commands/__init__.py index 61ee62d352..e521acc9ad 100644 --- a/frappe/commands/__init__.py +++ b/frappe/commands/__init__.py @@ -28,6 +28,10 @@ def pass_context(f): except frappe.exceptions.SiteNotSpecifiedError as e: click.secho(str(e), fg='yellow') sys.exit(1) + except frappe.exceptions.IncorrectSitePath: + site = ctx.obj.get("sites", "")[0] + click.secho(f'Site {site} does not exist!', fg='yellow') + sys.exit(1) if profile: pr.disable() From 1752e5b0e52f9cd1366306fe2cd1df7eda2753ab Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 7 May 2021 12:14:00 +0530 Subject: [PATCH 06/19] feat(cli): Show progress bar for website search index building --- frappe/search/full_text_search.py | 5 +++-- frappe/search/website_search.py | 35 ++++++++++++++++++++----------- frappe/utils/__init__.py | 21 +++++++++++++++---- frappe/website/context.py | 2 +- 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/frappe/search/full_text_search.py b/frappe/search/full_text_search.py index ecb018dbb4..9dd181323e 100644 --- a/frappe/search/full_text_search.py +++ b/frappe/search/full_text_search.py @@ -1,8 +1,8 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt -from __future__ import unicode_literals import frappe +from frappe.utils import update_progress_bar from whoosh.index import create_in, open_dir, EmptyIndexError from whoosh.fields import TEXT, ID, Schema @@ -95,9 +95,10 @@ class FullTextSearch: ix = self.create_index() writer = ix.writer() - for document in self.documents: + for i, document in enumerate(self.documents): if document: writer.add_document(**document) + update_progress_bar("Building Index", i, len(self.documents)) writer.commit(optimize=True) diff --git a/frappe/search/website_search.py b/frappe/search/website_search.py index 87ef4b08ad..3b2c51ef41 100644 --- a/frappe/search/website_search.py +++ b/frappe/search/website_search.py @@ -1,15 +1,16 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt -from __future__ import unicode_literals -import frappe -from bs4 import BeautifulSoup -from whoosh.fields import TEXT, ID, Schema -from frappe.search.full_text_search import FullTextSearch -from frappe.website.render import render_page -from frappe.utils import set_request import os +from bs4 import BeautifulSoup +from whoosh.fields import ID, TEXT, Schema + +import frappe +from frappe.search.full_text_search import FullTextSearch +from frappe.utils import set_request, update_progress_bar +from frappe.website.render import render_page + INDEX_NAME = "web_routes" class WebsiteSearch(FullTextSearch): @@ -30,11 +31,21 @@ class WebsiteSearch(FullTextSearch): Returns: self (object): FullTextSearch Instance """ - routes = get_static_pages_from_all_apps() - routes += slugs_with_web_view() - documents = [self.get_document_to_index(route) for route in routes] - return documents + if getattr(self, "_items_to_index", False): + return self._items_to_index + + routes = get_static_pages_from_all_apps() + slugs_with_web_view() + + self._items_to_index = [] + + for i, route in enumerate(routes): + update_progress_bar(f"Retrieving Routes", i, len(routes)) + self._items_to_index += [self.get_document_to_index(route)] + + print() + + return self.get_items_to_index() def get_document_to_index(self, route): """Render a page and parse it using BeautifulSoup @@ -114,4 +125,4 @@ def remove_document_from_index(path): def build_index_for_all_routes(): ws = WebsiteSearch(INDEX_NAME) - return ws.build() \ No newline at end of file + return ws.build() diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 1da4cd3bae..ccd5283b1a 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -225,14 +225,17 @@ def get_gravatar(email): return gravatar_url -def get_traceback(): +def get_traceback() -> str: """ Returns the traceback of the Exception """ exc_type, exc_value, exc_tb = sys.exc_info() + + if not any(exc_type, exc_value, exc_tb): + return "" + trace_list = traceback.format_exception(exc_type, exc_value, exc_tb) - body = "".join(cstr(t) for t in trace_list) - return body + return "".join(cstr(t) for t in trace_list) def log(event, details): frappe.logger().info(details) @@ -439,6 +442,16 @@ def call_hook_method(hook, *args, **kwargs): return out +def is_cli() -> bool: + """Returns True if current instance is being run via a terminal + """ + invoked_from_terminal = False + try: + invoked_from_terminal = bool(os.get_terminal_size()) + except Exception: + invoked_from_terminal = sys.stdin.isatty() + return invoked_from_terminal + def update_progress_bar(txt, i, l): if os.environ.get("CI"): if i == 0: @@ -448,7 +461,7 @@ def update_progress_bar(txt, i, l): sys.stdout.flush() return - if not getattr(frappe.local, 'request', None): + if not getattr(frappe.local, 'request', None) or is_cli(): lt = len(txt) try: col = 40 if os.get_terminal_size().columns > 80 else 20 diff --git a/frappe/website/context.py b/frappe/website/context.py index b39770dcd4..401912d407 100644 --- a/frappe/website/context.py +++ b/frappe/website/context.py @@ -61,7 +61,7 @@ def update_controller_context(context, controller): except (frappe.PermissionError, frappe.PageDoesNotExistError, frappe.Redirect): raise except: - if not frappe.flags.in_migrate: + if not any(frappe.flags.in_migrate, frappe.flags.in_website_search_build): frappe.errprint(frappe.utils.get_traceback()) if hasattr(module, "get_children"): From 855efcfd59034224cb16f9739c72af616af64a44 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 7 May 2021 12:15:32 +0530 Subject: [PATCH 07/19] style: Trim whitespace and remove unnecessary parenthesis --- frappe/utils/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index ccd5283b1a..c9123a4ca6 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -161,7 +161,7 @@ def validate_url(txt, throw=False, valid_schemes=None): Parameters: throw (`bool`): throws a validationError if URL is not valid - valid_schemes (`str` or `list`): if provided checks the given URL's scheme against this + valid_schemes (`str` or `list`): if provided checks the given URL's scheme against this Returns: bool: if `txt` represents a valid URL @@ -428,7 +428,7 @@ def get_test_client(): return Client(application) def get_hook_method(hook_name, fallback=None): - method = (frappe.get_hooks().get(hook_name)) + method = frappe.get_hooks().get(hook_name) if method: method = frappe.get_attr(method[0]) return method From d43046a4cb9a0fef05b27e9238bc7d30efacc43e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 7 May 2021 13:24:48 +0530 Subject: [PATCH 08/19] fix: any takes an iterable not multi args --- frappe/utils/__init__.py | 2 +- frappe/website/context.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index c9123a4ca6..43a7b82c16 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -231,7 +231,7 @@ def get_traceback() -> str: """ exc_type, exc_value, exc_tb = sys.exc_info() - if not any(exc_type, exc_value, exc_tb): + if not any([exc_type, exc_value, exc_tb]): return "" trace_list = traceback.format_exception(exc_type, exc_value, exc_tb) diff --git a/frappe/website/context.py b/frappe/website/context.py index 401912d407..c898d39869 100644 --- a/frappe/website/context.py +++ b/frappe/website/context.py @@ -61,7 +61,7 @@ def update_controller_context(context, controller): except (frappe.PermissionError, frappe.PageDoesNotExistError, frappe.Redirect): raise except: - if not any(frappe.flags.in_migrate, frappe.flags.in_website_search_build): + if not any([frappe.flags.in_migrate, frappe.flags.in_website_search_build]): frappe.errprint(frappe.utils.get_traceback()) if hasattr(module, "get_children"): From b3af9f0c72835bf21d9e99e32b4c803f7e03166b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 7 May 2021 17:59:20 +0530 Subject: [PATCH 09/19] fix: Use raw strings for strings with \ Avoid DeprecationWarning which will turn into SyntaxError in later Python versions --- frappe/core/doctype/data_export/exporter.py | 2 +- frappe/core/doctype/doctype/test_doctype.py | 2 +- frappe/email/receive.py | 4 ++-- frappe/utils/boilerplate.py | 2 +- frappe/utils/redis_wrapper.py | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/data_export/exporter.py b/frappe/core/doctype/data_export/exporter.py index bec8cde7ea..5cbad9a380 100644 --- a/frappe/core/doctype/data_export/exporter.py +++ b/frappe/core/doctype/data_export/exporter.py @@ -282,7 +282,7 @@ class DataExporter: try: sflags = self.docs_to_export.get("flags", "I,U").upper() flags = 0 - for a in re.split('\W+',sflags): + for a in re.split(r'\W+',sflags): flags = flags | reflags.get(a,0) c = re.compile(names, flags) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index bfa9d0ec8a..b787f677e4 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -92,7 +92,7 @@ class TestDocType(unittest.TestCase): fields=["parent", "depends_on", "collapsible_depends_on", "mandatory_depends_on",\ "read_only_depends_on", "fieldname", "fieldtype"]) - pattern = """[\w\.:_]+\s*={1}\s*[\w\.@'"]+""" + pattern = r"""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""" for field in docfields: for depends_on in ["depends_on", "collapsible_depends_on", "mandatory_depends_on", "read_only_depends_on"]: condition = field.get(depends_on) diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 949da4a343..6d60007cdb 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -284,7 +284,7 @@ class EmailServer: flags = [] for flag in imaplib.ParseFlags(flag_string) or []: - pattern = re.compile("\w+") + pattern = re.compile(r"\w+") match = re.search(pattern, frappe.as_unicode(flag)) flags.append(match.group(0)) @@ -555,7 +555,7 @@ class Email: def get_thread_id(self): """Extract thread ID from `[]`""" - l = re.findall('(?<=\[)[\w/-]+', self.subject) + l = re.findall(r'(?<=\[)[\w/-]+', self.subject) return l and l[0] or None diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index ba20562544..a666597ce5 100755 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -42,7 +42,7 @@ def make_boilerplate(dest, app_name): if hook_key=="app_name" and hook_val.lower().replace(" ", "_") != hook_val: print("App Name must be all lowercase and without spaces") hook_val = "" - elif hook_key=="app_title" and not re.match("^(?![\W])[^\d_\s][\w -]+$", hook_val, re.UNICODE): + elif hook_key=="app_title" and not re.match(r"^(?![\W])[^\d_\s][\w -]+$", hook_val, re.UNICODE): print("App Title should start with a letter and it can only consist of letters, numbers, spaces and underscores") hook_val = "" diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index 20bbb283a3..ecd97a4a18 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -98,7 +98,7 @@ class RedisWrapper(redis.Redis): return self.keys(key) except redis.exceptions.ConnectionError: - regex = re.compile(cstr(key).replace("|", "\|").replace("*", "[\w]*")) + regex = re.compile(cstr(key).replace("|", r"\|").replace("*", r"[\w]*")) return [k for k in list(frappe.local.cache) if regex.match(cstr(k))] def delete_keys(self, key): From 134b2dcd5c938ba555004d3a1a8c995f0dc0bf2d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 11 May 2021 13:02:45 +0530 Subject: [PATCH 10/19] chore: Remove f prefix on plain string --- frappe/search/website_search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/search/website_search.py b/frappe/search/website_search.py index 3b2c51ef41..452ea2a427 100644 --- a/frappe/search/website_search.py +++ b/frappe/search/website_search.py @@ -40,7 +40,7 @@ class WebsiteSearch(FullTextSearch): self._items_to_index = [] for i, route in enumerate(routes): - update_progress_bar(f"Retrieving Routes", i, len(routes)) + update_progress_bar("Retrieving Routes", i, len(routes)) self._items_to_index += [self.get_document_to_index(route)] print() From b5a121a1cbc1d037c84c59a314bdb1cb832ec8db Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 11 May 2021 15:19:35 +0530 Subject: [PATCH 11/19] fix: Drop compatability code * Use raw text for regex patterns to avoid Deprecation warnings --- frappe/build.py | 27 +++++++++------------ frappe/core/doctype/data_import/importer.py | 2 +- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index 5b541b4852..05a4bea7e6 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -1,14 +1,11 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt -from __future__ import print_function, unicode_literals - import os import re import json import shutil -import warnings -import tempfile +from tempfile import mkdtemp from distutils.spawn import find_executable import frappe @@ -16,8 +13,7 @@ from frappe.utils.minify import JavascriptMinify import click import psutil -from six import iteritems, text_type -from six.moves.urllib.parse import urlparse +from urllib.parse import urlparse timestamps = {} @@ -75,8 +71,8 @@ def get_assets_link(frappe_head): from requests import head tag = getoutput( - "cd ../apps/frappe && git show-ref --tags -d | grep %s | sed -e 's,.*" - " refs/tags/,,' -e 's/\^{}//'" + r"cd ../apps/frappe && git show-ref --tags -d | grep %s | sed -e 's,.*" + r" refs/tags/,,' -e 's/\^{}//'" % frappe_head ) @@ -99,7 +95,6 @@ def download_frappe_assets(verbose=True): """ from simple_chalk import green from subprocess import getoutput - from tempfile import mkdtemp assets_setup = False frappe_head = getoutput("cd ../apps/frappe && git rev-parse HEAD") @@ -166,7 +161,7 @@ def symlink(target, link_name, overwrite=False): # Create link to target with temporary filename while True: - temp_link_name = tempfile.mktemp(dir=link_dir) + temp_link_name = mktemp(dir=link_dir) # os.* functions mimic as closely as possible system functions # The POSIX symlink() returns EEXIST if link_name already exists @@ -348,7 +343,7 @@ def get_build_maps(): if os.path.exists(path): with open(path) as f: try: - for target, sources in iteritems(json.loads(f.read())): + for target, sources in (json.loads(f.read() or "{}")).items(): # update app path source_paths = [] for source in sources: @@ -381,7 +376,7 @@ def pack(target, sources, no_compress, verbose): timestamps[f] = os.path.getmtime(f) try: with open(f, "r") as sourcefile: - data = text_type(sourcefile.read(), "utf-8", errors="ignore") + data = str(sourcefile.read(), "utf-8", errors="ignore") extn = f.rsplit(".", 1)[1] @@ -396,7 +391,7 @@ def pack(target, sources, no_compress, verbose): jsm.minify(tmpin, tmpout) minified = tmpout.getvalue() if minified: - outtxt += text_type(minified or "", "utf-8").strip("\n") + ";" + outtxt += str(minified or "", "utf-8").strip("\n") + ";" if verbose: print("{0}: {1}k".format(f, int(len(minified) / 1024))) @@ -426,16 +421,16 @@ def html_to_js_template(path, content): def scrub_html_template(content): """Returns HTML content with removed whitespace and comments""" # remove whitespace to a single space - content = re.sub("\s+", " ", content) + content = re.sub(r"\s+", " ", content) # strip comments - content = re.sub("()", "", content) + content = re.sub(r"()", "", content) return content.replace("'", "\'") def files_dirty(): - for target, sources in iteritems(get_build_maps()): + for target, sources in get_build_maps().items(): for f in sources: if ":" in f: f, suffix = f.split(":") diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 720fe1dda7..0eb1752d78 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -641,7 +641,7 @@ class Row: return elif df.fieldtype == "Duration": import re - is_valid_duration = re.match("^(?:(\d+d)?((^|\s)\d+h)?((^|\s)\d+m)?((^|\s)\d+s)?)$", value) + is_valid_duration = re.match(r"^(?:(\d+d)?((^|\s)\d+h)?((^|\s)\d+m)?((^|\s)\d+s)?)$", value) if not is_valid_duration: self.warnings.append( { From e54b1d58351c39f9d8163dcde3d15066853f1dce Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 11 May 2021 15:23:02 +0530 Subject: [PATCH 12/19] refactor: Link static assets from apps to sites/assets --- frappe/build.py | 131 +++++++++++++++++++++++++++--------------------- 1 file changed, 74 insertions(+), 57 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index 05a4bea7e6..60c8ebf79a 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -5,7 +5,7 @@ import os import re import json import shutil -from tempfile import mkdtemp +from tempfile import mkdtemp, mktemp from distutils.spawn import find_executable import frappe @@ -188,7 +188,8 @@ def symlink(target, link_name, overwrite=False): def setup(): - global app_paths + global app_paths, assets_path + pymodules = [] for app in frappe.get_all_apps(True): try: @@ -196,6 +197,7 @@ def setup(): except ImportError: pass app_paths = [os.path.dirname(pymodule.__file__) for pymodule in pymodules] + assets_path = os.path.join(frappe.local.sites_path, "assets") def get_node_pacman(): @@ -261,75 +263,90 @@ def get_safe_max_old_space_size(): return safe_max_old_space_size -def make_asset_dirs(make_copy=False, restore=False): - # don't even think of making assets_path absolute - rm -rf ahead. - assets_path = os.path.join(frappe.local.sites_path, "assets") +def generate_assets_map(): + symlinks = {} - for dir_path in [os.path.join(assets_path, "js"), os.path.join(assets_path, "css")]: - if not os.path.exists(dir_path): - os.makedirs(dir_path) + for app_name in frappe.get_all_apps(): + app_doc_path = None - for app_name in frappe.get_all_apps(True): pymodule = frappe.get_module(app_name) app_base_path = os.path.abspath(os.path.dirname(pymodule.__file__)) - - symlinks = [] app_public_path = os.path.join(app_base_path, "public") - # app/public > assets/app - symlinks.append([app_public_path, os.path.join(assets_path, app_name)]) - # app/node_modules > assets/app/node_modules - if os.path.exists(os.path.abspath(app_public_path)): - symlinks.append( - [ - os.path.join(app_base_path, "..", "node_modules"), - os.path.join(assets_path, app_name, "node_modules"), - ] - ) + app_node_modules_path = os.path.join(app_base_path, "..", "node_modules") + app_docs_path = os.path.join(app_base_path, "docs") + app_www_docs_path = os.path.join(app_base_path, "www", "docs") - app_doc_path = None - if os.path.isdir(os.path.join(app_base_path, "docs")): + app_assets = os.path.abspath(app_public_path) + app_node_modules = os.path.abspath(app_node_modules_path) + + # {app}/public > assets/{app} + if os.path.isdir(app_assets): + symlinks[app_assets] = os.path.join(assets_path, app_name) + + # {app}/node_modules > assets/{app}/node_modules + if os.path.isdir(app_node_modules): + symlinks[app_node_modules] = os.path.join(assets_path, app_name, "node_modules") + + # {app}/docs > assets/{app}_docs + if os.path.isdir(app_docs_path): app_doc_path = os.path.join(app_base_path, "docs") - - elif os.path.isdir(os.path.join(app_base_path, "www", "docs")): + elif os.path.isdir(app_www_docs_path): app_doc_path = os.path.join(app_base_path, "www", "docs") - if app_doc_path: - symlinks.append([app_doc_path, os.path.join(assets_path, app_name + "_docs")]) + app_docs = os.path.abspath(app_doc_path) + symlinks[app_docs] = os.path.join(assets_path, app_name + "_docs") - for source, target in symlinks: - source = os.path.abspath(source) - if os.path.exists(source): - if restore: - if os.path.exists(target): - if os.path.islink(target): - os.unlink(target) - else: - shutil.rmtree(target) - shutil.copytree(source, target) - elif make_copy: - if os.path.exists(target): - warnings.warn("Target {target} already exists.".format(target=target)) - else: - shutil.copytree(source, target) - else: - if os.path.exists(target): - if os.path.islink(target): - os.unlink(target) - else: - shutil.rmtree(target) - try: - symlink(source, target, overwrite=True) - except OSError: - print("Cannot link {} to {}".format(source, target)) + return symlinks + + +def setup_assets_dirs(): + for dir_path in (os.path.join(assets_path, x) for x in ("js", "css")): + os.makedirs(dir_path, exist_ok=True) + + +def clear_broken_symlinks(): + for path in os.listdir(assets_path): + path = os.path.join(assets_path, path) + if os.path.islink(path) and not os.path.exists(path): + os.remove(path) + + +def make_asset_dirs(make_copy=False, restore=False): + setup_assets_dirs() + clear_broken_symlinks() + symlinks = generate_assets_map() + + for source, target in symlinks.items(): + link_assets_dir(source, target, make_copy=make_copy, restore=restore) + + +def link_assets_dir(source, target, restore=False, make_copy=False): + if not os.path.exists(source): + return + + if restore: + if os.path.exists(target): + if os.path.islink(target): + os.unlink(target) else: - warnings.warn('Source {source} does not exist.'.format(source = source)) - pass + shutil.rmtree(target) + shutil.copytree(source, target) + elif make_copy: + shutil.copytree(source, target, dirs_exist_ok=True) + else: + if os.path.exists(target): + if os.path.islink(target): + os.unlink(target) + else: + shutil.rmtree(target) + try: + symlink(source, target, overwrite=True) + except OSError: + print("Cannot link {} to {}".format(source, target)) def build(no_compress=False, verbose=False): - assets_path = os.path.join(frappe.local.sites_path, "assets") - - for target, sources in iteritems(get_build_maps()): + for target, sources in get_build_maps().items(): pack(os.path.join(assets_path, target), sources, no_compress, verbose) From e18f0804539a7930720c51964a19a18d2ec94a48 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 11 May 2021 15:25:21 +0530 Subject: [PATCH 13/19] chore: Hide all warnings in bench console --- frappe/commands/utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index b917126696..2451b88cc9 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -488,6 +488,8 @@ frappe.db.connect() @pass_context def console(context): "Start ipython console for a site" + import warnings + site = get_site(context) frappe.init(site=site) frappe.connect() @@ -508,6 +510,7 @@ def console(context): if failed_to_import: print("\nFailed to import:\n{}".format(", ".join(failed_to_import))) + warnings.simplefilter('ignore') IPython.embed(display_banner="", header="", colors="neutral") From 82685d2545a4740609df91ec2b985e3fd73dbb21 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 11 May 2021 16:11:49 +0530 Subject: [PATCH 14/19] fix: Convert multi-line translation string to single --- frappe/core/doctype/data_import/importer.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 0eb1752d78..d3f981add4 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -929,10 +929,7 @@ class Column: self.warnings.append( { "col": self.column_number, - "message": _( - "Date format could not be determined from the values in" - " this column. Defaulting to yyyy-mm-dd." - ), + "message": _("Date format could not be determined from the values in this column. Defaulting to yyyy-mm-dd."), "type": "info", } ) From 548eb079c8b0bac8911d2cd6d2ecc0e1a7e889bb Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 11 May 2021 20:09:21 +0530 Subject: [PATCH 15/19] fix: Don't re-copy node_modules if public already has it public folder already has a symlink to node_modules. So we can just check if the realpath already exists and ignore copying it again if the same exists --- frappe/build.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index 60c8ebf79a..99f1dd3fc0 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -268,14 +268,17 @@ def generate_assets_map(): for app_name in frappe.get_all_apps(): app_doc_path = None - pymodule = frappe.get_module(app_name) app_base_path = os.path.abspath(os.path.dirname(pymodule.__file__)) + app_public_path = os.path.join(app_base_path, "public") app_node_modules_path = os.path.join(app_base_path, "..", "node_modules") + internal_app_node_modules_path = os.path.join(app_base_path, "..", "node_modules") + node_modules_required = ( + os.path.realpath(app_node_modules_path) != os.path.realpath(internal_app_node_modules_path) + ) app_docs_path = os.path.join(app_base_path, "docs") app_www_docs_path = os.path.join(app_base_path, "www", "docs") - app_assets = os.path.abspath(app_public_path) app_node_modules = os.path.abspath(app_node_modules_path) @@ -284,7 +287,8 @@ def generate_assets_map(): symlinks[app_assets] = os.path.join(assets_path, app_name) # {app}/node_modules > assets/{app}/node_modules - if os.path.isdir(app_node_modules): + # node_modules_required is set if its not already a path of {app}/public + if os.path.isdir(app_node_modules) and node_modules_required: symlinks[app_node_modules] = os.path.join(assets_path, app_name, "node_modules") # {app}/docs > assets/{app}_docs From 098f1564f3904c7111b9828eed75cbea326a5b7c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 11 May 2021 21:12:35 +0530 Subject: [PATCH 16/19] refactor: bench build * Deprecate --make-copy and --restore options for build in favour of --hard-link * Show feedback for linking/copying application assets --- frappe/build.py | 57 ++++++++++++++++++++++++---------------- frappe/commands/utils.py | 23 ++++++++++++---- 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index 99f1dd3fc0..e2145e6ac3 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -14,6 +14,7 @@ from frappe.utils.minify import JavascriptMinify import click import psutil from urllib.parse import urlparse +from simple_chalk import green timestamps = {} @@ -93,7 +94,6 @@ def download_frappe_assets(verbose=True): commit HEAD. Returns True if correctly setup else returns False. """ - from simple_chalk import green from subprocess import getoutput assets_setup = False @@ -207,10 +207,10 @@ def get_node_pacman(): raise ValueError("Yarn not found") -def bundle(no_compress, app=None, make_copy=False, restore=False, verbose=False, skip_frappe=False): +def bundle(no_compress, app=None, hard_link=False, verbose=False, skip_frappe=False): """concat / minify js files""" setup() - make_asset_dirs(make_copy=make_copy, restore=restore) + make_asset_dirs(hard_link=hard_link) pacman = get_node_pacman() mode = "build" if no_compress else "production" @@ -315,38 +315,49 @@ def clear_broken_symlinks(): os.remove(path) -def make_asset_dirs(make_copy=False, restore=False): + +def unstrip(message): + try: + max_str = os.get_terminal_size().columns + except Exception: + max_str = 80 + _len = len(message) + _rem = max_str - _len + return f"{message}{' ' * _rem}" + + +def make_asset_dirs(hard_link=False): setup_assets_dirs() clear_broken_symlinks() symlinks = generate_assets_map() for source, target in symlinks.items(): - link_assets_dir(source, target, make_copy=make_copy, restore=restore) + start_message = unstrip(f"{'Copying assets from' if hard_link else 'Linking'} {source} to {target}") + fail_message = unstrip(f"Cannot {'copy' if hard_link else 'link'} {source} to {target}") + + try: + print(start_message, end="\r") + link_assets_dir(source, target, hard_link=hard_link) + except Exception: + print(fail_message, end="\r") + + print(unstrip(f"{green('✔')} Application Assets Linked") + "\n") -def link_assets_dir(source, target, restore=False, make_copy=False): +def link_assets_dir(source, target, hard_link=False): if not os.path.exists(source): return - if restore: - if os.path.exists(target): - if os.path.islink(target): - os.unlink(target) - else: - shutil.rmtree(target) - shutil.copytree(source, target) - elif make_copy: + if os.path.exists(target): + if os.path.islink(target): + os.unlink(target) + else: + shutil.rmtree(target) + + if hard_link: shutil.copytree(source, target, dirs_exist_ok=True) else: - if os.path.exists(target): - if os.path.islink(target): - os.unlink(target) - else: - shutil.rmtree(target) - try: - symlink(source, target, overwrite=True) - except OSError: - print("Cannot link {} to {}".format(source, target)) + symlink(source, target, overwrite=True) def build(no_compress=False, verbose=False): diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 2451b88cc9..13a2ef61e0 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -16,13 +16,13 @@ from frappe.utils import get_bench_path, update_progress_bar, cint @click.command('build') @click.option('--app', help='Build assets for app') -@click.option('--make-copy', is_flag=True, default=False, help='Copy the files instead of symlinking') -@click.option('--restore', is_flag=True, default=False, help='Copy the files instead of symlinking with force') +@click.option('--hard-link', is_flag=True, default=False, help='Copy the files instead of symlinking') +@click.option('--make-copy', is_flag=True, default=False, help='[DEPRECATED] Copy the files instead of symlinking') +@click.option('--restore', is_flag=True, default=False, help='[DEPRECATED] Copy the files instead of symlinking with force') @click.option('--verbose', is_flag=True, default=False, help='Verbose') @click.option('--force', is_flag=True, default=False, help='Force build assets instead of downloading available') -def build(app=None, make_copy=False, restore=False, verbose=False, force=False): +def build(app=None, hard_link=False, make_copy=False, restore=False, verbose=False, force=False): "Minify + concatenate JS and CSS files, build translations" - import frappe.build frappe.init('') # don't minify in developer_mode for faster builds no_compress = frappe.local.conf.developer_mode or False @@ -34,7 +34,20 @@ def build(app=None, make_copy=False, restore=False, verbose=False, force=False): else: skip_frappe = False - frappe.build.bundle(no_compress, app=app, make_copy=make_copy, restore=restore, verbose=verbose, skip_frappe=skip_frappe) + if make_copy or restore: + hard_link = make_copy or restore + click.secho( + "bench build: --make-copy and --restore options are deprecated in favour of --hard-link", + fg="yellow", + ) + + frappe.build.bundle( + skip_frappe=skip_frappe, + no_compress=no_compress, + hard_link=hard_link, + verbose=verbose, + app=app, + ) @click.command('watch') From 6581ed886aa9a70e71e77e5c7788ac2fdb5c0784 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 12 May 2021 21:00:07 +0530 Subject: [PATCH 17/19] Revert "fix: Don't re-copy node_modules if public already has it" This reverts commit 548eb079c8b0bac8911d2cd6d2ecc0e1a7e889bb. Seems to break UI components. Learnt this from Cypress UI tests ref: https://github.com/frappe/frappe/pull/13145/checks?check_run_id=2567366717 --- frappe/build.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index e2145e6ac3..321a9bf734 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -268,17 +268,14 @@ def generate_assets_map(): for app_name in frappe.get_all_apps(): app_doc_path = None + pymodule = frappe.get_module(app_name) app_base_path = os.path.abspath(os.path.dirname(pymodule.__file__)) - app_public_path = os.path.join(app_base_path, "public") app_node_modules_path = os.path.join(app_base_path, "..", "node_modules") - internal_app_node_modules_path = os.path.join(app_base_path, "..", "node_modules") - node_modules_required = ( - os.path.realpath(app_node_modules_path) != os.path.realpath(internal_app_node_modules_path) - ) app_docs_path = os.path.join(app_base_path, "docs") app_www_docs_path = os.path.join(app_base_path, "www", "docs") + app_assets = os.path.abspath(app_public_path) app_node_modules = os.path.abspath(app_node_modules_path) @@ -287,8 +284,7 @@ def generate_assets_map(): symlinks[app_assets] = os.path.join(assets_path, app_name) # {app}/node_modules > assets/{app}/node_modules - # node_modules_required is set if its not already a path of {app}/public - if os.path.isdir(app_node_modules) and node_modules_required: + if os.path.isdir(app_node_modules): symlinks[app_node_modules] = os.path.join(assets_path, app_name, "node_modules") # {app}/docs > assets/{app}_docs From 085290630ec56277434a90b774f97e96a0e0ca6e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 13 May 2021 11:11:45 +0530 Subject: [PATCH 18/19] fix(build): Use NamedTemporaryFile from mktemp --- frappe/build.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index 321a9bf734..e2a3e29f72 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -5,7 +5,7 @@ import os import re import json import shutil -from tempfile import mkdtemp, mktemp +from tempfile import NamedTemporaryFile, mkdtemp from distutils.spawn import find_executable import frappe @@ -161,7 +161,8 @@ def symlink(target, link_name, overwrite=False): # Create link to target with temporary filename while True: - temp_link_name = mktemp(dir=link_dir) + # updated usage from mktemp to NamedTemporaryFile + temp_link_name = NamedTemporaryFile(dir=link_dir) # os.* functions mimic as closely as possible system functions # The POSIX symlink() returns EEXIST if link_name already exists From 7930b84b71b94bd61aea57bc193a310a8bbf0548 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 13 May 2021 12:28:12 +0530 Subject: [PATCH 19/19] Revert "fix(build): Use NamedTemporaryFile from mktemp" This reverts commit 085290630ec56277434a90b774f97e96a0e0ca6e. Breaks via https://github.com/frappe/frappe/pull/13145/checks?check_run_id=2573309928 --- frappe/build.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index e2a3e29f72..321a9bf734 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -5,7 +5,7 @@ import os import re import json import shutil -from tempfile import NamedTemporaryFile, mkdtemp +from tempfile import mkdtemp, mktemp from distutils.spawn import find_executable import frappe @@ -161,8 +161,7 @@ def symlink(target, link_name, overwrite=False): # Create link to target with temporary filename while True: - # updated usage from mktemp to NamedTemporaryFile - temp_link_name = NamedTemporaryFile(dir=link_dir) + temp_link_name = mktemp(dir=link_dir) # os.* functions mimic as closely as possible system functions # The POSIX symlink() returns EEXIST if link_name already exists