Opened 13 years ago
Closed 10 years ago
#16368 closed Bug (fixed)
Custom sites model overridden by contrib.sites.model.Site
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | contrib.sites | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
We have a custom 'sites framework' implementation that basically consists of a Site model and SiteManager. For some reason, when running our application, the Site model from django.contrib.sites is used instead of our own.
I did the following from within the app:
from myapp.sites.models import Site print Site.objects <class 'django.contrib.sites.models.Site'>
When doing the same from the management shell (./manage.py shell), I get the expected output:
>>> from myapp.sites.models import Site >>> type(Site.objects) <class 'foobar.sites.models.Site'>
Our application used to run fine on Django 1.0 and subsequently on 1.1 and 1.2 during my current upgrade attempt to get the application onto version 1.3. I also tried this using the current trunk (revision 16483) and got the same result.
I'm attaching a minimal Django project that replicates this behavior on my machine. You will see that running ./manage syncdb actually creates a 'django_site' table instead of 'sites'.
Attachments (1)
Change History (20)
by , 13 years ago
Attachment: | foobar.tar.gz added |
---|
comment:1 by , 13 years ago
comment:2 by , 13 years ago
Version: | 1.3 → SVN |
---|
I can also reproduce this error (django trunk SVN-16452).
#settings.py INSTALLED_APPS = ('xxx.sites',)
#sites/models.py from django.db import models class Site(models.Model): domain = models.CharField(unique=True, max_length=100, db_index=True)
$ python manage.py syncdb Creating tables ... Creating table django_site Installing custom SQL ... Installing indexes ... No fixtures found. $ mysql -uuser -p test -e"show tables;" +----------------+ | Tables_in_test | +----------------+ | django_site | +----------------+
comment:3 by , 13 years ago
This is most likely because django.contrib.sites gets imported at some point, and it clashes with your app — app names must be unique.
comment:4 by , 13 years ago
comment:5 by , 13 years ago
I removed the code from r14563 in django/core/management/validation.py and django/utils/itercompat.py as you suggest and now it works.
+----------------+ | Tables_in_test | +----------------+ | sites_site | +----------------+
follow-up: 19 comment:6 by , 13 years ago
Applying the patches from r14563 on a working copy of 1.3 does not fix the issue for me. I tried on trunk as well and that seems to leave the code in a broken state.
For what it is worth when I rename my 'sites' app to something else the problem goes away. If I'm not allowed to have a Django app called 'sites' (even when I'm not using django.contrib.sites) then surely Django needs to enforce that somehow instead of allowing me to do so and then trampling all over my imports.
comment:7 by , 13 years ago
comment:8 by , 13 years ago
I just did a patch -uR
which as I understand it reverts the changes from r14563.
comment:10 by , 13 years ago
Thanks for the update. I just tried with r16500 and still no luck. Let me know if there is anything else I can do to help track down the problem.
comment:11 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I still believe the root cause of the problem is that django.contrib.sites
gets imported at some point. Can you figure out why? For instance, patch django/contrib/sites/models.py to raise an exception, and paste the traceback here.
comment:12 by , 13 years ago
Raising an exception from django.contrib.sites.models
gave me this traceback:
..snip... from django.contrib.contenttypes import generic File "/home/briehan/.virtualenvs/django-trunk/src/django/django/contrib/contenttypes/generic.py", line 14, in <module> from django.contrib.admin.options import InlineModelAdmin, flatten_fieldsets File "/home/briehan/.virtualenvs/django-trunk/src/django/django/contrib/admin/__init__.py", line 6, in <module> from django.contrib.admin.sites import AdminSite, site File "/home/briehan/.virtualenvs/django-trunk/src/django/django/contrib/admin/sites.py", line 4, in <module> from django.contrib.admin.forms import AdminAuthenticationForm File "/home/briehan/.virtualenvs/django-trunk/src/django/django/contrib/admin/forms.py", line 4, in <module> from django.contrib.auth.forms import AuthenticationForm File "/home/briehan/.virtualenvs/django-trunk/src/django/django/contrib/auth/forms.py", line 10, in <module> from django.contrib.sites.models import get_current_site File "/home/briehan/.virtualenvs/django-trunk/src/django/django/contrib/sites/models.py", line 5, in <module> raise Exception
So it looks like the problem is caused by importing generic
from django.contrib.contenttypes
. When I remove those imports from my project everything works.
comment:13 by , 13 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Can't reproduce the reported issue using the project contained in foobar.tar.gz
and the following revisions:
trunk as of now (post r16493):
django/conf/__init__.py:75: DeprecationWarning: The ADMIN_MEDIA_PREFIX setting has been removed; use STATIC_URL instead. "use STATIC_URL instead.", DeprecationWarning) django/conf/__init__.py:75: DeprecationWarning: The ADMIN_MEDIA_PREFIX setting has been removed; use STATIC_URL instead. "use STATIC_URL instead.", DeprecationWarning) Validating models... 0 errors found Django version 1.4 pre-alpha, using settings 'foobar.settings' Development server is running at http://127.0.0.1:8000/ Quit the server with CONTROL-C. <class 'foobar.sites.models.Site'> [13/Jul/2011 22:03:45] "GET / HTTP/1.1" 200 0
trunk as of r16500 (mentioned in comment:10):
django/conf/__init__.py:75: DeprecationWarning: The ADMIN_MEDIA_PREFIX setting has been removed; use STATIC_URL instead. "use STATIC_URL instead.", DeprecationWarning) django/conf/__init__.py:75: DeprecationWarning: The ADMIN_MEDIA_PREFIX setting has been removed; use STATIC_URL instead. "use STATIC_URL instead.", DeprecationWarning) Validating models... 0 errors found Django version 1.4 pre-alpha, using settings 'foobar.settings' Development server is running at http://127.0.0.1:8000/ Quit the server with CONTROL-C. <class 'foobar.sites.models.Site'> [13/Jul/2011 21:53:01] "GET / HTTP/1.1" 200 0
1.3.X branch as of now (post r16541):
Validating models... 0 errors found Django version 1.3, using settings 'foobar.settings' Development server is running at http://127.0.0.1:8000/ Quit the server with CONTROL-C. <class 'foobar.sites.models.Site'> [13/Jul/2011 21:54:02] "GET / HTTP/1.1" 200 0
And, as expected, I can reproduce it with 1.3:
Validating models... 0 errors found Django version 1.3, using settings 'foobar.settings' Development server is running at http://127.0.0.1:8000/ Quit the server with CONTROL-C. <class 'django.contrib.sites.models.Site'> [13/Jul/2011 21:51:06] "GET / HTTP/1.1" 200 0
So I'm going to close this as duplicate of #16283.
comment:14 by , 13 years ago
Thanks ramiro, but when I add from django.contrib.contenttypes import generic
to my sites model I get the following:
./manage.py runserver django/conf/__init__.py:75: DeprecationWarning: The ADMIN_MEDIA_PREFIX setting has been removed; use STATIC_URL instead. "use STATIC_URL instead.", DeprecationWarning) django/conf/__init__.py:75: DeprecationWarning: The ADMIN_MEDIA_PREFIX setting has been removed; use STATIC_URL instead. "use STATIC_URL instead.", DeprecationWarning) Validating models... 0 errors found Django version 1.4 pre-alpha SVN-16541, using settings 'foobar.settings' Development server is running at http://127.0.0.1:8000/ Quit the server with CONTROL-C. <class 'django.contrib.sites.models.Site'> [14/Jul/2011 03:38:39] "GET / HTTP/1.1" 200 0
Should I open a new ticket for this issue?
comment:15 by , 13 years ago
Resolution: | duplicate |
---|---|
Status: | closed → reopened |
I'm re-opening this as I feel that I can better explain what is happening here. It is the combination of a few different, but important aspects and there may be a few different fixes that are in order.
First off - from the sites documentation - https://docs.djangoproject.com/en/dev/ref/contrib/sites/#requestsite-objects There are parts of django that use the sites framework even if it's not installed. So ultimately, there valid use cases where the sites app is NOT installed, but it IS imported. This is an important distinction. In these cases, I feel that it is reasonable that one should be able to make a new sites app with a model called Site.
The above is not a bug, in and of itself. However, when the sites app is imported, its models.py is also imported. When the models.py is imported the ModelBase metaclass runs and prepares this model. It also caches this model for future use so that subsequent imports don't go through the costly process of preparing the model a second time. However, it caches the model based on the app_label and model class name, which in this case is 'sites.Site'. When the new, custom app is imported, the same process occurs. However - this time around it notices that it already prepared a 'sites.Site' - so rather than making a new one, it returns a reference to the old one. However, in this particular case it will always the wrong reference. It will always be a reference to the non-used django.contrib.sites.models.Site model instead of the custom one. For reference, this is the code that does the caching: https://code.djangoproject.com/browser/django/trunk/django/db/models/base.py#L91
There are many potential solutions to this - but I'm not sure which is the appropriate one. Ultimately I feel that django's use of app_labels as unique across all installed apps is the actual errant decision - but this ticket isn't about fixing that. This ticket is about fixing it so that one can use a custom sites.models.Site model when the django.contrib.sites.models.Site is not being used.
I think the appropriate usage would be to update it so that the Meta class for django's models only caches models that it has prepared when they are part of an installed application. If they are not part of an installed application then they should be ignored.
You can test this using the same attached example, but this time include django.contrib.admin in the INSTALLED_APPS (which imports sites). Or you can import the contenttypes app which also uses it.
Does that help explain the issue?
comment:16 by , 13 years ago
Some findings and notes:
Bisection betwen 1.2 and 1.3 (the two releases the OP reports things changed from working to not working e.g. the custom Sites model gets replaced by the contrib.sites
app one even when that app isn't listed in INSTALLED_APPS) shows the commit where that change in behavior appeared is r14769.
(Conditions are: Using the simple foobar
project attached to this ticket and adding from django.contrib.contenttypes import generic
to its sites/models.py
file).
I don't think the changes from that commit in particular are wrong regarding this issue. Looking at the traceback in comment:12 IMHO what's the culprit of this unexpected behavior is the fact that django.contrib.contenttypes.generic
contains both the definitions of the model-related generic stuff (GenericForeignKey, etc.) AND the admin app-related specialized inlines (GenericInlineModelAdmin, GenericStackedInline, GenericTabularInline.)
So, for example, if you import django.contrib.contenttypes.generic
from your models.py
because you need GenericForeignKey then the Generic*Inline* stuff imports django.contrib.admin
and it in its own turn imports django.contrib.sites
causing the observed failure. Again, the Sites framework isn't listed in INSTALLED_APPS
and what is worse: The admin app isn't either (!).
Maybe it's time we move GenericInlineModelAdmin, GenericStackedInline, GenericTabularInline from django.contrib.contenttypes.generic
to, say, django.contrib.contenttypes.generic_admin
? (of course this would need a deprecation process)
comment:17 by , 12 years ago
Status: | reopened → new |
---|
comment:19 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I fixed that bug while working on app-loading, most likely in 9ffab9ce.
Apologies for the inconsistent naming in my code examples. They should be:
and the expected output