From 6427a32072530864008c66650eeeb030697df9fb Mon Sep 17 00:00:00 2001 From: Finn-Thorben Sell Date: Fri, 18 Mar 2022 21:12:49 +0100 Subject: [PATCH] raise specific exceptions during value retrieval and processing This makes it easier to determine what exactly went wrong and thus build better error handling in a later commit. Tests were adapted accordingly to assert that only those specific errors are raised instead of the plain ValueErrors. --- configurations/values.py | 72 ++++++++++++++++++++++++++++------------ tests/test_values.py | 60 ++++++++++++++++++++------------- 2 files changed, 87 insertions(+), 45 deletions(-) diff --git a/configurations/values.py b/configurations/values.py index 8eb3bbc..215091d 100644 --- a/configurations/values.py +++ b/configurations/values.py @@ -11,6 +11,31 @@ from django.utils.module_loading import import_string from .utils import getargspec +class ValueRetrievalError(ValueError): + """ + Exception is raised when errors occur during the retrieval of a dynamic Value. + This can happen when the environment variable corresponding to the value is not defined. + """ + + def __init__(self, value_instance: 'Value', error_msg: str) -> None: + super().__init__(error_msg) + self.value_instance = value_instance + self.error_msg = error_msg + + +class ValueProcessingError(ValueError): + """ + Exception that is raised when a dynamic Value failed to be processed after retrieval. + Processing could be i.e. converting from string to a native datatype but also validation. + """ + + def __init__(self, value_instance: 'Value', str_value: str, error_msg: str) -> None: + super().__init__(error_msg) + self.value_instance = value_instance + self.str_value = str_value + self.error_msg = error_msg + + def setup_value(target, name, value): actual_value = value.setup(name) # overwriting the original Value class with the result @@ -102,9 +127,9 @@ class Value: if full_environ_name in os.environ: value = self.to_python(os.environ[full_environ_name]) elif self.environ_required: - raise ValueError('Value {0!r} is required to be set as the ' - 'environment variable {1!r}' - .format(name, full_environ_name)) + raise ValueRetrievalError(self, 'Value {0!r} is required to be set as the ' + 'environment variable {1!r}' + .format(name, full_environ_name)) self.value = value return value @@ -128,8 +153,8 @@ class BooleanValue(Value): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) if self.default not in (True, False): - raise ValueError('Default value {0!r} is not a ' - 'boolean value'.format(self.default)) + raise ImproperlyConfigured('Default value {0!r} is not a ' + 'boolean value'.format(self.default)) def to_python(self, value): normalized_value = value.strip().lower() @@ -138,8 +163,8 @@ class BooleanValue(Value): elif normalized_value in self.false_values: return False else: - raise ValueError('Cannot interpret ' - 'boolean value {0!r}'.format(value)) + raise ValueProcessingError(self, value, 'Cannot interpret ' + 'boolean value {0!r}'.format(value)) class CastingMixin: @@ -159,7 +184,7 @@ class CastingMixin: else: error = 'Cannot use caster of {0} ({1!r})'.format(self, self.caster) - raise ValueError(error) + raise ImproperlyConfigured(error) try: arg_names = getargspec(self._caster)[0] self._params = {name: kwargs[name] for name in arg_names if name in kwargs} @@ -173,7 +198,7 @@ class CastingMixin: else: return self._caster(value) except self.exception: - raise ValueError(self.message.format(value)) + raise ValueProcessingError(self, value, self.message.format(value)) class IntegerValue(CastingMixin, Value): @@ -185,7 +210,7 @@ class PositiveIntegerValue(IntegerValue): def to_python(self, value): int_value = super().to_python(value) if int_value < 0: - raise ValueError(self.message.format(value)) + raise ValueProcessingError(self, value, self.message.format(value)) return int_value @@ -231,7 +256,7 @@ class SequenceValue(Value): try: converted_values.append(self.converter(value)) except (TypeError, ValueError): - raise ValueError(self.message.format(value, value)) + raise ValueProcessingError(self, self.separator.join(sequence), self.message.format(value, value)) return self.sequence_type(converted_values) def to_python(self, value): @@ -296,7 +321,7 @@ class BackendsValue(ListValue): try: import_string(value) except ImportError as err: - raise ValueError(err).with_traceback(sys.exc_info()[2]) + raise ValueProcessingError(self, value, str(err)).with_traceback(sys.exc_info()[2]) return value @@ -331,9 +356,9 @@ class DictValue(Value): try: evaled_value = ast.literal_eval(value) except ValueError: - raise ValueError(self.message.format(value)) + raise ValueProcessingError(self, value, self.message.format(value)) if not isinstance(evaled_value, dict): - raise ValueError(self.message.format(value)) + raise ValueProcessingError(self, value, self.message.format(value)) return evaled_value @@ -350,16 +375,19 @@ class ValidationMixin: elif callable(self.validator): self._validator = self.validator else: - raise ValueError('Cannot use validator of ' - '{0} ({1!r})'.format(self, self.validator)) + raise ImproperlyConfigured('Cannot use validator of ' + '{0} ({1!r})'.format(self, self.validator)) if self.default: - self.to_python(self.default) + try: + self.to_python(self.default) + except ValueProcessingError as e: + raise ImproperlyConfigured(e.error_msg) def to_python(self, value): try: self._validator(value) except ValidationError: - raise ValueError(self.message.format(value)) + raise ValueProcessingError(self, value, self.message.format(value)) else: return value @@ -397,7 +425,7 @@ class PathValue(Value): value = super().setup(name) value = os.path.expanduser(value) if self.check_exists and not os.path.exists(value): - raise ValueError('Path {0!r} does not exist.'.format(value)) + raise ValueProcessingError(self, value, 'Path {0!r} does not exist.'.format(value)) return os.path.abspath(value) @@ -408,13 +436,13 @@ class SecretValue(Value): kwargs['environ_required'] = True super().__init__(*args, **kwargs) if self.default is not None: - raise ValueError('Secret values are only allowed to ' - 'be set as environment variables') + raise ImproperlyConfigured('Secret values are only allowed to ' + 'be set as environment variables') def setup(self, name): value = super().setup(name) if not value: - raise ValueError('Secret value {0!r} is not set'.format(name)) + raise ValueRetrievalError(self, 'Secret value {0!r} is not set'.format(name)) return value diff --git a/tests/test_values.py b/tests/test_values.py index 2547e50..6eb1ca4 100644 --- a/tests/test_values.py +++ b/tests/test_values.py @@ -15,8 +15,8 @@ from configurations.values import (Value, BooleanValue, IntegerValue, RegexValue, PathValue, SecretValue, DatabaseURLValue, EmailURLValue, CacheURLValue, BackendsValue, - CastingMixin, SearchURLValue, - setup_value, PositiveIntegerValue) + SearchURLValue, PositiveIntegerValue, CastingMixin, + setup_value, ValueRetrievalError, ValueProcessingError) @contextmanager @@ -58,6 +58,19 @@ class ValueTests(TestCase): self.assertEqual(repr(value), repr('override')) + def test_environ_required(self): + for ValueClass in (Value, BooleanValue, IntegerValue, + FloatValue, DecimalValue, ListValue, + TupleValue, SingleNestedTupleValue, + SingleNestedListValue, SetValue, + DictValue, URLValue, EmailValue, IPValue, + RegexValue, PathValue, SecretValue, + DatabaseURLValue, EmailURLValue, + CacheURLValue, BackendsValue, + SearchURLValue, PositiveIntegerValue): + value = ValueClass(environ_required=True) + self.assertRaises(ValueRetrievalError, value.setup, "TEST") + def test_value_truthy(self): value = Value('default') self.assertTrue(bool(value)) @@ -110,7 +123,7 @@ class ValueTests(TestCase): self.assertTrue(bool(value.setup('TEST'))) def test_boolean_values_faulty(self): - self.assertRaises(ValueError, BooleanValue, 'false') + self.assertRaises(ImproperlyConfigured, BooleanValue, 'false') def test_boolean_values_false(self): value = BooleanValue(True) @@ -121,7 +134,7 @@ class ValueTests(TestCase): def test_boolean_values_nonboolean(self): value = BooleanValue(True) with env(DJANGO_TEST='nonboolean'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_boolean_values_assign_false_to_another_booleanvalue(self): value1 = BooleanValue(False) @@ -134,30 +147,30 @@ class ValueTests(TestCase): with env(DJANGO_TEST='2'): self.assertEqual(value.setup('TEST'), 2) with env(DJANGO_TEST='noninteger'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_positive_integer_values(self): value = PositiveIntegerValue(1) with env(DJANGO_TEST='2'): self.assertEqual(value.setup('TEST'), 2) with env(DJANGO_TEST='noninteger'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') with env(DJANGO_TEST='-1'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_float_values(self): value = FloatValue(1.0) with env(DJANGO_TEST='2.0'): self.assertEqual(value.setup('TEST'), 2.0) with env(DJANGO_TEST='noninteger'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_decimal_values(self): value = DecimalValue(decimal.Decimal(1)) with env(DJANGO_TEST='2'): self.assertEqual(value.setup('TEST'), decimal.Decimal(2)) with env(DJANGO_TEST='nondecimal'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_failing_caster(self): self.assertRaises(ImproperlyConfigured, FailingCasterValue) @@ -194,7 +207,7 @@ class ValueTests(TestCase): def test_list_values_converter_exception(self): value = ListValue(converter=int) with env(DJANGO_TEST='2,b'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_tuple_values_default(self): value = TupleValue() @@ -292,21 +305,21 @@ class ValueTests(TestCase): with env(DJANGO_TEST=''): self.assertEqual(value.setup('TEST'), {}) with env(DJANGO_TEST='spam'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_email_values(self): value = EmailValue('spam@eg.gs') with env(DJANGO_TEST='spam@sp.am'): self.assertEqual(value.setup('TEST'), 'spam@sp.am') with env(DJANGO_TEST='spam'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_url_values(self): value = URLValue('http://eggs.spam') with env(DJANGO_TEST='http://spam.eggs'): self.assertEqual(value.setup('TEST'), 'http://spam.eggs') with env(DJANGO_TEST='httb://spam.eggs'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_url_values_with_no_default(self): value = URLValue() # no default @@ -314,7 +327,7 @@ class ValueTests(TestCase): self.assertEqual(value.setup('TEST'), 'http://spam.eggs') def test_url_values_with_wrong_default(self): - self.assertRaises(ValueError, URLValue, 'httb://spam.eggs') + self.assertRaises(ImproperlyConfigured, URLValue, 'httb://spam.eggs') def test_ip_values(self): value = IPValue('0.0.0.0') @@ -323,14 +336,14 @@ class ValueTests(TestCase): with env(DJANGO_TEST='::1'): self.assertEqual(value.setup('TEST'), '::1') with env(DJANGO_TEST='spam.eggs'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_regex_values(self): value = RegexValue('000--000', regex=r'\d+--\d+') with env(DJANGO_TEST='123--456'): self.assertEqual(value.setup('TEST'), '123--456') with env(DJANGO_TEST='123456'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_path_values_with_check(self): value = PathValue() @@ -339,7 +352,7 @@ class ValueTests(TestCase): with env(DJANGO_TEST='~/'): self.assertEqual(value.setup('TEST'), os.path.expanduser('~')) with env(DJANGO_TEST='/does/not/exist'): - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_path_values_no_check(self): value = PathValue(check_exists=False) @@ -354,17 +367,17 @@ class ValueTests(TestCase): def test_secret_value(self): # no default allowed, only environment values are - self.assertRaises(ValueError, SecretValue, 'default') + self.assertRaises(ImproperlyConfigured, SecretValue, 'default') value = SecretValue() - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueRetrievalError, value.setup, 'TEST') with env(DJANGO_SECRET_KEY='123'): self.assertEqual(value.setup('SECRET_KEY'), '123') value = SecretValue(environ_name='FACEBOOK_API_SECRET', environ_prefix=None, late_binding=True) - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueRetrievalError, value.setup, 'TEST') with env(FACEBOOK_API_SECRET='123'): self.assertEqual(value.setup('TEST'), '123') @@ -424,7 +437,7 @@ class ValueTests(TestCase): 'EMAIL_USE_SSL': False, 'EMAIL_USE_TLS': False}) with env(EMAIL_URL='smtps://user@domain.com:password@smtp.example.com:wrong'): # noqa: E501 - self.assertRaises(ValueError, value.setup, 'TEST') + self.assertRaises(ValueProcessingError, value.setup, 'TEST') def test_cache_url_value(self): cache_setting = { @@ -445,7 +458,7 @@ class ValueTests(TestCase): value.setup('TEST') self.assertEqual(cm.exception.args[0], 'Unknown backend: "wrong"') with env(CACHE_URL='redis://user@host:port/1'): - with self.assertRaises(ValueError) as cm: + with self.assertRaises(ValueProcessingError) as cm: value.setup('TEST') self.assertEqual( cm.exception.args[0], @@ -468,7 +481,7 @@ class ValueTests(TestCase): self.assertEqual(value.setup('TEST'), backends) backends = ['non.existing.Backend'] - self.assertRaises(ValueError, BackendsValue, backends) + self.assertRaises(ValueProcessingError, BackendsValue, backends) def test_tuple_value(self): value = TupleValue(None) @@ -503,6 +516,7 @@ class ValueTests(TestCase): 'EMAIL_HOST_PASSWORD': 'password', 'EMAIL_HOST_USER': 'user@domain.com', 'EMAIL_PORT': 587, + 'EMAIL_TIMEOUT': None, 'EMAIL_USE_SSL': False, 'EMAIL_USE_TLS': True })