Opened 13 years ago
Closed 13 years ago
#18018 closed Bug (invalid)
MessageFailure in Django 1.4 admin
Reported by: | Simon | Owned by: | nobody |
---|---|---|---|
Component: | Uncategorized | Version: | 1.4 |
Severity: | Normal | Keywords: | admin, messages, failure |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
When not having the Django messages framework installed, Django 1.4 throws as exception on saving objects via the admin interface:
MessageFailure at /admin/forum/post/5/
You cannot add messages without installing django.contrib.messages.middleware.MessageMiddleware
When I disable loc 696 in contrib/admin/options.py, everything works fine:
def message_user(self, request, message):
"""
Send a message to the user. The default implementation
posts a message using the django.contrib.messages backend.
"""
# messages.info(request, message)
pass
messages.info can only be used when the optional messages framework is installed ...?
Another minor thing:
Even with the new cookie based session backend, Django still creates a database session table. Intentionally?
Great work and thanks a lot for Django 1.4!!
Change History (14)
comment:1 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
follow-up: 4 comment:2 by , 13 years ago
Should these dependencies be validated? Maybe part of improve error messages project?
comment:3 by , 13 years ago
Another minor thing: Even with the new cookie based session backend, Django still creates a database session table. Intentionally?
This is a known trade-off, I don't believe there are any performance drawbacks to creating a single, unused table.
comment:4 by , 13 years ago
Replying to akaariai:
Should these dependencies be validated? Maybe part of improve error messages project?
No, at some point we can't prevent people who don't read the first line of the docs from shooting themselves into the foot.
comment:5 by , 13 years ago
Thanks for the hint. Obviously this changed over the last Django version, since up to now, we neither had the required message context processor, nor middleware or app installed. For only *one* occurrence of the messages framework inside the admin code this is actually quite some overhead. IMHO, it would make sense to make this optional and simply check availability of the framework here:
contrib/admin/options.py
messages.info(request, message)
Best,
Simon
comment:6 by , 13 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
Triage Stage: | Unreviewed → Accepted |
I think this is a backwards incompatible change, in 1.3 you did not need the messages framework, in 1.4 you do, and this is not mentioned in release notes. I am reopening this, please re-close if I am missing something obvious.
comment:7 by , 13 years ago
Indeed - it wasn't obvious in the original report that it's a regression. It could still end up as wontfix, but with more arguments :)
comment:8 by , 13 years ago
It's a planned regression -- the old messages support was deprecated and finally removed in 1.4. Thus the new way is required by admin in 1.4. I've seen other tickets go by where someone complained that final removal of something on the deprecation path wasn't noted in the release notes. This is the same issue. Answer in the past has been we don't note those in the release notes...given how many times things like this are being raised I think it might be better if we DID actually note those things that are now really gone due to finishing their deprecation cycle. People don't seem to be paying attention to the plans from previous release notes or seeing/acting on the deprecation warnings.
comment:9 by , 13 years ago
I still think there is problem. If you installed admin by the docs of 1.3:
Add 'django.contrib.admin' to your INSTALLED_APPS setting. The admin has four dependencies - django.contrib.auth, django.contrib.contenttypes, django.contrib.messages and django.contrib.sessions. If these applications are not in your INSTALLED_APPS list, add them. Determine which of your application’s models should be editable in the admin interface. For each of those models, optionally create a ModelAdmin class that encapsulates the customized admin functionality and options for that particular model. Instantiate an AdminSite and tell it about each of your models and ModelAdmin classes. Hook the AdminSite instance into your URLconf.
I think you will get a non-functioning site in 1.4. 1.4 docs has this addition, and it is not mentioned in release notes.
Add django.contrib.messages.context_processors.messages to TEMPLATE_CONTEXT_PROCESSORS and MessageMiddleware to MIDDLEWARE_CLASSES. (These are both active by default, so you only need to do this if you’ve manually tweaked the settings.)
Of course, the proper test is to install Django + admin using 1.3 docs, upgrade to 1.4 using release notes and see if the admin still works. I haven't done that, but the above suggests there could be a problem.
Note that I am not saying that the dependency should be removed, just that the release notes should mention this.
comment:10 by , 13 years ago
OK, I tested upgrading from 1.3 to 1.4. In fact, things do work with the 1.3 default settings and admin installation according to docs.
The problem is if you have for some reason removed the MessageMiddleware from the default settings (or, maybe you are upgrading from 1.x -> 1.3 -> 1.4). In that case you will not get any warnings in 1.3, and in 1.4 things will not work. So, the problem is that in 1.3 MessageMiddleware was not required, in 1.4 it is and the release notes (or 1.3 warnings) do not mention this. 1.3 docs do not mention that MessageMiddleware is required. A note "make sure MessageMiddleware is in the MIDDLEWARE_CLASSES" should be in release notes. As this is a rare situation, and the error message is clear enough when you hit it, I don't think anything more needs to be done.
comment:11 by , 13 years ago
The deprecation timeline document says, under the 1.4 section:
[...] The Message model (in django.contrib.auth) [...] will be removed. The messages framework should be used instead. [...] Note that this means that the admin application will depend on the messages context processor.
IMHO adding notes with these kind of caveats to the release notes will render that document in a heavy piece and could introduce the risk of having manually maintained, duplicate information in two pieces of documentation.
Two different proposals:
- Maybe we can add prominent links pointing to the:
- Deprecation timeline and
- Latest release notes
(in that order) for users that are upgrading from previous versions to the Installation document (https://docs.djangoproject.com/en/1.4/intro/install/#remove-any-old-versions-of-django)?
- Currently, the main document index has this text in a link: "Release notes and upgrading instructions" but the target (https://docs.djangoproject.com/en/1.4/releases/) of such link is little more than a perfunctor¡y list of release notes documents, there are not actual upgrading intructions. Maybe we can add a prominent seealso or warning ReST admonition at the top of such document warning/pointing users upgrading from older version that they should read the deprecation timeline document.
Option 1 could be too heavy for a introductory installation document that is mostly read by new users. I'd prefer option 2.
comment:12 by , 13 years ago
I did not see that this was mentioned in the deprecation timeline. I just searched for "message" in the 1.4 release notes.
I would hope there would be a single document you need to follow to update. I am not sure, but it seems you need to read through both the deprecation time line and the release notes to upgrade, the first contains deprecations, the second backwards incompatible changes. At least adding the seealso/warning sounds like a good idea.
At this point I am not too attached to the "must add release note" idea. It is mentioned in the deprecation timeline. Install 1.3 + admin -> 1.4 works, and even if you hit the error you get a clear indication what went wrong. I am still leaving this open, but re-closing this doesn't sound like a bad idea to me.
comment:13 by , 13 years ago
The original error message was enough for us to see what the problem was - pretty clear. Just keep the system the way it is. I think everything works really great. I just thought it might be a bug ...
comment:14 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | reopened → closed |
reclosing as invalid. Sorry for the noise.
django.contrib.admin
is documented as depending ondjango.contrib.messages
, anddjango.contrib.messages.middleware.MessageMiddleware
. So unfortunately those things are not optional if you want to use the admin site.See https://docs.djangoproject.com/en/1.4/ref/contrib/admin/