Repository: ModioAB/caramel (language: python)

Issue:
Make it possible to configure with environment variables
Currently the tooling requires a config file on the command line. This is a bit annoying in production and other places, as it means we need various shell scripts to run the code when we could avoid that.

Better way would be to take most of the configuration out of the ini files and move them into environment variables.


1. Change config file from a parameter on the command line to an environment flag that points to the right config file


After that, the basic settings that are in the config file should probably be replaced:

- make log levels a command line option rather than editing configuration files. ( default: warnings and above )
- take database-settings via the environment  ( DB_URL )
- Set paths for the root certificate via environment flags. Error if they aren't set.

The end goals:  

- Admin tools should be usable from containers without dealing with ini-files. 
- Server should be configurable using only environment and/or command line flags




Discussion / hints:

Fix patch:
diff --git a/caramel/config.py b/caramel/config.py
index 6c4120e..3793a45 100644
--- a/caramel/config.py
+++ b/caramel/config.py
@@ -4,8 +4,16 @@
  in one place used by the caramel CLI tools/scripts"""
 
 import argparse
+import logging
 import os
 
+LOG_LEVEL = (
+    logging.ERROR,
+    logging.WARNING,
+    logging.INFO,
+    logging.DEBUG,
+)
+
 
 def add_inifile_argument(parser, env=None):
     """Adds an argument to the parser for the config-file, defaults to
@@ -33,6 +41,18 @@ def add_db_url_argument(parser, env=None):
     )
 
 
+def add_verbosity_argument(parser):
+    """Adds an argument for verbosity to a given parser, counting the amount of
+    'v's and 'verbose' on the commandline"""
+    parser.add_argument(
+        "-v",
+        "--verbose",
+        help="Verbosity of root logger, increasing the more 'v's are added",
+        action="count",
+        default=0,
+    )
+
+
 class CheckInifilePathSet(argparse.Action):
     """An arparse.Action to raise an error if no config file has been
     defined by the user or  in the environment"""
@@ -93,3 +113,30 @@ def get_db_url(arguments=None, settings=None, required=True):
         setting_name="sqlalchemy.url",
         settings=settings,
     )
+
+
+def get_log_level(argument_level, logger=None, env=None):
+    """Calculates the highest verbosity(here inverted) from the argument,
+    environment and root, capping it to between 0-3, returning the log level"""
+
+    if env is None:
+        env = os.environ
+    env_level = int(env.get("CARAMEL_LOG_LEVEL", 0))
+
+    if logger is None:
+        logger = logging.getLogger()
+    current_level = LOG_LEVEL.index(logger.level) if logger.level in LOG_LEVEL else 0
+
+    verbosity = max(argument_level, env_level, current_level)
+    verbosity = min(verbosity, len(LOG_LEVEL) - 1)
+    log_level = LOG_LEVEL[verbosity]
+    return log_level
+
+
+def configure_log_level(arguments: argparse.Namespace, logger=None):
+    """Sets the root loggers level to the highest verbosity from the argument,
+    environment and config-file"""
+    log_level = get_log_level(arguments.verbose)
+    if logger is None:
+        logger = logging.getLogger()
+    logger.setLevel(log_level)
diff --git a/caramel/scripts/autosign.py b/caramel/scripts/autosign.py
index d32c492..238ee11 100644
--- a/caramel/scripts/autosign.py
+++ b/caramel/scripts/autosign.py
@@ -32,7 +32,11 @@
 import uuid
 import concurrent.futures
 
-from pyramid.paster import bootstrap
+from pyramid.paster import (
+    bootstrap,
+    setup_logging,
+)
+
 from sqlalchemy import create_engine
 
 import caramel.models as models
@@ -81,10 +85,14 @@ def mainloop(delay, ca, delta):
 def cmdline():
     """Basically just parsing the arguments and returning them"""
     parser = argparse.ArgumentParser()
+
     config.add_inifile_argument(parser)
     config.add_db_url_argument(parser)
+    config.add_verbosity_argument(parser)
+
     parser.add_argument("--delay", help="How long to sleep. (ms)")
     parser.add_argument("--valid", help="How many hours the certificate is valid for")
+
     args = parser.parse_args()
     return args
 
@@ -98,13 +106,18 @@ def error_out(message, closer):
 
 def main():
     """Main, as called from the script instance by pyramid"""
-    logging.basicConfig()
-    logger.setLevel(logging.DEBUG)
     args = cmdline()
-    env = bootstrap(args.inifile)
+    config_path = args.inifile
+
+    setup_logging(config_path)
+    config.configure_log_level(args)
+
+    env = bootstrap(config_path)
     settings, closer = env["registry"].settings, env["closer"]
+
     db_url = config.get_db_url(args, settings)
     engine = create_engine(db_url)
+
     models.init_session(engine)
     delay = int(settings.get("delay", 500)) / 1000
     valid = int(settings.get("valid", 3))
