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)

0001-Fix-16559-Deprecate-mimetype-for-HttpResponse.patch (17.8 KB ) - added by Yaşar Arabacı 12 years ago.
Fixed all necessary things, but didn't write tests.

Download all attachments as: .zip

Change History (15)

comment:1 by Jannis Leidel, 13 years ago

Triage Stage: UnreviewedAccepted

I don't think it hurts to keep the backwards compatibility code for another release, so this should definitely be a PendingDeprecationWarning

comment:2 by Paul McMillan, 13 years ago

Owner: changed from nobody to Paul McMillan

I suppose it doesn't hurt anything to have it pending for a bit. I'll work on a patch when I get a chance.

Last edited 13 years ago by Paul McMillan (previous) (diff)

comment:3 by Paul McMillan, 13 years ago

Easy pickings: set
Owner: Paul McMillan removed

comment:4 by Yaşar Arabacı, 12 years ago

Owner: set to Yaşar Arabacı
Status: newassigned

comment:5 by Yaşar Arabacı, 12 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 Paul McMillan, 12 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 Yaşar Arabacı, 12 years ago

Cc: Yaşar Arabacı 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 :)

in reply to:  5 comment:8 by Ramiro Morales, 12 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 Yaşar Arabacı, 12 years ago

Fixed all necessary things, but didn't write tests.

comment:9 by Yaşar Arabacı, 12 years ago

Has patch: set
Patch needs improvement: set
Triage Stage: AcceptedReady 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 Yaşar Arabacı, 12 years ago

Triage Stage: Ready for checkinAccepted

comment:11 by Yaşar Arabacı, 12 years ago

Sorry about marking as RFC. I didn't know I didn't supposed to do that.

comment:12 by Adrian Holovaty, 12 years ago

In [17223]:

Converted some of the built-in views to use content_type instead of mimetype HttpResponse argument. Refs #16519

comment:13 by Claude Paroz <claude@…>, 12 years ago

In [deed192dda621f1cdc8eb7240f1e5914045402dd]:

Removed usage of mimetype kwarg of HttpResponse

Refs #16519.

comment:14 by Claude Paroz <claude@…>, 12 years ago

Resolution: fixed
Status: assignedclosed

In [da200c5e35cdbb617572977bcbf97fae33920b2e]:

Fixed #16519 -- Deprecated mimetype kwarg of HttpResponse init

This keyword was already deprecated in the code (supported for
backwards compatibility only), but never formally deprecated.
Thanks Paul McMillan for the report and yasar11732 for the initial
patch.

Note: See TracTickets for help on using tickets.
Back to Top