From 40130303ddcc737eb51dc1d0ff92aae0701b4867 Mon Sep 17 00:00:00 2001
From: Jonas L <jooola@users.noreply.github.com>
Date: Sat, 8 Jan 2022 07:16:08 +0100
Subject: [PATCH] feat: replace verbosity flag with log-level flag (#1496)

using a string flag is better when using environement variables.
---
 shared/libretime_shared/app.py     |  7 ++++---
 shared/libretime_shared/cli.py     | 27 +++++++++------------------
 shared/libretime_shared/logging.py | 30 +++++++++++++++++-------------
 shared/tests/logging_test.py       | 25 ++++++++++++++-----------
 4 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/shared/libretime_shared/app.py b/shared/libretime_shared/app.py
index d3fe20b10..114db3398 100644
--- a/shared/libretime_shared/app.py
+++ b/shared/libretime_shared/app.py
@@ -5,7 +5,7 @@ from typing import Optional
 
 from loguru import logger
 
-from .logging import LogLevel, level_from_verbosity, setup_logger
+from .logging import LogLevel, level_from_name, setup_logger
 
 
 # pylint: disable=too-few-public-methods
@@ -26,13 +26,14 @@ class AbstractApp(ABC):
     def __init__(
         self,
         *,
-        verbosity: int,
+        log_level: str,
         log_filepath: Optional[PathLike] = None,
     ):
+        self.log_level = level_from_name(log_level)
+
         if log_filepath is not None:
             self.log_filepath = Path(log_filepath)
 
-        self.log_level = level_from_verbosity(verbosity)
         setup_logger(level=self.log_level, filepath=self.log_filepath)
 
         logger.info(f"Starting {self.name}...")
diff --git a/shared/libretime_shared/cli.py b/shared/libretime_shared/cli.py
index 9ada00807..1b2348078 100644
--- a/shared/libretime_shared/cli.py
+++ b/shared/libretime_shared/cli.py
@@ -3,6 +3,7 @@ from typing import Callable
 import click
 
 from .config import DEFAULT_ENV_PREFIX
+from .logging import INFO, LOG_LEVEL_MAP
 
 
 def cli_logging_options(func: Callable) -> Callable:
@@ -10,26 +11,16 @@ def cli_logging_options(func: Callable) -> Callable:
     Decorator function to add logging options to a click application.
 
     This decorator add the following arguments:
-    - verbosity: int
-    - log_filepath: Path
+    - log_level: str
+    - log_filepath: Optional[Path]
     """
     func = click.option(
-        "-v",
-        "--verbose",
-        "verbosity",
-        envvar=f"{DEFAULT_ENV_PREFIX}_VERBOSITY",
-        count=True,
-        default=0,
-        help="Increase logging verbosity (use -vvv to debug).",
-    )(func)
-
-    func = click.option(
-        "-q",
-        "--quiet",
-        "verbosity",
-        is_flag=True,
-        flag_value=-1,
-        help="Decrease logging verbosity.",
+        "--log-level",
+        "log_level",
+        envvar=f"{DEFAULT_ENV_PREFIX}_LOG_LEVEL",
+        type=click.Choice(list(LOG_LEVEL_MAP.keys())),
+        default=INFO.name,
+        help="Name of the logging level.",
     )(func)
 
     func = click.option(
diff --git a/shared/libretime_shared/logging.py b/shared/libretime_shared/logging.py
index c68403932..0c1366014 100644
--- a/shared/libretime_shared/logging.py
+++ b/shared/libretime_shared/logging.py
@@ -26,23 +26,27 @@ INFO = LogLevel(name="info", no=20)
 DEBUG = LogLevel(name="debug", no=10)
 TRACE = LogLevel(name="trace", no=5)
 
+LOG_LEVEL_MAP = {
+    ERROR.name: ERROR,
+    WARNING.name: WARNING,
+    INFO.name: INFO,
+    DEBUG.name: DEBUG,
+    TRACE.name: TRACE,
+}
 
-def level_from_verbosity(verbosity: int) -> LogLevel:
+
+def level_from_name(name: str) -> LogLevel:
     """
-    Find logging level, depending on the verbosity requested.
+    Find logging level, depending on the name provided.
 
-    -q         -1   =>  ERROR
-    default     0   =>  WARNING
-    -v          1   =>  INFO
-    -vv         2   =>  DEBUG
-    -vvv        3   =>  TRACE
-
-    :param verbosity: verbosity (between -1 and 3) of the logger
-    :returns: log level guessed from the verbosity
+    :param name: name (one of "error", "warning", "info", "debug", "trace") of the log level
+    :returns: log level guessed from the name
+    :raises ValueError: on invalid level name
     """
-    if verbosity < 0:
-        return ERROR
-    return [WARNING, INFO, DEBUG, TRACE][min(3, verbosity)]
+    name = name.lower()
+    if name not in LOG_LEVEL_MAP:
+        raise ValueError(f"invalid level name '{name}'")
+    return LOG_LEVEL_MAP[name]
 
 
 def setup_logger(
diff --git a/shared/tests/logging_test.py b/shared/tests/logging_test.py
index 07af6e6b0..a80512275 100644
--- a/shared/tests/logging_test.py
+++ b/shared/tests/logging_test.py
@@ -7,29 +7,32 @@ from libretime_shared.logging import (
     DEBUG,
     INFO,
     create_task_logger,
-    level_from_verbosity,
+    level_from_name,
     setup_logger,
 )
 
 
 @pytest.mark.parametrize(
-    "verbosity,level_name,level_no",
+    "name,level_name,level_no",
     [
-        (-100, "error", 40),
-        (-1, "error", 40),
-        (0, "warning", 30),
-        (1, "info", 20),
-        (2, "debug", 10),
-        (3, "trace", 5),
-        (100, "trace", 5),
+        ("error", "error", 40),
+        ("warning", "warning", 30),
+        ("info", "info", 20),
+        ("debug", "debug", 10),
+        ("trace", "trace", 5),
     ],
 )
-def test_level_from_verbosity(verbosity, level_name, level_no):
-    level = level_from_verbosity(verbosity)
+def test_level_from_name(name, level_name, level_no):
+    level = level_from_name(name)
     assert level.name == level_name
     assert level.no == level_no
 
 
+def test_level_from_name_invalid():
+    with pytest.raises(ValueError):
+        level_from_name("invalid")
+
+
 def test_setup_logger(tmp_path: Path):
     log_filepath = tmp_path / "test.log"
     extra_log_filepath = tmp_path / "extra.log"