Skip to content

Commit

Permalink
[PATCH] Fix #9: Catch circular Django settings import Conformity error
Browse files Browse the repository at this point in the history
  • Loading branch information
nickwilliams-eventbrite committed Mar 5, 2020
1 parent 06584da commit 936e7b6
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 2 deletions.
8 changes: 7 additions & 1 deletion pymetrics/recorders/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
)

from conformity import fields
from conformity.error import ValidationError
import six

from pymetrics.configuration import (
Expand Down Expand Up @@ -246,11 +247,16 @@ def get_django_settings(cls):
try:
from django.conf import settings
if settings:
# Django won't actually raise ImproperlyConfigured unless you try to _use_ the settings
# Django won't actually raise ImproperlyConfigured unless you try to _use_ the settings.
getattr(settings, 'DEBUG', False)
cls.django_settings = settings
except (ImportError, cls.DjangoImproperlyConfigured):
# Could be a circular import or problem with settings that might be resolved at a later time.
pass
except ValidationError as e:
# Likely a circular import that will be resolved at a later time.
if not (e.args[0] and 'ImportError: ' in e.args[0]):
raise

return cls.django_settings

Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,5 @@ ignore_missing_imports = True
test=pytest

[tool:pytest]
junit_family = xunit2
addopts = -s --junitxml=pytests.xml --cov=pymetrics --cov-branch --cov-fail-under=85 --cov-report=term-missing
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def readme():
include_package_data=True,
install_requires=[
'attrs>=17.4,<20',
'conformity~=1.26,>=1.26.1',
'conformity>=1.26.9,!=1.27.0,<2.0',
'enum34;python_version<"3.4"',
'six',
'typing~=3.7.4;python_version<"3.5"',
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/recorders/test_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
)

from conformity import fields
from conformity.error import ValidationError
import freezegun
import mock
import pytest
import six

from pymetrics.instruments import (
Expand Down Expand Up @@ -80,6 +82,52 @@ def test_config_explicit(self):
# re-config should do nothing
recorder.configure({'bad_config': 'bad_value'})

def test_config_django_causes_conformity_import_error(self):
django_exceptions = mock.MagicMock()
django_exceptions.ImproperlyConfigured = FakeImproperlyConfigured

django_conf = mock.MagicMock()
e = ValidationError(
"Invalid keyword arguments:\n - middleware.0.path: ImportError: cannot import name 'baz' from 'foo.bar' "
)
if six.PY2:
django_conf.settings.__nonzero__.side_effect = e
else:
django_conf.settings.__bool__.side_effect = e

with mock.patch.dict('sys.modules', {
'django': mock.MagicMock(),
'django.conf': django_conf,
'django.core': mock.MagicMock(),
'django.core.exceptions': django_exceptions,
}):
recorder = DefaultMetricsRecorder('me')

assert recorder.is_configured is False

def test_config_django_causes_conformity_other_error(self):
django_exceptions = mock.MagicMock()
django_exceptions.ImproperlyConfigured = FakeImproperlyConfigured

django_conf = mock.MagicMock()
e = ValidationError(
"Invalid keyword arguments:\n - middleware.0.path: Some other error that isn't an import error "
)
if six.PY2:
django_conf.settings.__nonzero__.side_effect = e
else:
django_conf.settings.__bool__.side_effect = e

with mock.patch.dict('sys.modules', {
'django': mock.MagicMock(),
'django.conf': django_conf,
'django.core': mock.MagicMock(),
'django.core.exceptions': django_exceptions,
}), pytest.raises(ValidationError) as error_context:
DefaultMetricsRecorder('me')

assert error_context.value is e

def test_config_django_available_but_settings_broken1(self):
django_exceptions = mock.MagicMock()
django_exceptions.ImproperlyConfigured = FakeImproperlyConfigured
Expand Down

0 comments on commit 936e7b6

Please sign in to comment.