Opened 13 years ago

Closed 13 years ago

Last modified 10 years ago

#16353 closed Bug (fixed)

With multi-databases, if django.contrib.sites.models.Site is not synchronized on all databases, Django's tests fail

Reported by: Aymeric Augustin Owned by: nobody
Component: Testing framework Version: 1.3
Severity: Release blocker Keywords:
Cc: ubernostrum@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Here are the detailed steps to reproduce (in a fresh checkout):

$ PYTHONPATH=. django/bin/django-admin.py startproject testproj
$ cd testproj
$ PYTHONPATH=.. ./manage.py startapp testapp
$ vi settings.py
DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.sqlite3',
        'NAME': 'default.sqlite3',
    },
    'other': {
        'ENGINE': 'django.db.backends.sqlite3',
        'NAME': 'other.sqlite3',
    }
}

DATABASE_ROUTERS = ['testapp.routers.MyAppRouter']
$ vi testapp/routers.py
class MyAppRouter(object):

    def db_for_read(self, model, **hints):
        return 'other' if model._meta.app_label == 'testapp' else 'default'

    def db_for_write(self, model, **hints):
        return 'other' if model._meta.app_label == 'testapp' else 'default'

    def allow_relation(self, obj1, obj2, **hints):
        pass

    def allow_syncdb(self, db, model):
        return db == ('other' if model._meta.app_label == 'testapp' else 'default')
$ PYTHONPATH=.. ./manage.py test
Creating test database for alias 'default'...
Creating test database for alias 'other'...
Traceback (most recent call last):
  File "./manage.py", line 14, in <module>
    execute_manager(settings)
  File "/Users/myk/Desktop/django-trunk/django/core/management/__init__.py", line 442, in execute_manager
    utility.execute()
  File "/Users/myk/Desktop/django-trunk/django/core/management/__init__.py", line 379, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/myk/Desktop/django-trunk/django/core/management/commands/test.py", line 34, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/Users/myk/Desktop/django-trunk/django/core/management/base.py", line 191, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/myk/Desktop/django-trunk/django/core/management/base.py", line 220, in execute
    output = self.handle(*args, **options)
  File "/Users/myk/Desktop/django-trunk/django/core/management/commands/test.py", line 54, in handle
    failures = test_runner.run_tests(test_labels)
  File "/Users/myk/Desktop/django-trunk/django/test/simple.py", line 354, in run_tests
    old_config = self.setup_databases()
  File "/Users/myk/Desktop/django-trunk/django/test/simple.py", line 291, in setup_databases
    test_db_name = connection.creation.create_test_db(self.verbosity, autoclobber=not self.interactive)
  File "/Users/myk/Desktop/django-trunk/django/db/backends/creation.py", line 256, in create_test_db
    if Site is not None and Site.objects.using(self.connection.alias).count() == 1:
  File "/Users/myk/Desktop/django-trunk/django/db/models/query.py", line 334, in count
    return self.query.get_count(using=self.db)
  File "/Users/myk/Desktop/django-trunk/django/db/models/sql/query.py", line 406, in get_count
    number = obj.get_aggregation(using=using)[None]
  File "/Users/myk/Desktop/django-trunk/django/db/models/sql/query.py", line 372, in get_aggregation
    result = query.get_compiler(using).execute_sql(SINGLE)
  File "/Users/myk/Desktop/django-trunk/django/db/models/sql/compiler.py", line 754, in execute_sql
    cursor.execute(sql, params)
  File "/Users/myk/Desktop/django-trunk/django/db/backends/sqlite3/base.py", line 226, in execute
    return Database.Cursor.execute(self, query, params)
django.db.utils.DatabaseError: no such table: django_site

Attached patch fixes this problem.

Attachments (3)

sites-and-multi-db.diff (1.3 KB ) - added by Aymeric Augustin 13 years ago.
16353.diff (3.2 KB ) - added by Aymeric Augustin 13 years ago.
16353.2.diff (3.2 KB ) - added by Aymeric Augustin 13 years ago.

