#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)
Change History (15)
by , 13 years ago
Attachment: | sites-and-multi-db.diff added |
---|
comment:1 by , 13 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 13 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
comment:3 by , 13 years ago
Needs tests: | set |
---|
comment:4 by , 13 years ago
comment:5 by , 13 years ago
Cc: | added |
---|
comment:6 by , 13 years ago
#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.
comment:7 by , 13 years ago
Severity: | Normal → Release blocker |
---|
comment:8 by , 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 , 13 years ago
Attachment: | 16353.diff added |
---|
by , 13 years ago
Attachment: | 16353.2.diff added |
---|
comment:9 by , 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.
#16515 was a duplicate.