Code

Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#20636 closed Bug (fixed)

AttributeError: 'Settings' object has no attribute '_original_allowed_hosts'

Reported by: aaugustin Owned by: nobody
Component: Testing framework Version: master
Severity: Release blocker Keywords:
Cc: charettes Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When running the tests for http://github.com/aaugustin/django-sesame against stable/1.5.x, I'm hitting this exception after they complete:

Traceback (most recent call last):
  File "/Users/myk/Documents/dev/django/django/core/management/base.py", line 222, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/myk/Documents/dev/django/django/core/management/commands/test.py", line 72, in execute
    super(Command, self).execute(*args, **options)
  File "/Users/myk/Documents/dev/django/django/core/management/base.py", line 255, in execute
    output = self.handle(*args, **options)
  File "/Users/myk/Documents/dev/django/django/core/management/commands/test.py", line 89, in handle
    failures = test_runner.run_tests(test_labels)
  File "/Users/myk/Documents/dev/django/django/test/simple.py", line 370, in run_tests
    self.teardown_test_environment()
  File "/Users/myk/Documents/dev/django/django/test/simple.py", line 341, in teardown_test_environment
    teardown_test_environment()
  File "/Users/myk/Documents/dev/django/django/test/utils.py", line 104, in teardown_test_environment
    settings.ALLOWED_HOSTS = settings._original_allowed_hosts
  File "/Users/myk/Documents/dev/django/django/conf/__init__.py", line 54, in __getattr__
    return getattr(self._wrapped, name)
AttributeError: 'Settings' object has no attribute '_original_allowed_hosts'

I'm positive the tests used to work, and this application is only a few dozen lines of code and doesn't do anything really weird with settings.

It uses @override_settings, but I've tried to reproduce a similar pattern in Django's test suite and I didn't hit the bug:

from django.test.utils import override_settings

@override_settings(MIDDLEWARE_CLASSES=('a',))
class Decorated(TestCase):
    def test_basic_math(self):
        self.assertEqual(2 + 2, 4)

@override_settings(MIDDLEWARE_CLASSES=('b',))
class DoubleDecorated(TestCase):
    pass

I'm not sure why this happens, but since something that used to work now fails with a cryptic error, we should probably do something about it.

Attachments (0)

Change History (9)

comment:1 Changed 13 months ago by charettes

  • Cc charettes added

I hit a similar issue last week. Some people on IRC suggested that it might be due to an unbalanced override_setting but it wasn't the case.

Somehow I ended up with two django.conf.Settings instance, one that was assigned the _original_allowed_hosts attribute while the test suite teardown mechanism attempted to delete it from the second one thus raising the same exception.

I suggest you place pdb a breakpoint in django.conf.Settings.__init__ and make sure only one instance is created. Hope that helps.

comment:2 Changed 13 months ago by aaugustin

I did some debug and could confirm _original_allowed_hosts was set, and later not found, on the same object (same id). Something must have deleted it in the mean time.

comment:3 Changed 13 months ago by claudep

Here is what seems to happen

a) get_commands() accesses settings.INSTALLED_APPS, which triggers Settings.__init__ (instance A)
b) inside Settings.__init__, import of settings module (sesame.tests.settings)
c) as settings is inside tests module, this is triggering a new import chain (sesame.tests, django, etc.) ... until django.db accesses settings
d) LazySettings._wrapped is still empty (remember we are still in Settings.__init__), hence creation of a new Settings instance (B).
e) still inside the import process, override_settings is setting its wrapped attribute to Settings B
f) initial Settings.__init__ finishes, so settings._wrapped is instance A
g) setup_test_environment set _original_allowed_hosts on Settings A
h) after tests finishes, override_settings restore settings._wrapped to Settings B (value of override_settings.wrapped, see e)
i) teardown_test_environment is complaining that Settings B has no _original_allowed_hosts.

We could point at several problems in this process:

  • override_settings.wrapped could be set in enable() instead of in __init__ (to be tested, but may be #20290)
  • Django should not access settings at module import time (I already addressed this for django.db in 1.6)
  • Do not put your settings modules inside another module which imports much of Django

I let you choose which to address first...

comment:4 Changed 13 months ago by charettes

@claudep I hit the exact same issue you described but I don't think it's the case with @aaugustin since the _original_allowed_hosts is set and deleted on the same instance of Settings.

comment:5 Changed 13 months ago by aaugustin

I double-checked in case I had missed something, and I confirm that steps g) and i) operate on the same settings object. However, its _wrapped attribute has changed!

