From d89f19e02ea8aba027f7205a6fe8fb2c93609ad7 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 17 Mar 2022 12:58:05 +0530 Subject: [PATCH 1/5] fix: Show better user messages when failed to update supervisord --- bench/config/supervisor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bench/config/supervisor.py b/bench/config/supervisor.py index 8db76c3ad..765e9989e 100644 --- a/bench/config/supervisor.py +++ b/bench/config/supervisor.py @@ -109,7 +109,9 @@ def update_supervisord_config(user=None, yes=False): supervisord_conf_changes += '\n' + action if not supervisord_conf_changes: - logger.log("supervisord.conf not updated") + logger.error("supervisord.conf not updated") + contents = "\n".join(f"{x}={y}" for x, y in updated_values.items()) + print(f"Update your {supervisord_conf} with the following values:\n[{section}]\n{contents}") return if not yes: From 7a4ade9192ca451cfce87280ea05a89caba51054 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 17 Mar 2022 13:03:04 +0530 Subject: [PATCH 2/5] fix: Try setting supervisord conditionally --- bench/commands/setup.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bench/commands/setup.py b/bench/commands/setup.py index a6526cda2..99b066063 100755 --- a/bench/commands/setup.py +++ b/bench/commands/setup.py @@ -41,9 +41,12 @@ def reload_nginx(): @click.option("--yes", help="Yes to regeneration of supervisor config", is_flag=True, default=False) @click.option("--skip-redis", help="Skip redis configuration", is_flag=True, default=False) def setup_supervisor(user=None, yes=False, skip_redis=False): + from bench.utils import get_cmd_output from bench.config.supervisor import update_supervisord_config, generate_supervisor_config - update_supervisord_config(user=user, yes=yes) + if "Permission denied" in get_cmd_output("supervisorctl status"): + update_supervisord_config(user=user, yes=yes) + generate_supervisor_config(bench_path=".", user=user, yes=yes, skip_redis=skip_redis) From fc7c047c280c443fa85f33f90e572412f206dbff Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 17 Mar 2022 15:04:22 +0530 Subject: [PATCH 3/5] fix: Allow sudo restart for supervisor (backwards compatibility) --- bench/utils/bench.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/bench/utils/bench.py b/bench/utils/bench.py index 9f170ba14..28ca31f30 100644 --- a/bench/utils/bench.py +++ b/bench/utils/bench.py @@ -253,7 +253,13 @@ def restart_supervisor_processes(bench_path=".", web_workers=False): bench.run(cmd) else: - supervisor_status = get_cmd_output("supervisorctl status", cwd=bench_path) + sudo = "" + try: + supervisor_status = get_cmd_output("supervisorctl status", cwd=bench_path) + except Exception as e: + if e.returncode == 127: + sudo = "sudo " + supervisor_status = get_cmd_output("sudo supervisorctl status", cwd=bench_path) if web_workers and f"{bench_name}-web:" in supervisor_status: group = f"{bench_name}-web:\t" @@ -269,7 +275,7 @@ def restart_supervisor_processes(bench_path=".", web_workers=False): else: group = "frappe:" - bench.run(f"supervisorctl restart {group}") + bench.run(f"{sudo}supervisorctl restart {group}") def restart_systemd_processes(bench_path=".", web_workers=False): From af14bafe2137da799694a0ad3dd5a2b138c311e6 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 21 Mar 2022 13:37:16 +0530 Subject: [PATCH 4/5] fix(restart): Setup restarts vars with fallback --- bench/app.py | 4 ++-- bench/bench.py | 4 ++-- bench/commands/setup.py | 4 +++- bench/commands/utils.py | 3 +++ bench/utils/__init__.py | 2 +- bench/utils/bench.py | 6 ++++-- 6 files changed, 15 insertions(+), 8 deletions(-) diff --git a/bench/app.py b/bench/app.py index 5ce0f8fc5..f8c1ba221 100755 --- a/bench/app.py +++ b/bench/app.py @@ -320,6 +320,7 @@ def get_app( repo_name = app.repo branch = app.tag bench_setup = False + restart_bench = not init_bench if not is_bench_directory(bench_path): if not init_bench: @@ -343,7 +344,6 @@ def get_app( "color": None, }) - cloned_path = os.path.join(bench_path, "apps", repo_name) dir_already_exists = os.path.isdir(cloned_path) to_clone = not dir_already_exists @@ -368,7 +368,7 @@ def get_app( or overwrite or click.confirm("Do you want to reinstall the existing application?") ): - app.install(verbose=verbose, skip_assets=skip_assets) + app.install(verbose=verbose, skip_assets=skip_assets, restart_bench=restart_bench) def new_app(app, no_git=None, bench_path="."): diff --git a/bench/bench.py b/bench/bench.py index 655693e29..f359be7cd 100644 --- a/bench/bench.py +++ b/bench/bench.py @@ -138,9 +138,9 @@ def reload(self, web=False, supervisor=True, systemd=True): if conf.get("developer_mode"): restart_process_manager(bench_path=self.name, web_workers=web) - if supervisor or conf.get("restart_supervisor_on_update"): + if supervisor and conf.get("restart_supervisor_on_update"): restart_supervisor_processes(bench_path=self.name, web_workers=web) - if systemd or conf.get("restart_systemd_on_update"): + if systemd and conf.get("restart_systemd_on_update"): restart_systemd_processes(bench_path=self.name, web_workers=web) def get_installed_apps(self) -> List: diff --git a/bench/commands/setup.py b/bench/commands/setup.py index 99b066063..42672aa14 100755 --- a/bench/commands/setup.py +++ b/bench/commands/setup.py @@ -6,7 +6,7 @@ import click # imports - module imports -from bench.utils import exec_cmd, run_playbook +from bench.utils import exec_cmd, run_playbook, which @click.group(help="Setup command group for enabling setting up a Frappe environment") @@ -44,6 +44,8 @@ def setup_supervisor(user=None, yes=False, skip_redis=False): from bench.utils import get_cmd_output from bench.config.supervisor import update_supervisord_config, generate_supervisor_config + which("supervisorctl", raise_err=True) + if "Permission denied" in get_cmd_output("supervisorctl status"): update_supervisord_config(user=user, yes=yes) diff --git a/bench/commands/utils.py b/bench/commands/utils.py index 20b5a796e..b2d8a301e 100644 --- a/bench/commands/utils.py +++ b/bench/commands/utils.py @@ -23,6 +23,9 @@ def start(no_dev, concurrency, procfile, no_prefix, man): @click.option('--systemd', is_flag=True, default=False) def restart(web, supervisor, systemd): from bench.bench import Bench + if not systemd and not web: + supervisor = True + Bench(".").reload(web, supervisor, systemd) diff --git a/bench/utils/__init__.py b/bench/utils/__init__.py index f75fe2634..dde12acc4 100644 --- a/bench/utils/__init__.py +++ b/bench/utils/__init__.py @@ -133,7 +133,7 @@ def which(executable: str, raise_err: bool = False) -> str: exec_ = which(executable) if not exec_ and raise_err: - raise ValueError(f"{executable} not found.") + raise FileNotFoundError(f"{executable} not found in PATH") return exec_ diff --git a/bench/utils/bench.py b/bench/utils/bench.py index 28ca31f30..e0be48b90 100644 --- a/bench/utils/bench.py +++ b/bench/utils/bench.py @@ -258,8 +258,10 @@ def restart_supervisor_processes(bench_path=".", web_workers=False): supervisor_status = get_cmd_output("supervisorctl status", cwd=bench_path) except Exception as e: if e.returncode == 127: - sudo = "sudo " - supervisor_status = get_cmd_output("sudo supervisorctl status", cwd=bench_path) + log("restart failed: Couldn't find supervisorctl in PATH", level=3) + return + sudo = "sudo " + supervisor_status = get_cmd_output("sudo supervisorctl status", cwd=bench_path) if web_workers and f"{bench_name}-web:" in supervisor_status: group = f"{bench_name}-web:\t" From ea947523b36211fbc73fc51c971f44794fd38d46 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 21 Mar 2022 13:38:04 +0530 Subject: [PATCH 5/5] fix(find): Handle OS' PermissionError --- bench/utils/__init__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bench/utils/__init__.py b/bench/utils/__init__.py index dde12acc4..9c8d98a4d 100644 --- a/bench/utils/__init__.py +++ b/bench/utils/__init__.py @@ -314,7 +314,13 @@ def find_benches(directory: str = None) -> List: return benches = [] - for sub in os.listdir(directory): + + try: + sub_directories = os.listdir(directory) + except PermissionError: + return benches + + for sub in sub_directories: sub = os.path.join(directory, sub) if os.path.isdir(sub) and not os.path.islink(sub): if is_bench_directory(sub):