Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#17323 closed Cleanup/optimization (fixed)

Rename request.raw_post_data to request.body

Reported by: Carl Meyer Owned by: Adrian Holovaty
Component: HTTP handling Version: 1.3
Severity: Normal Keywords:
Cc: Donald Stufft Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

request.raw_post_data is a bad name. It has nothing to do with POST in particular, it's just the body of the HTTP request. This confuses users, and makes it look like Django doesn't understand how HTTP works. We should change the name to request.body and start a deprecation process.

Attachments (3)

rename-raw_post_data-to-body.diff (19.0 KB ) - added by Donald Stufft 12 years ago.
Rename raw_post_data to body
rename_raw_post_data_to_body_updated_docs.diff (20.7 KB ) - added by Donald Stufft 12 years ago.
Updated for Fix in Docs
rename-raw_post_data-to-body-updated2.diff (16.2 KB ) - added by Donald Stufft 12 years ago.
Removed Duplicate Tests + Added A Test to Verify raw_post_data == body

Download all attachments as: .zip

Change History (20)

by Donald Stufft, 12 years ago

Rename raw_post_data to body

comment:1 by Donald Stufft, 12 years ago

Owner: changed from nobody to Donald Stufft

Attached is a patch that fixes this ticket. I wasn't 100% sure on how the documentation/tests should be done.

For documentation I left the docs for raw_post_data alone (other then I added a deprecation in Django 1.4) and copy/pasted the raw_post_data and renamed it to body and added that it was added in Django 1.4.

For tests I copied all the raw_post_data tests and duplicated them, except testing against body. I left the original raw_post_data tests intact, but I catch PendingDeprecation and Deprecation Warnings in them so that those tests do not emit the warning.

Hopefully this is correct!

comment:2 by Donald Stufft, 12 years ago

Cc: Donald Stufft added

comment:3 by Donald Stufft, 12 years ago

Has patch: set

comment:4 by Preston Holmes, 12 years ago

Patch needs improvement: set

The documentation should reflect the new way immediately - docs don't go through a deprecation timeline they way the code does, it is only to give people a chance to fix code.

I would change the docs to say "changed in 1.4" and see if there is a way to note that the change was a name change.

A reference to this change is needed in both the release notes and the deprecation timeline:

https://docs.djangoproject.com/en/dev/internals/deprecation/

by Donald Stufft, 12 years ago

Updated for Fix in Docs

comment:5 by Donald Stufft, 12 years ago

Patch needs improvement: unset

This has been updated. You can see it at:

https://github.com/django/django/pull/90

Diff is available at:

https://github.com/django/django/pull/90.diff

Or i've uploaded a copy of the diff to the ticket as well.

comment:6 by Carl Meyer, 12 years ago

Patch needs improvement: set

warnings.catch_warnings() is Python 2.6+ only, and 1.4 is still supporting Python 2.5.

In any case, I don't think an entire duplication of the tests under the raw_post_data name is necessary. In fact, I don't think it's necessary to leave in tests for the backward-compat shim at all in a case like this.

comment:7 by Preston Holmes, 12 years ago

Does this render #9054 invalid?

in reply to:  7 comment:8 by Carl Meyer, 12 years ago

Replying to ptone:

Does this render #9054 invalid?

No, this is purely a name-change, it has zero functionality impact, so it's not going to have any effect on any other bugs (unless those bugs are also about the mis-naming of request.raw_post_data in which case they are probably dupes).

comment:9 by Donald Stufft, 12 years ago

I removed the duplicate tests.

Should I add in a test at all to test that raw_post_data still works? If so where should that live?

comment:10 by Donald Stufft, 12 years ago

Alright I updated this again so that we test that HttpRequest.raw_post_data is equal to HttpRequest.body.

The Pull Request is updated and i'll attach another updated diff.

by Donald Stufft, 12 years ago

Removed Duplicate Tests + Added A Test to Verify raw_post_data == body

comment:11 by Donald Stufft, 12 years ago

Patch needs improvement: unset

comment:12 by Adrian Holovaty, 12 years ago

Owner: changed from Donald Stufft to Adrian Holovaty

Thanks, dstufft. I'm going to have a look at this now.

in reply to:  12 comment:13 by Carl Meyer, 12 years ago

Replying to adrian:

Thanks, dstufft. I'm going to have a look at this now.

Oh, cool, I was just about to do so myself. Made one comment at github (posting it here in case you're not watching the pull request for comments): https://github.com/dstufft/django/commit/f857631197f5d1b55033735436231bd307e231c7#L0R430

comment:14 by Adrian Holovaty, 12 years ago

Resolution: fixed
Status: newclosed

In [17210]:

Fixed #17323 -- Renamed HttpRequest.raw_post_data to request.body. Thanks for the patch, dstufft

comment:15 by Adrian Holovaty, 12 years ago

Aha, Carl, sorry we got our wires crossed. It'll be much easier when we're on a single system. :) I'll let you commit the try/finally bit that you pointed out in that Github comment.

comment:16 by Donald Stufft, 12 years ago

For what it's worth. I pushed a fix to the pull request right as I got the email saying this ticket got closed for the try/finally :|

comment:17 by Carl Meyer, 12 years ago

In [17211]:

Refs #17323 -- Updated a test to use try/finally to avoid state leakage. Thanks dstufft for the patch.

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