Opened 5 years ago

Closed 5 years ago

Last modified 2 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 5 years ago.
16353.diff (3.2 KB) - added by Aymeric Augustin 5 years ago.
16353.2.diff (3.2 KB) - added by Aymeric Augustin 5 years ago.

Download all attachments as: .zip

Change History (15)

Changed 5 years ago by Aymeric Augustin

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

comment:1 Changed 5 years ago by Michael Manfre

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

comment:2 Changed 5 years ago by Michael Manfre

Owner: changed from Michael Manfre to nobody
Status: assignednew

comment:3 Changed 5 years ago by Ramiro Morales

Needs tests: set

comment:4 Changed 5 years ago by Aymeric Augustin

#16515 was a duplicate.

comment:5 Changed 5 years ago by James Bennett

Cc: ubernostrum@… added

comment:6 Changed 5 years ago by Aymeric Augustin

#16828 was a duplicate. It has the same patch, and more information demonstrating that it's a regression from 1.3 to 1.3.1 => marking as release blocker.

Last edited 5 years ago by Aymeric Augustin (previous) (diff)

comment:7 Changed 5 years ago by Aymeric Augustin

Severity: NormalRelease blocker

comment:8 Changed 5 years ago by Aymeric Augustin

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.

Changed 5 years ago by Aymeric Augustin

Attachment: 16353.diff added

Changed 5 years ago by Aymeric Augustin

Attachment: 16353.2.diff added

comment:9 Changed 5 years ago by Aymeric Augustin

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 Changed 5 years ago by Carl Meyer

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 Changed 5 years ago by Carl Meyer

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 Changed 2 years ago by Tim Graham <timograham@…>

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