From a94ea19bf43d0359c5f59d2aa886742eb8832c65 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 9 Sep 2021 13:19:44 +0530 Subject: [PATCH 1/6] fix: Hit command cache before fetching all commands --- bench/cli.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/bench/cli.py b/bench/cli.py index 9fb29966..765ae758 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -45,7 +45,11 @@ def cli(): return old_frappe_cli() elif len(sys.argv) > 1: - if sys.argv[1] in get_frappe_commands() + ["--site", "--verbose", "--force", "--profile"]: + if sys.argv[1] in ["--site", "--verbose", "--force", "--profile"]: + return frappe_cmd() + if sys.argv[1] in get_cached_frappe_commands(): + return frappe_cmd() + if sys.argv[1] in get_frappe_commands(): return frappe_cmd() elif sys.argv[1] == "--help": @@ -124,16 +128,16 @@ def frappe_cmd(bench_path='.'): os.execv(f, [f] + ['-m', 'frappe.utils.bench_helper', 'frappe'] + sys.argv[1:]) -def get_frappe_commands(): - if not is_bench_directory(): - return [] - +def get_cached_frappe_commands(): if os.path.exists(bench_cache_file): command_dump = open(bench_cache_file, 'r').read() or '[]' return json.loads(command_dump) - else: - return generate_command_cache() +def get_frappe_commands(): + if not is_bench_directory(): + return [] + + return generate_command_cache() def get_frappe_help(bench_path='.'): From 552b935f6b7d35933907c05421171827b30a967c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 9 Sep 2021 13:20:26 +0530 Subject: [PATCH 2/6] fix: Always set return code via cli --- bench/cli.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bench/cli.py b/bench/cli.py index 765ae758..3d3d2454 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -69,6 +69,13 @@ def cli(): return_code = getattr(e, "code", 0) if return_code: logger.warning(f"{command} executed with exit code {return_code}") + if isinstance(e, Exception): + raise e + finally: + try: + return_code + except NameError: + return_code = 0 sys.exit(return_code) From 6160ff48ab1c04ebcf4436e40a51efd564647aee Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 9 Sep 2021 13:54:19 +0530 Subject: [PATCH 3/6] style: Black --- bench/cli.py | 107 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 76 insertions(+), 31 deletions(-) diff --git a/bench/cli.py b/bench/cli.py index 3d3d2454..b2a79e2c 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -1,7 +1,6 @@ # imports - standard imports import atexit import json -import logging import os import pwd import sys @@ -14,7 +13,21 @@ import bench from bench.app import get_apps from bench.commands import bench_command from bench.config.common_site_config import get_config -from bench.utils import PatchError, bench_cache_file, check_latest_version, drop_privileges, find_parent_bench, generate_command_cache, get_cmd_output, get_env_cmd, get_frappe, is_bench_directory, is_dist_editable, is_root, log, setup_logging +from bench.utils import ( + bench_cache_file, + check_latest_version, + drop_privileges, + find_parent_bench, + generate_command_cache, + get_cmd_output, + get_env_cmd, + get_frappe, + is_bench_directory, + is_dist_editable, + is_root, + log, + setup_logging, +) from_command_line = False change_uid_msg = "You should not run this command as root" @@ -30,15 +43,30 @@ def cli(): logger = setup_logging() logger.info(command) - if len(sys.argv) > 1 and sys.argv[1] not in ("src", ): + if len(sys.argv) > 1 and sys.argv[1] not in ("src",): check_uid() change_uid() change_dir() - if is_dist_editable(bench.PROJECT_NAME) and len(sys.argv) > 1 and sys.argv[1] != "src" and not get_config(".").get("developer_mode"): - log("bench is installed in editable mode!\n\nThis is not the recommended mode of installation for production. Instead, install the package from PyPI with: `pip install frappe-bench`\n", level=3) + if ( + is_dist_editable(bench.PROJECT_NAME) + and len(sys.argv) > 1 + and sys.argv[1] != "src" + and not get_config(".").get("developer_mode") + ): + log( + "bench is installed in editable mode!\n\nThis is not the recommended mode" + " of installation for production. Instead, install the package from PyPI" + " with: `pip install frappe-bench`\n", + level=3, + ) - if not is_bench_directory() and not cmd_requires_root() and len(sys.argv) > 1 and sys.argv[1] not in ("init", "find", "src"): + if ( + not is_bench_directory() + and not cmd_requires_root() + and len(sys.argv) > 1 + and sys.argv[1] not in ("init", "find", "src") + ): log("Command not being executed in bench directory", level=3) if len(sys.argv) > 2 and sys.argv[1] == "frappe": @@ -81,24 +109,38 @@ def cli(): def check_uid(): if cmd_requires_root() and not is_root(): - log('superuser privileges required for this command', level=3) + log("superuser privileges required for this command", level=3) sys.exit(1) def cmd_requires_root(): - if len(sys.argv) > 2 and sys.argv[2] in ('production', 'sudoers', 'lets-encrypt', 'fonts', - 'print', 'firewall', 'ssh-port', 'role', 'fail2ban', 'wildcard-ssl'): + if len(sys.argv) > 2 and sys.argv[2] in ( + "production", + "sudoers", + "lets-encrypt", + "fonts", + "print", + "firewall", + "ssh-port", + "role", + "fail2ban", + "wildcard-ssl", + ): return True - if len(sys.argv) >= 2 and sys.argv[1] in ('patch', 'renew-lets-encrypt', 'disable-production'): + if len(sys.argv) >= 2 and sys.argv[1] in ( + "patch", + "renew-lets-encrypt", + "disable-production", + ): return True - if len(sys.argv) > 2 and sys.argv[1] in ('install'): + if len(sys.argv) > 2 and sys.argv[1] in ("install"): return True def change_dir(): - if os.path.exists('config.json') or "init" in sys.argv: + if os.path.exists("config.json") or "init" in sys.argv: return - dir_path_file = '/etc/frappe_bench_dir' + dir_path_file = "/etc/frappe_bench_dir" if os.path.exists(dir_path_file): with open(dir_path_file) as f: dir_path = f.read().strip() @@ -108,38 +150,39 @@ def change_dir(): def change_uid(): if is_root() and not cmd_requires_root(): - frappe_user = get_config(".").get('frappe_user') + frappe_user = get_config(".").get("frappe_user") if frappe_user: drop_privileges(uid_name=frappe_user, gid_name=frappe_user) - os.environ['HOME'] = pwd.getpwnam(frappe_user).pw_dir + os.environ["HOME"] = pwd.getpwnam(frappe_user).pw_dir else: log(change_uid_msg, level=3) sys.exit(1) -def old_frappe_cli(bench_path='.'): +def old_frappe_cli(bench_path="."): f = get_frappe(bench_path=bench_path) - os.chdir(os.path.join(bench_path, 'sites')) + os.chdir(os.path.join(bench_path, "sites")) os.execv(f, [f] + sys.argv[2:]) -def app_cmd(bench_path='.'): - f = get_env_cmd('python', bench_path=bench_path) - os.chdir(os.path.join(bench_path, 'sites')) - os.execv(f, [f] + ['-m', 'frappe.utils.bench_helper'] + sys.argv[1:]) +def app_cmd(bench_path="."): + f = get_env_cmd("python", bench_path=bench_path) + os.chdir(os.path.join(bench_path, "sites")) + os.execv(f, [f] + ["-m", "frappe.utils.bench_helper"] + sys.argv[1:]) -def frappe_cmd(bench_path='.'): - f = get_env_cmd('python', bench_path=bench_path) - os.chdir(os.path.join(bench_path, 'sites')) - os.execv(f, [f] + ['-m', 'frappe.utils.bench_helper', 'frappe'] + sys.argv[1:]) +def frappe_cmd(bench_path="."): + f = get_env_cmd("python", bench_path=bench_path) + os.chdir(os.path.join(bench_path, "sites")) + os.execv(f, [f] + ["-m", "frappe.utils.bench_helper", "frappe"] + sys.argv[1:]) def get_cached_frappe_commands(): if os.path.exists(bench_cache_file): - command_dump = open(bench_cache_file, 'r').read() or '[]' + command_dump = open(bench_cache_file, "r").read() or "[]" return json.loads(command_dump) + def get_frappe_commands(): if not is_bench_directory(): return [] @@ -147,12 +190,14 @@ def get_frappe_commands(): return generate_command_cache() -def get_frappe_help(bench_path='.'): - python = get_env_cmd('python', bench_path=bench_path) - sites_path = os.path.join(bench_path, 'sites') +def get_frappe_help(bench_path="."): + python = get_env_cmd("python", bench_path=bench_path) + sites_path = os.path.join(bench_path, "sites") try: - out = get_cmd_output(f"{python} -m frappe.utils.bench_helper get-frappe-help", cwd=sites_path) - return "\n\nFramework commands:\n" + out.split('Commands:')[1] + out = get_cmd_output( + f"{python} -m frappe.utils.bench_helper get-frappe-help", cwd=sites_path + ) + return "\n\nFramework commands:\n" + out.split("Commands:")[1] except: return "" From a01550169905a20b8322931338331cc80bc57766 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 9 Sep 2021 13:54:35 +0530 Subject: [PATCH 4/6] fix: Return fallback list in cached_frappe_commands --- bench/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bench/cli.py b/bench/cli.py index b2a79e2c..9d49dbbe 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -181,6 +181,7 @@ def get_cached_frappe_commands(): if os.path.exists(bench_cache_file): command_dump = open(bench_cache_file, "r").read() or "[]" return json.loads(command_dump) + return [] def get_frappe_commands(): From b35af193d34d45ed068888bba8bf4d5490154e9f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 9 Sep 2021 14:56:32 +0530 Subject: [PATCH 5/6] chore: Remove unnecessary returns, re-order cli resolution Changes suggested by LGTM https://lgtm.com/projects/g/frappe/bench/rev/pr-560978725461013796d605b409ffc5c85c774829 --- bench/cli.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/bench/cli.py b/bench/cli.py index 9d49dbbe..f194349c 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -70,23 +70,25 @@ def cli(): log("Command not being executed in bench directory", level=3) if len(sys.argv) > 2 and sys.argv[1] == "frappe": - return old_frappe_cli() + old_frappe_cli() elif len(sys.argv) > 1: - if sys.argv[1] in ["--site", "--verbose", "--force", "--profile"]: - return frappe_cmd() - if sys.argv[1] in get_cached_frappe_commands(): - return frappe_cmd() - if sys.argv[1] in get_frappe_commands(): - return frappe_cmd() - - elif sys.argv[1] == "--help": + if sys.argv[1] == "--help": print(click.Context(bench_command).get_help()) print(get_frappe_help()) return - elif sys.argv[1] in get_apps(): - return app_cmd() + if sys.argv[1] in ["--site", "--verbose", "--force", "--profile"]: + frappe_cmd() + + if sys.argv[1] in get_cached_frappe_commands(): + frappe_cmd() + + if sys.argv[1] in get_frappe_commands(): + frappe_cmd() + + if sys.argv[1] in get_apps(): + app_cmd() if not (len(sys.argv) > 1 and sys.argv[1] == "src"): atexit.register(check_latest_version) @@ -199,7 +201,7 @@ def get_frappe_help(bench_path="."): f"{python} -m frappe.utils.bench_helper get-frappe-help", cwd=sites_path ) return "\n\nFramework commands:\n" + out.split("Commands:")[1] - except: + except Exception: return "" From 6fd3ad738b75a974763aec26ff8124c4fcf4147e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 9 Sep 2021 16:05:42 +0530 Subject: [PATCH 6/6] fix(command_cache): Return iterable as fallback --- bench/utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bench/utils.py b/bench/utils.py index 39d0ca56..bfe6469c 100755 --- a/bench/utils.py +++ b/bench/utils.py @@ -1076,6 +1076,8 @@ def generate_command_cache(bench_path='.'): if hasattr(e, "stderr"): print(e.stderr.decode('utf-8')) + return [] + def clear_command_cache(bench_path='.'): """Clears commands cached