Opened 7 years ago

Closed 5 years ago

#9886 closed (fixed)

File-like interface for HttpRequest

Reported by: isagalaev Owned by: isagalaev
Component: HTTP handling Version: 1.0
Severity: Keywords: streaming request
Cc: alexkoshelev, kmike Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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 isagalaev 7 years ago.
Patch for review
9886-with-docs-test.diff (16.3 KB) - added by isagalaev 6 years ago.
New patch with docs and tests
9886.3.diff (16.3 KB) - added by isagalaev 6 years ago.
Patch updated to current trunk
9886.4.diff (16.3 KB) - added by isagalaev 5 years ago.
Patch updated to current trunk
9886.5.diff (17.3 KB) - added by isagalaev 5 years ago.
Updated to current trunk, tests converted to unittests
9886.6.diff (17.3 KB) - added by isagalaev 5 years ago.
Patch updated to current trunk

Download all attachments as: .zip

Change History (17)

Changed 7 years ago by isagalaev

Patch for review

comment:1 Changed 7 years ago by isagalaev

  • Has patch set
  • Needs documentation set
  • Needs tests set
  • Patch needs improvement unset

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 Changed 7 years ago by isagalaev

Just tested with Apache + mod_wsgi. Works good.

comment:3 Changed 7 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 7 years ago by isagalaev

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 Changed 7 years ago by jacob

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.

Changed 6 years ago by isagalaev

New patch with docs and tests

comment:6 Changed 6 years ago by alexkoshelev

  • Cc alexkoshelev added

comment:7 Changed 6 years ago by isagalaev

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from mtredinnick to isagalaev

Changed 6 years ago by isagalaev

Patch updated to current trunk

comment:8 Changed 6 years ago by isagalaev

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 :-)

Changed 5 years ago by isagalaev

Patch updated to current trunk

Changed 5 years ago by isagalaev

Updated to current trunk, tests converted to unittests

comment:9 Changed 5 years ago by jacob

  • Triage Stage changed from Design decision needed to Ready for checkin

comment:10 Changed 5 years ago by kmike

  • Cc kmike added

Changed 5 years ago by isagalaev

Patch updated to current trunk

comment:11 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(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