Download all attachments as: .zip

Change History (15)

by Aymeric Augustin, 13 years ago

Attachment: sites-and-multi-db.diff added

comment:1 by Michael Manfre, 13 years ago

Owner: changed from nobody to Michael Manfre
Status: newassigned
Triage Stage: UnreviewedAccepted

comment:2 by Michael Manfre, 13 years ago

Owner: changed from Michael Manfre to nobody
Status: assignednew

comment:3 by Ramiro Morales, 13 years ago

Needs tests: set

comment:4 by Aymeric Augustin, 13 years ago

#16515 was a duplicate.

comment:5 by James Bennett, 13 years ago

Cc: ubernostrum@… added

comment:6 by Aymeric Augustin, 13 years ago

#16828 was a duplicate.

Version 0, edited 13 years ago by Aymeric Augustin (next)

comment:7 by Aymeric Augustin, 13 years ago

Severity: NormalRelease blocker

comment:8 by Aymeric Augustin, 13 years ago

Needs tests: unset

This regression was introduced in r16027 (trunk) and r16028 (1.3.X). It was duplicated in GIS in r16749 (trunk) and r16751 (1.3.X).

The goal was to fix #15573, a problem linked to the re-implementation of auto-incremented sequences (which was itself a regression, but at this point the history isn't relevant any longer). However, the "fix" doesn't look optimal: it introduces in the core some code that's specific to a contrib app (sites) and also this regression. Furthermore, it doesn't include any tests.

Attached patch reverts r16027 and r16749, and proposes another fix for #15573, a much more simple and predictable one.

Note that the post_syncdb signal is fired right after the tables are created or flushed, so there's no risk to overwrite an existing Site.


I tested the patch manually with the procedure described in #16828.

I'm still stuck writing automated tests. In fact, it's extremely difficult to test the initialization of the django_site table in the test databases, because of a chicken'n'egg problem. Since the table is created before the tests start running, we can't pull a trick like in tests/modeltests/proxy_model_inheritance/tests.py. I don't think hacking the internals of the app cache to deregister the app, drop the table and resync it is a good idea.

Suggestions welcome. Otherwise, I hope we can revert a commit that didn't include tests and demonstrably broke stuff even if it's impossible to write automated tests for either behavior.

Finally, note that the new changes are already covered by all the tests that fail when SITE_ID != 1 — see #15573.


This patch must be backported to 1.3.X.

by Aymeric Augustin, 13 years ago

Attachment: 16353.diff added

by Aymeric Augustin, 13 years ago

Attachment: 16353.2.diff added

comment:9 by Aymeric Augustin, 13 years ago

The full test suite passes under SQLite (just a sanity check).

Under Oracle, with the patch, I get this result: http://ci.myks.org/job/Django%20+%20Oracle/5/#showFailuresLink
where the official CI server gets this: http://ci.django-cms.org/job/Django/database=oracle,python=python2.7/271/#showFailuresLink

Both have 81 failures, so it isn't more broken with my patch than without.

comment:10 by Carl Meyer, 13 years ago

Resolution: fixed
Status: newclosed

In [16868]:

Fixed #16353 -- don't try to create Site objects on all databases. Refs #15573, #15346. Thanks Aymeric Augustin for the report and the patch.

comment:11 by Carl Meyer, 13 years ago

In [16869]:

[1.3.X] Fixed #16353 -- don't try to create Site objects on all databases. Refs #15573, #15346. Thanks Aymeric Augustin for the report and the patch.

Backport of r16868 in trunk.

comment:12 by Tim Graham <timograham@…>, 10 years ago

In 1f98ec2e53e4636863396ab54f671f4546f9ba4c:

Fixed #23929 -- Added more tests for create_default_site.

Refs: #15346, #15573, #16353, #16828.

Note: See TracTickets for help on using tickets.
Back to Top