Skip to content

Commit

Permalink
fix(nginx): Allow specifying log_format setting
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gavindsouza committed Aug 4, 2022
1 parent 5370129 commit bbf0169
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 11 deletions.
12 changes: 10 additions & 2 deletions bench/commands/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

# 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")
Expand All @@ -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")
Expand Down
12 changes: 8 additions & 4 deletions bench/config/nginx.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@

# 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):
Expand Down Expand Up @@ -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(
{
Expand Down Expand Up @@ -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")

Expand Down
12 changes: 7 additions & 5 deletions bench/config/templates/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions bench/utils/cli.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from typing import List
import click
from click.core import _check_multicommand

Expand Down Expand Up @@ -34,6 +35,27 @@ def add_command(self, cmd, name=None):
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
Expand Down

0 comments on commit bbf0169

Please sign in to comment.