From dacca7e0736d444bac8245c969250878cfc976e9 Mon Sep 17 00:00:00 2001 From: Jannis Leidel Date: Fri, 6 Sep 2013 22:40:15 +0200 Subject: [PATCH] Use ValueError instead of Django's ImproperlyConfigured. This is to prevent hiding the real reason of a configuration failure behind one of Django's import time exceptions. --- configurations/tests/test_values.py | 39 ++++++++++++++++----------- configurations/values.py | 42 ++++++++++++++--------------- docs/values.rst | 6 ++--- 3 files changed, 46 insertions(+), 41 deletions(-) diff --git a/configurations/tests/test_values.py b/configurations/tests/test_values.py index 99a4688..0dc2271 100644 --- a/configurations/tests/test_values.py +++ b/configurations/tests/test_values.py @@ -68,7 +68,7 @@ class ValueTests(TestCase): self.assertTrue(value.setup('TEST')) def test_boolean_values_faulty(self): - self.assertRaises(ImproperlyConfigured, BooleanValue, 'false') + self.assertRaises(ValueError, BooleanValue, 'false') def test_boolean_values_false(self): value = BooleanValue(True) @@ -79,28 +79,28 @@ class ValueTests(TestCase): def test_boolean_values_nonboolean(self): value = BooleanValue(True) with env(DJANGO_TEST='nonboolean'): - self.assertRaises(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, value.setup, 'TEST') def test_integer_values(self): value = IntegerValue(1) with env(DJANGO_TEST='2'): self.assertEqual(value.setup('TEST'), 2) with env(DJANGO_TEST='noninteger'): - self.assertRaises(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, 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(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, 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(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, value.setup, 'TEST') def test_failing_caster(self): self.assertRaises(ImproperlyConfigured, FailingCasterValue) @@ -137,7 +137,7 @@ class ValueTests(TestCase): def test_list_values_converter_exception(self): value = ListValue(converter=int) with env(DJANGO_TEST='2,b'): - self.assertRaises(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, value.setup, 'TEST') def test_tuple_values_default(self): value = TupleValue() @@ -173,21 +173,21 @@ class ValueTests(TestCase): with env(DJANGO_TEST=''): self.assertEqual(value.setup('TEST'), {}) with env(DJANGO_TEST='spam'): - self.assertRaises(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, 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(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, 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(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, value.setup, 'TEST') def test_ip_values(self): value = IPValue('0.0.0.0') @@ -196,14 +196,14 @@ class ValueTests(TestCase): with env(DJANGO_TEST='::1'): self.assertEqual(value.setup('TEST'), '::1') with env(DJANGO_TEST='spam.eggs'): - self.assertRaises(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, 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(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, value.setup, 'TEST') def test_path_values_with_check(self): value = PathValue() @@ -212,7 +212,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(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, value.setup, 'TEST') def test_path_values_no_check(self): value = PathValue(check_exists=False) @@ -226,12 +226,19 @@ class ValueTests(TestCase): self.assertEqual(value.setup('TEST'), '/does/not/exist') def test_secret_value(self): - self.assertRaises(ImproperlyConfigured, SecretValue, 'default') + self.assertRaises(ValueError, SecretValue, 'default') + value = SecretValue() - self.assertRaises(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, 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) + self.assertRaises(ValueError, value.setup, 'TEST') + with env(FACEBOOK_API_SECRET='123'): + self.assertEqual(value.setup('TEST'), '123') + def test_database_url_value(self): value = DatabaseURLValue() self.assertEqual(value.default, {}) @@ -268,7 +275,7 @@ class ValueTests(TestCase): 'EMAIL_PORT': None, 'EMAIL_USE_TLS': False}) with env(EMAIL_URL='smtps://user@domain.com:password@smtp.example.com:wrong'): - self.assertRaises(ImproperlyConfigured, value.setup, 'TEST') + self.assertRaises(ValueError, value.setup, 'TEST') def test_cache_url_value(self): value = CacheURLValue() @@ -289,4 +296,4 @@ class ValueTests(TestCase): self.assertEqual(value.setup('TEST'), backends) backends = ['non.existing.Backend'] - self.assertRaises(ImproperlyConfigured, BackendsValue, backends) + self.assertRaises(ValueError, BackendsValue, backends) diff --git a/configurations/values.py b/configurations/values.py index f2d633c..36023c4 100644 --- a/configurations/values.py +++ b/configurations/values.py @@ -2,6 +2,7 @@ import ast import copy import decimal import os +import sys from django.core import validators from django.core.exceptions import ValidationError, ImproperlyConfigured @@ -79,8 +80,8 @@ class BooleanValue(Value): def __init__(self, *args, **kwargs): super(BooleanValue, self).__init__(*args, **kwargs) if self.default not in (True, False): - raise ImproperlyConfigured('Default value {0!r} is not a ' - 'boolean value'.format(self.default)) + raise ValueError('Default value {0!r} is not a ' + 'boolean value'.format(self.default)) def to_python(self, value): normalized_value = value.strip().lower() @@ -89,8 +90,8 @@ class BooleanValue(Value): elif normalized_value in self.false_values: return False else: - raise ImproperlyConfigured('Cannot interpret ' - 'boolean value {0!r}'.format(value)) + raise ValueError('Cannot interpret ' + 'boolean value {0!r}'.format(value)) class CastingMixin(object): @@ -106,13 +107,13 @@ class CastingMixin(object): else: error = 'Cannot use caster of {0} ({1!r})'.format(self, self.caster) - raise ImproperlyConfigured(error) + raise ValueError(error) def to_python(self, value): try: return self._caster(value) except self.exception: - raise ImproperlyConfigured(self.message.format(value)) + raise ValueError(self.message.format(value)) class IntegerValue(CastingMixin, Value): @@ -157,15 +158,17 @@ class ListValue(Value): try: converted_values.append(self.converter(list_value)) except (TypeError, ValueError): - raise ImproperlyConfigured(self.message.format(list_value, - value)) + raise ValueError(self.message.format(list_value, value)) return converted_values class BackendsValue(ListValue): def converter(self, value): - import_by_path(value) + try: + import_by_path(value) + except ImproperlyConfigured, err: + six.reraise(ValueError, ValueError(err), sys.exc_info()[2]) return value @@ -214,9 +217,9 @@ class DictValue(Value): try: evaled_value = ast.literal_eval(value) except ValueError: - raise ImproperlyConfigured(self.message.format(value)) + raise ValueError(self.message.format(value)) if not isinstance(evaled_value, dict): - raise ImproperlyConfigured(self.message.format(value)) + raise ValueError(self.message.format(value)) return evaled_value @@ -229,16 +232,15 @@ class ValidationMixin(object): elif callable(self.validator): self._validator = self.validator else: - error = 'Cannot use validator of {0} ({1!r})'.format(self, - self.validator) - raise ImproperlyConfigured(error) + raise ValueError('Cannot use validator of ' + '{0} ({1!r})'.format(self, self.validator)) self.to_python(self.default) def to_python(self, value): try: self._validator(value) except ValidationError: - raise ImproperlyConfigured(self.message.format(value)) + raise ValueError(self.message.format(value)) else: return value @@ -276,8 +278,7 @@ class PathValue(Value): value = super(PathValue, self).setup(name) value = os.path.expanduser(value) if self.check_exists and not os.path.exists(value): - raise ImproperlyConfigured('Path {0!r} does ' - 'not exist.'.format(value)) + raise ValueError('Path {0!r} does not exist.'.format(value)) return os.path.abspath(value) @@ -287,14 +288,13 @@ class SecretValue(Value): kwargs['environ'] = True super(SecretValue, self).__init__(*args, **kwargs) if self.default is not None: - raise ImproperlyConfigured('Secret values are only allowed to ' - 'be set as environment variables') + raise ValueError('Secret values are only allowed to ' + 'be set as environment variables') def setup(self, name): value = super(SecretValue, self).setup(name) if not value: - raise ImproperlyConfigured('Secret value {0!r} ' - 'is not set'.format(name)) + raise ValueError('Secret value {0!r} is not set'.format(name)) return value diff --git a/docs/values.rst b/docs/values.rst index 146e809..913ea55 100644 --- a/docs/values.rst +++ b/docs/values.rst @@ -204,12 +204,10 @@ Type values Use a custom converter to check for the given variables:: - from django.core.exceptions import ImproperlyConfigured - def check_monty_python(person): if not is_completely_different(person): error = '{0} is not a Monty Python member'.format(person) - raise ImproperlyConfigured(error) + raise ValueError(error) return person MONTY_PYTHONS = ListValue(['John Cleese', 'Eric Idle'], @@ -427,7 +425,7 @@ Other values to reduce the risk of accidentally storing secret values in the settings file. - :raises: ``ImproperlyConfigured`` when given a default value + :raises: ``ValueError`` when given a default value ::