diff --git a/caramel/scripts/generate_ca.py b/caramel/scripts/generate_ca.py
index 5861003..c1b6dc2 100644
--- a/caramel/scripts/generate_ca.py
+++ b/caramel/scripts/generate_ca.py
@@ -6,7 +6,10 @@
 import uuid
 import os
 import OpenSSL.crypto as _crypto
-from pyramid.paster import bootstrap
+from pyramid.paster import (
+    bootstrap,
+    setup_logging,
+)
 from caramel import config
 
 VERSION = 0x2
@@ -164,6 +167,7 @@ def writefile(data, name):
 def cmdline():
     parser = argparse.ArgumentParser()
     config.add_inifile_argument(parser)
+    config.add_verbosity_argument(parser)
     args = parser.parse_args()
     return args
 
@@ -194,7 +198,12 @@ def build_ca(keyname, certname):
 
 def main():
     args = cmdline()
-    env = bootstrap(args.inifile)
+    config_path = args.inifile
+
+    setup_logging(config_path)
+    config.configure_log_level(args)
+
+    env = bootstrap(config_path)
     settings, closer = env["registry"].settings, env["closer"]
 
     ca_cert = settings.get("ca.cert")
diff --git a/caramel/scripts/initializedb.py b/caramel/scripts/initializedb.py
index 8b8ba30..7ea0b63 100644
--- a/caramel/scripts/initializedb.py
+++ b/caramel/scripts/initializedb.py
@@ -15,17 +15,23 @@
 
 def cmdline():
     parser = argparse.ArgumentParser()
+
     config.add_inifile_argument(parser)
     config.add_db_url_argument(parser)
+    config.add_verbosity_argument(parser)
+
     args = parser.parse_args()
     return args
 
 
 def main():
     args = cmdline()
-    config_uri = args.inifile
-    setup_logging(config_uri)
-    settings = get_appsettings(config_uri)
+    config_path = args.inifile
+    settings = get_appsettings(config_path)
+
+    setup_logging(config_path)
+    config.configure_log_level(args)
+
     db_url = config.get_db_url(args, settings)
     engine = create_engine(db_url)
     init_session(engine, create=True)


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index 57464ac..5918131 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -3,6 +3,7 @@
 """tests.test_config contains the unittests for caramel.config"""
 import argparse
 import unittest
+import logging
 
 from caramel import config
 
@@ -174,3 +175,55 @@ def test_requierd_nothing(self):
                 settings={},
                 env={},
             )
+
+
+class TestVerbosity(unittest.TestCase):
+    """Tests for the verbosity configuration from caramel.config"""
+
+    """Data set used for testing config.get_log_level,
+    (argument_level, environment, root_level, expected)"""
+    VERBOSITY_DATA: tuple = (
+        (2, {}, logging.CRITICAL, logging.INFO),
+        (-1, {"CARAMEL_LOG_LEVEL": -1}, logging.CRITICAL, logging.ERROR),
+        (2, {"CARAMEL_LOG_LEVEL": 1}, logging.ERROR, logging.INFO),
+        (2, {"CARAMEL_LOG_LEVEL": 3}, logging.WARNING, logging.DEBUG),
+        (2, {"CARAMEL_LOG_LEVEL": 1}, logging.DEBUG, logging.DEBUG),
+        (2, {"CARAMEL_LOG_LEVEL": 3}, logging.WARNING, logging.DEBUG),
+        (0, {"CARAMEL_LOG_LEVEL": 2}, logging.DEBUG, logging.DEBUG),
+        (0, {"CARAMEL_LOG_LEVEL": 3}, logging.WARNING, logging.DEBUG),
+    )
+
+    """Data set used for testing config.add_verbosity_argument,
+    ([arguments], expected)"""
+    VERBOSITY_ARGUMENT_DATA: tuple = (
+        ([], 0),
+        (["-v"], 1),
+        (["-vv"], 2),
+        (["-vvv"], 3),
+        (["-vvvv"], 4),
+        (["--verbose", "-v"], 2),
+        (["--verbose", "-vv"], 3),
+        (["--verbose", "-v", "--verbose"], 3),
+    )
+
+    def test_verbosity_argument(self):
+        """tests multiple cases for config.add_verbosity_argument"""
+
+        for arg, expected in TestVerbosity.VERBOSITY_ARGUMENT_DATA:
+            with self.subTest(arg=arg, expected=expected):
+                parser = argparse.ArgumentParser()
+                config.add_verbosity_argument(parser)
+                args = parser.parse_args(arg)
+                self.assertEqual(expected, args.verbose)
+
+    def test_get_log_level(self):
+        """tests multiple cases for config.get_log_level"""
+        logger = logging.getLogger("fake")
+
+        for arg_lvl, env, root_lvl, expected in TestVerbosity.VERBOSITY_DATA:
+            with self.subTest(
+                arg_lvl=arg_lvl, env=env, root_lvl=root_lvl, expected=expected
+            ):
+                logger.setLevel(root_lvl)
+                verbosity = config.get_log_level(arg_lvl, logger, env)
+                self.assertEqual(expected, verbosity)