From a3af905d929fd1e69e35561d9601d56951650444 Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Thu, 4 Aug 2022 12:13:10 +0530
Subject: [PATCH 1/6] feat: Allow skipping supervisord config check in setup
 supervisor

---
 bench/commands/setup.py          | 14 +++++++++++---
 bench/config/production_setup.py |  2 +-
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/bench/commands/setup.py b/bench/commands/setup.py
index 4d1fa88a..6b9ead6c 100755
--- a/bench/commands/setup.py
+++ b/bench/commands/setup.py
@@ -49,7 +49,13 @@ def reload_nginx():
 @click.option(
 	"--skip-redis", help="Skip redis configuration", is_flag=True, default=False
 )
-def setup_supervisor(user=None, yes=False, skip_redis=False):
+@click.option(
+	"--skip-supervisord",
+	help="Skip supervisord configuration",
+	is_flag=True,
+	default=False,
+)
+def setup_supervisor(user=None, yes=False, skip_redis=False, skip_supervisord=False):
 	from bench.utils import get_cmd_output
 	from bench.config.supervisor import (
 		update_supervisord_config,
@@ -58,8 +64,10 @@ def setup_supervisor(user=None, yes=False, skip_redis=False):
 
 	which("supervisorctl", raise_err=True)
 
-	if "Permission denied" in get_cmd_output("supervisorctl status"):
-		update_supervisord_config(user=user, yes=yes)
+	if not skip_supervisord and "Permission denied" in get_cmd_output(
+		"supervisorctl status"
+	):
+		update_supervisord_config(user=user)
 
 	generate_supervisor_config(bench_path=".", user=user, yes=yes, skip_redis=skip_redis)
 
diff --git a/bench/config/production_setup.py b/bench/config/production_setup.py
index 719f90d2..390c3a57 100755
--- a/bench/config/production_setup.py
+++ b/bench/config/production_setup.py
@@ -48,7 +48,7 @@ def setup_production(user, bench_path=".", yes=False):
 		generate_systemd_config(bench_path=bench_path, user=user, yes=yes)
 	else:
 		print("Setting Up supervisor...")
-		update_supervisord_config(user=user, yes=yes)
+		update_supervisord_config(user=user)
 		generate_supervisor_config(bench_path=bench_path, user=user, yes=yes)
 
 	print("Setting Up NGINX...")

From db13ce3732eadd0438d1f3c1e9e7b47abf8d322c Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Thu, 4 Aug 2022 12:14:07 +0530
Subject: [PATCH 2/6] fix!: Don't update supervisord conf - instead just
 suggest

---
 bench/config/supervisor.py | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/bench/config/supervisor.py b/bench/config/supervisor.py
index d4d6b14e..42c7a1e3 100644
--- a/bench/config/supervisor.py
+++ b/bench/config/supervisor.py
@@ -79,10 +79,11 @@ def get_supervisord_conf():
 			return possibility
 
 
-def update_supervisord_config(user=None, yes=False):
+def update_supervisord_config(user=None):
 	"""From bench v5.x, we're moving to supervisor running as user"""
+	# i don't think bench should be responsible for this but we're way past this now...
+	# removed updating supervisord conf & reload in Aug 2022 - gavin@frappe.io
 	import configparser
-	from bench.config.production_setup import service
 
 	if not user:
 		user = getpass.getuser()
@@ -125,20 +126,3 @@ def update_supervisord_config(user=None, yes=False):
 		print(
 			f"Update your {supervisord_conf} with the following values:\n[{section}]\n{contents}"
 		)
-		return
-
-	if not yes:
-		click.confirm(
-			f"{supervisord_conf} will be updated with the following values:\n{supervisord_conf_changes}\nDo you want to continue?",
-			abort=True,
-		)
-
-	try:
-		with open(supervisord_conf, "w") as f:
-			config.write(f)
-			logger.log(f"Updated supervisord.conf at '{supervisord_conf}'")
-	except Exception as e:
-		logger.log(f"Updating supervisord.conf failed due to '{e}'")
-
-	# Reread supervisor configuration, reload supervisord and supervisorctl, restart services that were started
-	service("supervisor", "reload")

From f734eef7a7b25023e7cd288e01eda1970d5a0192 Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Thu, 4 Aug 2022 12:15:51 +0530
Subject: [PATCH 3/6] refactor: check_supervisord_config

---
 bench/commands/setup.py          | 4 ++--
 bench/config/production_setup.py | 4 ++--
 bench/config/supervisor.py       | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/bench/commands/setup.py b/bench/commands/setup.py
index 6b9ead6c..6869d034 100755
--- a/bench/commands/setup.py
+++ b/bench/commands/setup.py
@@ -58,7 +58,7 @@ def reload_nginx():
 def setup_supervisor(user=None, yes=False, skip_redis=False, skip_supervisord=False):
 	from bench.utils import get_cmd_output
 	from bench.config.supervisor import (
-		update_supervisord_config,
+		check_supervisord_config,
 		generate_supervisor_config,
 	)
 
@@ -67,7 +67,7 @@ def setup_supervisor(user=None, yes=False, skip_redis=False, skip_supervisord=Fa
 	if not skip_supervisord and "Permission denied" in get_cmd_output(
 		"supervisorctl status"
 	):
-		update_supervisord_config(user=user)
+		check_supervisord_config(user=user)
 
 	generate_supervisor_config(bench_path=".", user=user, yes=yes, skip_redis=skip_redis)
 
diff --git a/bench/config/production_setup.py b/bench/config/production_setup.py
index 390c3a57..d110c36c 100755
--- a/bench/config/production_setup.py
+++ b/bench/config/production_setup.py
@@ -9,7 +9,7 @@ import bench
 from bench.config.nginx import make_nginx_conf
 from bench.config.supervisor import (
 	generate_supervisor_config,
-	update_supervisord_config,
+	check_supervisord_config,
 )
 from bench.config.systemd import generate_systemd_config
 from bench.bench import Bench
@@ -48,7 +48,7 @@ def setup_production(user, bench_path=".", yes=False):
 		generate_systemd_config(bench_path=bench_path, user=user, yes=yes)
 	else:
 		print("Setting Up supervisor...")
-		update_supervisord_config(user=user)
+		check_supervisord_config(user=user)
 		generate_supervisor_config(bench_path=bench_path, user=user, yes=yes)
 
 	print("Setting Up NGINX...")
diff --git a/bench/config/supervisor.py b/bench/config/supervisor.py
index 42c7a1e3..d38da668 100644
--- a/bench/config/supervisor.py
+++ b/bench/config/supervisor.py
@@ -79,7 +79,7 @@ def get_supervisord_conf():
 			return possibility
 
 
-def update_supervisord_config(user=None):
+def check_supervisord_config(user=None):
 	"""From bench v5.x, we're moving to supervisor running as user"""
 	# i don't think bench should be responsible for this but we're way past this now...
 	# removed updating supervisord conf & reload in Aug 2022 - gavin@frappe.io

From 0bd0717f24936a51c209fad30ddcfdc3c4e83390 Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Thu, 4 Aug 2022 13:03:22 +0530
Subject: [PATCH 4/6] fix(patch): Rename parent dir instead of moving
 individual dirs

---
 bench/patches/v5/update_archived_sites.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bench/patches/v5/update_archived_sites.py b/bench/patches/v5/update_archived_sites.py
index ce24bb61..3bd2da04 100644
--- a/bench/patches/v5/update_archived_sites.py
+++ b/bench/patches/v5/update_archived_sites.py
@@ -38,8 +38,7 @@ def execute(bench_path):
 	if not os.path.exists(new_directory):
 		os.makedirs(new_directory)
 
-	for archived_site_path in old_directory.glob("*"):
-		archived_site_path.rename(new_directory)
+	old_directory.rename(new_directory)
 
 	click.secho(f"Archived sites are now stored under {new_directory}")
 

From 537012928384b479fd2307b43c90ea768d3d9d39 Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Thu, 4 Aug 2022 12:33:25 +0530
Subject: [PATCH 5/6] feat(nginx): Allow logging settings as site/combined/none

---
 bench/commands/setup.py           |  7 +++++--
 bench/config/nginx.py             |  3 ++-
 bench/config/templates/nginx.conf | 11 +++++++++--
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/bench/commands/setup.py b/bench/commands/setup.py
index 6869d034..4aa4a7fc 100755
--- a/bench/commands/setup.py
+++ b/bench/commands/setup.py
@@ -25,13 +25,16 @@ def setup_sudoers(user):
 
 
 @click.command("nginx", help="Generate configuration files for NGINX")
+@click.option(
+	"--logging", default="combined", type=click.Choice(["none", "site", "combined"])
+)
 @click.option(
 	"--yes", help="Yes to regeneration of nginx config file", default=False, is_flag=True
 )
-def setup_nginx(yes=False):
+def setup_nginx(yes=False, logging="combined"):
 	from bench.config.nginx import make_nginx_conf
 
-	make_nginx_conf(bench_path=".", yes=yes)
+	make_nginx_conf(bench_path=".", yes=yes, logging=logging)
 
 
 @click.command("reload-nginx", help="Checks NGINX config file and reloads service")
diff --git a/bench/config/nginx.py b/bench/config/nginx.py
index a19f6e56..274c4e70 100644
--- a/bench/config/nginx.py
+++ b/bench/config/nginx.py
@@ -13,7 +13,7 @@ from bench.bench import Bench
 from bench.utils import get_bench_name
 
 
-def make_nginx_conf(bench_path, yes=False):
+def make_nginx_conf(bench_path, yes=False, logging=None):
 	conf_path = os.path.join(bench_path, "config", "nginx.conf")
 
 	if not yes and os.path.exists(conf_path):
@@ -43,6 +43,7 @@ def make_nginx_conf(bench_path, yes=False):
 		"allow_rate_limiting": allow_rate_limiting,
 		# for nginx map variable
 		"random_string": "".join(random.choice(string.ascii_lowercase) for i in range(7)),
+		"logging": logging,
 	}
 
 	if allow_rate_limiting:
diff --git a/bench/config/templates/nginx.conf b/bench/config/templates/nginx.conf
index fa09553b..b03f16d2 100644
--- a/bench/config/templates/nginx.conf
+++ b/bench/config/templates/nginx.conf
@@ -114,11 +114,18 @@ server {
 
 	{% endfor -%}
 
-	# logs in var
+	{%- if logging == "site" -%}
+
 	access_log  /var/log/nginx/{{ site_name }}_access.log  main;
 	error_log  /var/log/nginx/{{ site_name }}_error.log;
 
-	
+	{%- elif logging == "combined" -%}
+
+	access_log  /var/log/nginx/access.log  main;
+	error_log  /var/log/nginx/error.log;
+
+	{%- endif %}
+
 	# optimizations
 	sendfile on;
 	keepalive_timeout 15;

From bbf016999450943bc0f7917191265183cf41e265 Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Thu, 4 Aug 2022 15:44:41 +0530
Subject: [PATCH 6/6] fix(nginx): Allow specifying log_format setting

Changes
* Added sugared option class that allows setting options only if another
  is set
* setup nginx command allows to set logging level & log_format options
---
 bench/commands/setup.py           | 12 ++++++++++--
 bench/config/nginx.py             | 12 ++++++++----
 bench/config/templates/nginx.conf | 12 +++++++-----
 bench/utils/cli.py                | 22 ++++++++++++++++++++++
 4 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/bench/commands/setup.py b/bench/commands/setup.py
index 4aa4a7fc..999f8dbd 100755
--- a/bench/commands/setup.py
+++ b/bench/commands/setup.py
@@ -7,6 +7,7 @@ import click
 
 # imports - module imports
 from bench.utils import exec_cmd, run_playbook, which
+from bench.utils.cli import SugaredOption
 
 
 @click.group(help="Setup command group for enabling setting up a Frappe environment")
@@ -28,13 +29,20 @@ def setup_sudoers(user):
 @click.option(
 	"--logging", default="combined", type=click.Choice(["none", "site", "combined"])
 )
+@click.option(
+	"--log_format",
+	help="Specify the log_format for nginx. Use none or '' to not set a value.",
+	only_if_set=["logging"],
+	cls=SugaredOption,
+	default="main",
+)
 @click.option(
 	"--yes", help="Yes to regeneration of nginx config file", default=False, is_flag=True
 )
-def setup_nginx(yes=False, logging="combined"):
+def setup_nginx(yes=False, logging="combined", log_format=None):
 	from bench.config.nginx import make_nginx_conf
 
-	make_nginx_conf(bench_path=".", yes=yes, logging=logging)
+	make_nginx_conf(bench_path=".", yes=yes, logging=logging, log_format=log_format)
 
 
 @click.command("reload-nginx", help="Checks NGINX config file and reloads service")
diff --git a/bench/config/nginx.py b/bench/config/nginx.py
index 274c4e70..1760ccc1 100644
--- a/bench/config/nginx.py
+++ b/bench/config/nginx.py
@@ -9,11 +9,12 @@ import click
 
 # imports - module imports
 import bench
+import bench.config
 from bench.bench import Bench
 from bench.utils import get_bench_name
 
 
-def make_nginx_conf(bench_path, yes=False, logging=None):
+def make_nginx_conf(bench_path, yes=False, logging=None, log_format=None):
 	conf_path = os.path.join(bench_path, "config", "nginx.conf")
 
 	if not yes and os.path.exists(conf_path):
@@ -43,9 +44,14 @@ def make_nginx_conf(bench_path, yes=False, logging=None):
 		"allow_rate_limiting": allow_rate_limiting,
 		# for nginx map variable
 		"random_string": "".join(random.choice(string.ascii_lowercase) for i in range(7)),
-		"logging": logging,
 	}
 
+	if logging and logging != "none":
+		_log_format = ""
+		if log_format and log_format != "none":
+			_log_format = log_format
+		template_vars["logging"] = {"level": logging, "log_format": _log_format}
+
 	if allow_rate_limiting:
 		template_vars.update(
 			{
@@ -281,8 +287,6 @@ def use_wildcard_certificate(bench_path, ret):
 
 
 def get_error_pages():
-	import bench
-
 	bench_app_path = os.path.abspath(bench.__path__[0])
 	templates = os.path.join(bench_app_path, "config", "templates")
 
diff --git a/bench/config/templates/nginx.conf b/bench/config/templates/nginx.conf
index b03f16d2..5cba5782 100644
--- a/bench/config/templates/nginx.conf
+++ b/bench/config/templates/nginx.conf
@@ -31,7 +31,7 @@ server {
 	{% if allow_rate_limiting %}
 	limit_conn per_host_{{ bench_name_hash }} 8;
 	{% endif %}
-	
+
 	proxy_buffer_size 128k;
 	proxy_buffers 4 256k;
 	proxy_busy_buffers_size 256k;
@@ -114,16 +114,18 @@ server {
 
 	{% endfor -%}
 
-	{%- if logging == "site" -%}
+	{% if logging %}
+	{%- if logging.level == "site" -%}
 
-	access_log  /var/log/nginx/{{ site_name }}_access.log  main;
+	access_log  /var/log/nginx/{{ site_name }}_access.log  {{ logging.log_format }};
 	error_log  /var/log/nginx/{{ site_name }}_error.log;
 
-	{%- elif logging == "combined" -%}
+	{%- elif logging.level == "combined" -%}
 
-	access_log  /var/log/nginx/access.log  main;
+	access_log  /var/log/nginx/access.log {{ logging.log_format }};
 	error_log  /var/log/nginx/error.log;
 
+	{%- endif %}
 	{%- endif %}
 
 	# optimizations
diff --git a/bench/utils/cli.py b/bench/utils/cli.py
index 7749e00f..6cae95f6 100644
--- a/bench/utils/cli.py
+++ b/bench/utils/cli.py
@@ -1,3 +1,4 @@
+from typing import List
 import click
 from click.core import _check_multicommand
 
@@ -34,6 +35,27 @@ class MultiCommandGroup(click.Group):
 					self.commands[_name] = cmd
 
 
+class SugaredOption(click.Option):
+	def __init__(self, *args, **kwargs):
+		self.only_if_set: List = kwargs.pop("only_if_set")
+		kwargs["help"] = (
+			kwargs.get("help", "")
+			+ f". Option is acceptable only if {', '.join(self.only_if_set)} is used."
+		)
+		super().__init__(*args, **kwargs)
+
+	def handle_parse_result(self, ctx, opts, args):
+		current_opt = self.name in opts
+		if current_opt and self.only_if_set:
+			for opt in self.only_if_set:
+				if opt not in opts:
+					deafaults_set = [x.default for x in ctx.command.params if x.name == opt]
+					if not deafaults_set:
+						raise click.UsageError(f"Illegal Usage: Set '{opt}' before '{self.name}'.")
+
+		return super().handle_parse_result(ctx, opts, args)
+
+
 def use_experimental_feature(ctx, param, value):
 	if not value:
 		return