Repository: markusressel/container-app-conf

Issue:
Rename 'none_allowed' to 'required' and invert meaning
`none_allowed` is a very technical term and although accurate, a more descriptive name like `required` should make its use more intuitive.

Discussion:

Patch:
diff --git a/README.md b/README.md
index f7cff93..5d0888d 100644
--- a/README.md
+++ b/README.md
@@ -36,7 +36,7 @@ class AppConfig(ConfigBase):
             "my_app",
             "example"
         ],
-        none_allowed=False)
+        required=True)
 
 ```
 
@@ -63,14 +63,20 @@ own by extending the `ConfigEntry` base class.
 A default value can be specified for every `ConfigEntry` by using the
 `default` constructor parameter.
 
-## Allow `None` 
+## Required values
 
-By default a `None` value is only allowed if the default value is `None`.
+By default a `None` value is only allowed for an entry if it has no 
+default (or it is set to `None`).
 This means it is not possible to set the `MY_CONFIG` entry in the example
-at the top to `None` even after initial parsing. Specifying an empty text
-in the yaml or corresponding environment variable will result in an
-exception. If you want to allow setting a `None` value even if the default 
-value is **not** `None`, use the `none_allowed=True` constructor parameter.
+at the top to `None` even after initial parsing. Omitting a value for 
+this entry in all data sources or specifying an empty text in the yaml
+or corresponding environment variable will result in an exception.
+
+If an entry requires a value and has no default set the `required`
+constructor parameter to `True`.
+
+If you want to allow setting a `None` value even if the default value 
+is **not** `None`, you have to explicitly set `required=False`.
 
 ## Data sources
 
diff --git a/container_app_conf/entry/__init__.py b/container_app_conf/entry/__init__.py
index 94f3e11..c892a15 100644
--- a/container_app_conf/entry/__init__.py
+++ b/container_app_conf/entry/__init__.py
@@ -22,19 +22,21 @@ import re
 
 from container_app_conf.const import KEY_PATH_REGEX
 
+LOGGER = logging.Logger(__name__)
+
 
 class ConfigEntry:
     _example = None
 
     def __init__(self, key_path: [str], example: any = None, description: str or None = None, default: any = None,
-                 none_allowed: bool = None):
+                 required: bool = None):
         """
         Creates a config entry
         :param key_path: list of strings representing f.ex. the yaml tree path
         :param example: example str value
         :param description: a description of this entry
         :param default: the default value
-        :param none_allowed: Set to True if a 'None' value may be allowed, False if not,
+        :param required: Set to True if a 'None' value may be allowed, False if not,
                              otherwise it will be True if the default value is not None.
         """
         if len(key_path) <= 0:
@@ -50,11 +52,11 @@ class ConfigEntry:
         if example is not None:
             self._example = example
 
-        if none_allowed is None:
-            none_allowed = default is None
-        self._none_allowed = none_allowed
+        if required is None:
+            required = default is not None
+        self._required = required
 
-        if default is not None or self._none_allowed:
+        if default is not None:
             self.default = self._parse_value(default)
         else:
             self.default = None
@@ -85,16 +87,24 @@ class ConfigEntry:
         :return: the parsed value
         """
         if value is None:
-            if self._none_allowed:
+            if not self._required:
                 return None
             else:
-                self._raise_invalid_value(value)
+                self._raise_invalid_value(value, "Value is required")
 
         try:
-            return self._value_to_type(value)
+            parsed_value = self._value_to_type(value)
+
+            if parsed_value is None:
+                if not self._required:
+                    return None
+                else:
+                    self._raise_invalid_value(parsed_value, "Value is required")
+
+            return parsed_value
         except Exception as ex:
-            logging.exception(ex)
-            self._raise_invalid_value(value)
+            LOGGER.exception(ex)
+            self._raise_invalid_value(value, ex)
 
     def _value_to_type(self, value: any) -> any:
         """
@@ -113,5 +123,9 @@ class ConfigEntry:
         """
         return str(type)
 
-    def _raise_invalid_value(self, value: any):
-        raise ValueError("Invalid value '{}' for config option `{}`".format(value, ">".join(self.key_path)))
+    def _raise_invalid_value(self, value: any, reason: str or None = None):
+        entry_key_path = ">".join(self.key_path)
+        message = "Invalid value '{}' for config option `{}`".format(value, entry_key_path)
+        if reason is not None:
+            message += ": {}".format(reason)
+        raise ValueError(message)
diff --git a/container_app_conf/entry/bool.py b/container_app_conf/entry/bool.py
index 58469c3..49de2a7 100644
--- a/container_app_conf/entry/bool.py
+++ b/container_app_conf/entry/bool.py
@@ -40,7 +40,7 @@ class BoolConfigEntry(ConfigEntry):
             elif s in ['n', 'no', 'false', 'f', '0']:
                 return False
             else:
-                self._raise_invalid_value(value)
+                raise ValueError("Unsupported value")
 
     def _type_to_value(self, type: any) -> any:
         return bool(type)
diff --git a/container_app_conf/entry/date.py b/container_app_conf/entry/date.py
index 81fe6bc..13c42bf 100644
--- a/container_app_conf/entry/date.py
+++ b/container_app_conf/entry/date.py
@@ -40,7 +40,7 @@ class DateConfigEntry(ConfigEntry):
         elif isinstance(value, str):
             return dateutil.parser.parse(value)
         else:
-            self._raise_invalid_value(value)
+            raise ValueError("Unsupported type: {}".format(type(value)))
 
     def _type_to_value(self, type: any) -> any:
         if isinstance(type, datetime):
diff --git a/container_app_conf/entry/file.py b/container_app_conf/entry/file.py
index c411bab..6e3b957 100644
--- a/container_app_conf/entry/file.py
+++ b/container_app_conf/entry/file.py
@@ -27,12 +27,12 @@ class FileConfigEntry(ConfigEntry):
     _example = "/tmp/temporary"
 
     def __init__(self, key_path: [str], example: any = None, description: str or None = None, default: any = None,
-                 none_allowed: bool = None, check_existence: bool = False):
+                 required: bool = None, check_existence: bool = False):
         """
         See ConfigEntry
         :param check_existence: whether to check for file existence
         """
-        super().__init__(key_path, example, description, default, none_allowed)
+        super().__init__(key_path, example, description, default, required)
         self.check_existence = check_existence
 
     def _value_to_type(self, value: any) -> str or None:
@@ -63,12 +63,12 @@ class DirectoryConfigEntry(ConfigEntry):
     _example = "/tmp"
 
     def __init__(self, key_path: [str], example: any = None, description: str or None = None, default: any = None,
-                 none_allowed: bool = None, check_existence: bool = False):
+                 required: bool = None, check_existence: bool = False):
         """
         See ConfigEntry
         :param check_existence: whether to check for folder existence
         """
-        super().__init__(key_path, example, description, default, none_allowed)
+        super().__init__(key_path, example, description, default, required)
         self.check_existence = check_existence
 
     def _value_to_type(self, value: any) -> str or None:
diff --git a/container_app_conf/entry/float.py b/container_app_conf/entry/float.py
index ee85223..7a64e46 100644
--- a/container_app_conf/entry/float.py
+++ b/container_app_conf/entry/float.py
@@ -26,9 +26,9 @@ class FloatConfigEntry(ConfigEntry):
     _example = "3.1415926535"
 
     def __init__(self, key_path: [str], example: any = None, description: str or None = None, default: any = None,
-                 none_allowed: bool = None, range: Range = None):
+                 required: bool = None, range: Range = None):
         self.range = range
-        super().__init__(key_path, example, description, default, none_allowed)
+        super().__init__(key_path, example, description, default, required)
 
     def _value_to_type(self, value: any) -> float or None:
         """
@@ -42,7 +42,7 @@ class FloatConfigEntry(ConfigEntry):
             parsed_value = float(value)
 
         if self.range is not None and parsed_value not in self.range:
-            self._raise_invalid_value(value)
+            raise ValueError("Value not in range")
         return parsed_value
 
     def _type_to_value(self, type: any) -> any:
diff --git a/container_app_conf/entry/int.py b/container_app_conf/entry/int.py
index beb6ad1..b08fca1 100644
--- a/container_app_conf/entry/int.py
+++ b/container_app_conf/entry/int.py
@@ -26,9 +26,9 @@ class IntConfigEntry(ConfigEntry):
     _example = "42"
 
     def __init__(self, key_path: [str], example: any = None, description: str or None = None, default: any = None,
-                 none_allowed: bool = None, range: Range = None):
+                 required: bool = None, range: Range = None):
         self.range = range
-        super().__init__(key_path, example, description, default, none_allowed)
+        super().__init__(key_path, example, description, default, required)
 
     def _value_to_type(self, value: any) -> int or None:
         """
@@ -38,7 +38,7 @@ class IntConfigEntry(ConfigEntry):
         """
         parsed_value = int(value)
         if self.range is not None and parsed_value not in self.range:
-            self._raise_invalid_value(value)
+            raise ValueError("Value not in range")
         return parsed_value
 
     def _type_to_value(self, type: any) -> any:
diff --git a/container_app_conf/entry/list.py b/container_app_conf/entry/list.py
index 8172f44..1cd7d5a 100644
--- a/container_app_conf/entry/list.py
+++ b/container_app_conf/entry/list.py
@@ -29,7 +29,7 @@ class ListConfigEntry(ConfigEntry):
 
     def __init__(self, item_type: Type[ConfigEntry], key_path: [str], example: any = None,
                  description: str or None = None,
-                 default: any = None, none_allowed: bool = None,
+                 default: any = None, required: bool = None,
                  item_args: dict = None,
                  delimiter: str = None):
         """
@@ -40,7 +40,7 @@ class ListConfigEntry(ConfigEntry):
         :param example: example str value
         :param description: a description of this list entry
         :param default: default value
-        :param none_allowed: if None is allowed for this config entry
+        :param required: if None is allowed for this config entry
         """
         if item_args is None:
             item_args = {}
@@ -52,7 +52,7 @@ class ListConfigEntry(ConfigEntry):
             description=description,
             example=example,
             default=default,
-            none_allowed=none_allowed
+            required=required
         )
 
     @property
diff --git a/container_app_conf/entry/range.py b/container_app_conf/entry/range.py
index 194627b..e1e1e8d 100644
--- a/container_app_conf/entry/range.py
+++ b/container_app_conf/entry/range.py
@@ -39,4 +39,4 @@ class RangeConfigEntry(ConfigEntry):
                 raise ValueError("Cannot parse the range format: {}".format(value))
             return parsed
         else:
-            self._raise_invalid_value(value)
+            raise ValueError("Unsupported type: {}".format(type(value)))
diff --git a/container_app_conf/entry/regex.py b/container_app_conf/entry/regex.py
index c950ae8..148b6d7 100644
--- a/container_app_conf/entry/regex.py
+++ b/container_app_conf/entry/regex.py
@@ -33,22 +33,22 @@ class RegexConfigEntry(ConfigEntry):
     _example = r"^[a-zA-z0-9]*$"
 
     def __init__(self, key_path: [str], example: any = None, description: str or None = None, default: any = None,
-                 none_allowed: bool = None, flags: int or None = None):
+                 required: bool = None, flags: int or None = None):
         """
         :param flags: Regex flags to mandate or None if no flag restrictions should be made
         """
         self.flags = flags | re.UNICODE if flags is not None else None
-        super().__init__(key_path, example, description, default, none_allowed)
+        super().__init__(key_path, example, description, default, required)
 
     def _value_to_type(self, value: any) -> Pattern or None:
-        if value is None and self._none_allowed:
+        if value is None and self._required:
             return None
 
         if isinstance(value, Pattern) and self.flags is not None:
             if value.flags == int(self.flags):
                 return value
             else:
-                self._raise_invalid_value(value)
+                raise ValueError("Value does not match expected flags: {}".format(self.flags))
 
         value = str(value)
         return re.compile(value, flags=self.flags if self.flags is not None else 0)
diff --git a/container_app_conf/entry/string.py b/container_app_conf/entry/string.py
index e2d2a6a..96b04b5 100644
--- a/container_app_conf/entry/string.py
+++ b/container_app_conf/entry/string.py
@@ -26,9 +26,9 @@ class StringConfigEntry(ConfigEntry):
     _example = "text"
 
     def __init__(self, key_path: [str], example: any = None, description: str or None = None, default: any = None,
-                 none_allowed: bool = None, regex: str = None):
+                 required: bool = None, regex: str = None):
         self.regex = re.compile(regex) if regex is not None else None
-        super().__init__(key_path, example, description, default, none_allowed)
+        super().__init__(key_path, example, description, default, required)
 
     def _value_to_type(self, value: any) -> str or None:
         """
@@ -37,12 +37,8 @@ class StringConfigEntry(ConfigEntry):
         :return: parsed value
         """
         s = str(value)
-        if self._none_allowed:
-            if s.lower() in ['none', 'null', 'nil']:
-                return None
-
         if self.regex is not None:
             if not self.regex.match(s):
-                self._raise_invalid_value(s)
+                raise ValueError("Value doesn't match regex: {}".format(self.regex))
 
         return s
diff --git a/container_app_conf/entry/timedelta.py b/container_app_conf/entry/timedelta.py
index 8f85c51..338db6c 100644
--- a/container_app_conf/entry/timedelta.py
+++ b/container_app_conf/entry/timedelta.py
@@ -41,4 +41,4 @@ class TimeDeltaConfigEntry(ConfigEntry):
                 raise ValueError("Cannot parse the given timedelta format: {}".format(value))
             return timedelta(seconds=parsed)
         else:
-            self._raise_invalid_value(value)
+            raise ValueError("Unsupported type: {}".format(type(value)))


Test patch:
diff --git a/tests/entry_test.py b/tests/entry_test.py
index e65d845..8906210 100644
--- a/tests/entry_test.py
+++ b/tests/entry_test.py
@@ -37,19 +37,21 @@ from tests import EntryTestBase
 class EntryTest(EntryTestBase):
 
     def test_string_entry(self):
-        config_entry = StringConfigEntry(key_path=["string"], none_allowed=True)
+        config_entry = StringConfigEntry(key_path=["string"], required=False)
 
         input_output = [
             ("5", "5"),
             ("hello", "hello"),
             ("$stuff=)(&/%$§", "$stuff=)(&/%$§"),
-            ("None", None)
+            ("", ""),
+            ("None", "None"),
+            (None, None)
         ]
 
         self.assert_input_output(config_entry, input_output)
 
     def test_string_entry_regex(self):
-        config_entry = StringConfigEntry(key_path=["string"], none_allowed=True, regex="^[0-9]*$")
+        config_entry = StringConfigEntry(key_path=["string"], required=True, regex="^[0-9]*$")
 
         input_output = [
             ("5", "5"),