Opened 15 years ago

Closed 13 years ago

#9886 closed (fixed)

File-like interface for HttpRequest

Reported by: Ivan Sagalaev Owned by: Ivan Sagalaev
Component: HTTP handling Version: 1.0
Severity: Keywords: streaming request
Cc: Alexander Koshelev, Mikhail Korobov Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is an implementation of a file-like interface for HttpRequest. Primary goal is to allow iterative reading and handling of raw request data without loading it into memory entirely. Secondary goal is refactoring some code duplication in ModPythonRequest and WSGIRequest.

Attachments (6)

9886.diff (12.1 KB ) - added by Ivan Sagalaev 15 years ago.
Patch for review
9886-with-docs-test.diff (16.3 KB ) - added by Ivan Sagalaev 14 years ago.
New patch with docs and tests
9886.3.diff (16.3 KB ) - added by Ivan Sagalaev 14 years ago.
Patch updated to current trunk
9886.4.diff (16.3 KB ) - added by Ivan Sagalaev 14 years ago.
Patch updated to current trunk
9886.5.diff (17.3 KB ) - added by Ivan Sagalaev 13 years ago.
Updated to current trunk, tests converted to unittests
9886.6.diff (17.3 KB ) - added by Ivan Sagalaev 13 years ago.
Patch updated to current trunk

Download all attachments as: .zip

Change History (17)

by Ivan Sagalaev, 15 years ago

Attachment: 9886.diff added

Patch for review

comment:1 by Ivan Sagalaev, 15 years ago

Has patch: set
Needs documentation: set
Needs tests: set

Attached first patch for review. No changes to docs and tests yet, just code. Here's an overview of what and why has changed:

  • Both subclasses of HttpRequest "publish" their internal byte stream to self._stream which is then accessed from HttpRequest
  • Both subclasses also define a boolean self._read_started that is set later to True from HttpRequest methods to signal that reading from the stream has started and things like raw_post_data and POST aren't acessible
  • On the other hand, if raw_post_data is accessed before any attempt to read from the stream it's wrapped in StringIO and becomes request._stream itself.
  • Parsing of post data by upload handlers and reading of raw_post_data is extracted into HttpRequest because they were almost identical in both subclasses. I liked very much a comment in one of the copies of _load_post_and_files saying "read the proper comment in another file" :-)
  • core.handlers.wsgi now has a special wrapper class LimitedInput which takes a stream, content_length as its size limit and defines read and readline for it. Important thing is that it's used only when working with development server. It is needed because dev server passes as 'wsgi.input' a pure socket._fileobject which hangs and waits forever when one tries to read past data that it has in it. I decided not to wrap proper production streams from mod_python and wsgi servers into LimitedInput to be able to use their own implementations of read and readline that I suspect to be more efficient. Both mod_python and flup are smart enough to close streams when read completely.

Comments are welcome!

comment:2 by Ivan Sagalaev, 15 years ago

Just tested with Apache + mod_wsgi. Works good.

comment:3 by Jacob, 15 years ago

Triage Stage: UnreviewedDesign decision needed

comment:4 by Ivan Sagalaev, 15 years ago

Jacob, Malcolm, please clarify does "DDN" mean that we should discuss it in django-developers and if yes, does it make sense to do it after or before 1.1?

comment:5 by Jacob, 15 years ago

I marked this DDN because I want a second opinion before this gets accepted; bringing it to django-dev is one way for you to prod that process along.

However, since this is a feature addition it would need to be in the beta to get into 1.1, and there's a long list of other features we decided upon earlier in the release process. So realistically if this gets in it'll be 1.2.

by Ivan Sagalaev, 14 years ago

Attachment: 9886-with-docs-test.diff added

New patch with docs and tests

comment:6 by Alexander Koshelev, 14 years ago

Cc: Alexander Koshelev added

comment:7 by Ivan Sagalaev, 14 years ago

Needs documentation: unset
Needs tests: unset
Owner: changed from Malcolm Tredinnick to Ivan Sagalaev

by Ivan Sagalaev, 14 years ago

Attachment: 9886.3.diff added

Patch updated to current trunk

comment:8 by Ivan Sagalaev, 14 years ago

An answer to Russel's concern about it playing well with WSGI improvement branch: http://groups.google.com/group/django-developers/msg/a0716eff23949427

I've looked at the branch's code and am certain that we don't conflict. That branch is changing response part of HTTP handling only and this patch is about refactoring request part. That simple :-)

by Ivan Sagalaev, 14 years ago

Attachment: 9886.4.diff added

Patch updated to current trunk

by Ivan Sagalaev, 13 years ago

Attachment: 9886.5.diff added

Updated to current trunk, tests converted to unittests

comment:9 by Jacob, 13 years ago

Triage Stage: Design decision neededReady for checkin

comment:10 by Mikhail Korobov, 13 years ago

Cc: Mikhail Korobov added

by Ivan Sagalaev, 13 years ago

Attachment: 9886.6.diff added

Patch updated to current trunk

comment:11 by Russell Keith-Magee, 13 years ago

Resolution: fixed
Status: newclosed

(In [14394]) Fixed #9886 -- Added a file-like interface to HttpRequest. Thanks to Ivan Sagalaev for the suggestion and patch.

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