Opened 16 years ago
Closed 11 years ago
#8713 closed New feature (fixed)
Django core depends on django.contrib
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | jwilk@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I believe that it should be possible to strip django/contrib
directory and still have a functional Django. However, currently there are some imports from django.contrib
present in non-contrib part of Django. It's contrary to the loose-coupling idea and description of contrib package (a variety of extra, optional tools).
$ egrep -ohr --include='*.py' --exclude-dir=contrib --exclude-dir=conf 'django[.]contrib[.][[:alnum:]]+' django-trunk/django/ | sort | uniq -c | sort -n 2 django.contrib.localflavor 2 django.contrib.sites 3 django.contrib.sessions 4 django.contrib.contenttypes 5 django.contrib.auth $ egrep -lr --include='*.py' --exclude-dir=contrib --exclude-dir=conf 'django.contrib.[[:alnum:]]+' django-trunk/django/ django-trunk/django/test/client.py django-trunk/django/db/models/sql/subqueries.py django-trunk/django/db/models/fields/__init__.py django-trunk/django/db/models/base.py django-trunk/django/core/management/commands/cleanup.py django-trunk/django/core/management/sql.py django-trunk/django/core/context_processors.py django-trunk/django/views/generic/create_update.py django-trunk/django/views/defaults.py django-trunk/django/middleware/cache.py
Attachments (3)
Change History (21)
comment:1 by , 16 years ago
follow-up: 3 comment:2 by , 16 years ago
I suspect many of these are not what you think they are, or involve optional features of core functionality (e.g., generic views use auth for optional "login required" functionality).
comment:3 by , 16 years ago
Replying to ubernostrum:
I suspect many of these are not what you think they are, or involve optional features of core functionality (e.g., generic views use auth for optional "login required" functionality).
Why generic views should even know anything about auth-related stuff? After all, we have login_required
decorator in django.contrib.auth.decorators
which should handle it. Now this same job is done by two distinct pieces of code - looks like DRY violation to me.
comment:4 by , 16 years ago
Backwards compatibile issues
tests/client.py
Presence of django.contrib.session
in INSTALLED_APPS
is mostly checked before import. It can be done with django.contrib.auth
as well, which is actually used only in Client.login()
method. Or at least relevant import could be just moved to this method (ugly).
db/models/sql/subqueries.py
django.contrib.contenttypes
is used in DeleteQuery.delete_batch_related()
method. At least a check in INSTALLED_APPS
could be done.
core/management/commands/cleanup.py
Move this command to django/contrib/session/management/commands/
.
core/management/sql.py
Import is present but not used. Remove it.
views/defaults.py
Additional check for django.contrib.contenttypes
and django.contrib.sites
in INSTALLED_APPS
.
Backwards incompatible issues
django/db/models/fields/__init__.py
views/generic/create_update.py
create_object()
should not have login_required
argument. django.contrib.auth.decorators.login_required
should be used instead.
core/context_processors.py
Move django.core.context_processors.auth
to django.contrib.auth.context_processsors
.
No issues
db/models/base.py
django.contrib
used only in a comment.
middleware/cache.py
django.contrib
used only in an assertion string.
comment:5 by , 16 years ago
Owner: | changed from | to
---|
by , 16 years ago
Attachment: | core-management-sql.diff added |
---|
by , 16 years ago
Attachment: | core-management-commands-cleanup.diff added |
---|
comment:6 by , 16 years ago
Owner: | changed from | to
---|
by , 16 years ago
Attachment: | test-client.diff added |
---|
comment:7 by , 16 years ago
Component: | Uncategorized → Core framework |
---|
comment:8 by , 16 years ago
Triage Stage: | Unreviewed → Accepted |
---|
We've discussed this at DjangoCon (independently of this ticket) and decided it would be a good plan to refactor things so that django.contrib could be safely deleted without affecting the rest of the framework. I have not looked at the patches in this particular ticket, but I'm marking the ticket as accepted because we like the concept.
comment:9 by , 14 years ago
Cc: | added |
---|---|
Easy pickings: | unset |
Severity: | → Normal |
Type: | → Uncategorized |
comment:10 by , 14 years ago
Cc: | removed |
---|
comment:11 by , 14 years ago
Type: | Uncategorized → New feature |
---|
comment:13 by , 12 years ago
Running the command provided by the OP I still found the following uses of contrib in core.
Action needed:
django/middleware/doc.py
: checksrequest.user.is_active/is_staff
django/test/client.py
: depends ondjango.contrib.auth
and has coupling with 'django.contrib.sessions'django/test/testcases.py
: depends ondjango.contrib.staticfiles
(at least)django/views/defaults.py
:shortcut
has a comment saying it should be deprecated in Django 2.0, but I don't see much point in waiting; we have a well established deprecation policy now and this is a trivial case.
No action needed:
django/core/management/commands/cleanup.py
: it's deprecateddjango/db/models/base.py
: it's just a commentdjango/middleware/cache.py
: testsrequest.user.is_authenticated()
whenCACHE_MIDDLEWARE_ANONYMOUS_ONLY
is set. #15201 proposes to decoupleCACHE_MIDDLEWARE_ANONYMOUS_ONLY
fromdjango.contrib.auth
by testing only if the response varies on cookie. That covers the issue.
comment:18 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Since all changes identified in comment 13 have been done, except the one that's still tracked in #20915, I believe we can close this ticket.
Dependency on
django.contrib.localflavor
is already covered by #8210 and #8664.