Opened 13 years ago
Closed 12 years ago
#16519 closed Cleanup/optimization (fixed)
mimetype used with HttpResponse should be deprecated
Reported by: | Paul McMillan | Owned by: | Yaşar Arabacı |
---|---|---|---|
Component: | HTTP handling | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Yaşar Arabacı | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | yes | UI/UX: | no |
Description
Passing the mimetype
argument when instantiating HttpResponse
should raise a DeprecationWarning
.
The mimetype argument has been deprecated via comment in the code for 4 years: https://code.djangoproject.com/changeset/5844/django/trunk/django/http/__init__.py
It's about time we actually raised a DeprecationWarning
and remove the thing ASAP. If we swap the position of the mimetype and content_type arguments, code the relies on argument position will continue to function without raising the warning. The only code that will need to be changed is code that explicitly uses the mimetype kwarg (including a bunch of django tests).
This situation is long-lived enough to bypass the PendingDeprecationWarning.
Attachments (1)
Change History (15)
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 13 years ago
Owner: | changed from | to
---|
Alright. I suppose it doesn't hurt anything. And as to the matter of re-ordering the kwargs? That'll fix a great deal of existing code directly.
comment:3 by , 13 years ago
Easy pickings: | set |
---|---|
Owner: | removed |
comment:4 by , 13 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
follow-up: 8 comment:5 by , 13 years ago
I am new to contributing, and I shoud ask if I should fix contrib packages too. I mean replacing mimetype with content_type where needed?
comment:6 by , 13 years ago
Yes, you'll need to fix it everywhere it occurs in the Django codebase, including the tests. You'll also probably need to write some new tests. If you swap the order of mimetype and content_type in the definition, the functions that are using mimetype as a positional argument will transition more easily.
comment:7 by , 13 years ago
Cc: | added |
---|
Ok. A patch would be available in couple of days I guess. I am excited as this will be my first actual contrib to django :)
comment:8 by , 13 years ago
Replying to yasar11732:
I am new to contributing, and I shoud ask if I should fix contrib packages too. I mean replacing mimetype with content_type where needed?
If you are talking about django.contrib.*
then the answer is yes. Thanks for looking at this.
by , 13 years ago
Attachment: | 0001-Fix-16559-Deprecate-mimetype-for-HttpResponse.patch added |
---|
Fixed all necessary things, but didn't write tests.
comment:9 by , 13 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Accepted → Ready for checkin |
I made a patch file where I made warning and changed mimetype kwarg, to content_type kwarg. However, I didn't write tests for it, as I am not sure what I am test for here, something like this maybe?
import warnings from django.http import HttpResponse with warnings.catch_warnings(record=True) as w: warnings.simplefilter("always") HttpResponse(mimetype="text/html") assert len(w) == 1 assert issubclass(w[-1].category, DeprecationWarning) assert "deprecated" in str(w[-1].message)
comment:10 by , 13 years ago
Triage Stage: | Ready for checkin → Accepted |
---|
comment:11 by , 13 years ago
Sorry about marking as RFC. I didn't know I didn't supposed to do that.
comment:14 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
I don't think it hurts to keep the backwards compatibility code for another release, so this should definitely be a
PendingDeprecationWarning