I think I'm seeing a variant of the scenario described by Claude, and the root cause lies in his first bullet point: override_settings.wrapped is set in __init__ rather than in enable, which is indeed #20290.

I see three options:

  • backport #20290,
  • store _original_allowed_hosts somewhere else — like original_email_backend which is stored on the django.core.mail module.
  • catch and drop the AttributeError — after all we're in test teardown, we don't really care.

comment:6 Changed 13 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 02619227a9271047c7d4b62a6aec744ca17f7270:

[1.5.x] Fixed #20636 -- Stopped stuffing values in the settings.

In Django < 1.6, override_settings restores the settings module that was
active when the override_settings call was executed, not when it was
run. This can make a difference when override_settings is applied to a
class, since it's executed when the module is imported, not when the
test case is run.

In addition, if the settings module for tests is stored alongside the
tests themselves, importing the settings module can trigger an import
of the tests. Since the settings module isn't fully imported yet,
class-level override_settings statements may store a reference to an
incorrect settings module. Eventually this will result in a crash during
test teardown because the settings module restored by override_settings
won't the one that was active during test setup.

While Django should prevent this situation in the future by failing
loudly in such dubious import sequences, that change won't be backported
to 1.5 and 1.4. However, these versions received the "allowed hosts"
patch and they're prone to "AttributeError: 'Settings' object has no
attribute '_original_allowed_hosts'". To mitigate this regression, this
commits stuffs _original_allowed_hosts on a random module instead of the
settings module.

This problem shouldn't occur in Django 1.6, see #20290, but this patch
will be forward-ported for extra safety.

Also tweaked backup variable names for consistency.

comment:7 Changed 13 months ago by Aymeric Augustin <aymeric.augustin@…>

In e3b6fed320f6e67b7e5dc28bab610f342a81f7d7:

[1.4.x] Fixed #20636 -- Stopped stuffing values in the settings.

In Django < 1.6, override_settings restores the settings module that was
active when the override_settings call was executed, not when it was
run. This can make a difference when override_settings is applied to a
class, since it's executed when the module is imported, not when the
test case is run.

In addition, if the settings module for tests is stored alongside the
tests themselves, importing the settings module can trigger an import
of the tests. Since the settings module isn't fully imported yet,
class-level override_settings statements may store a reference to an
incorrect settings module. Eventually this will result in a crash during
test teardown because the settings module restored by override_settings
won't the one that was active during test setup.

While Django should prevent this situation in the future by failing
loudly in such dubious import sequences, that change won't be backported
to 1.5 and 1.4. However, these versions received the "allowed hosts"
patch and they're prone to "AttributeError: 'Settings' object has no
attribute '_original_allowed_hosts'". To mitigate this regression, this
commits stuffs _original_allowed_hosts on a random module instead of the
settings module.

This problem shouldn't occur in Django 1.6, see #20290, but this patch
will be forward-ported for extra safety.

Also tweaked backup variable names for consistency.

Backport of 0261922 from stable/1.5.x.

Conflicts:

django/test/utils.py

comment:8 Changed 13 months ago by Aymeric Augustin <aymeric.augustin@…>

In 5a6f12182ef14883df6534dc3465059b78f54a1c:

Fixed #20636 -- Stopped stuffing values in the settings.

In Django < 1.6, override_settings restores the settings module that was
active when the override_settings call was executed, not when it was
run. This can make a difference when override_settings is applied to a
class, since it's executed when the module is imported, not when the
test case is run.

In addition, if the settings module for tests is stored alongside the
tests themselves, importing the settings module can trigger an import
of the tests. Since the settings module isn't fully imported yet,
class-level override_settings statements may store a reference to an
incorrect settings module. Eventually this will result in a crash during
test teardown because the settings module restored by override_settings
won't the one that was active during test setup.

While Django should prevent this situation in the future by failing
loudly in such dubious import sequences, that change won't be backported
to 1.5 and 1.4. However, these versions received the "allowed hosts"
patch and they're prone to "AttributeError: 'Settings' object has no
attribute '_original_allowed_hosts'". To mitigate this regression, this
commits stuffs _original_allowed_hosts on a random module instead of the
settings module.

This problem shouldn't occur in Django 1.6, see #20290, but this patch
will be forward-ported for extra safety.

Also tweaked backup variable names for consistency.

Forward port of 0261922 from stable/1.5.x.

Conflicts:

django/test/utils.py

comment:9 Changed 13 months ago by Aymeric Augustin <aymeric.augustin@…>

In e2b86571bfa3503fe43adfa92e9c9f4271a7a135:

[1.4.x] Fixed oversight in e3b6fed3. Refs #20636.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.