Opened 12 years ago
Closed 12 years ago
#18796 closed Cleanup/optimization (fixed)
Refactor HttpResponse.__iter__ / _get_content
Reported by: | Aymeric Augustin | Owned by: | Aymeric Augustin |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The current implementation isn't DRY. It seems to me that _get_content
should just return b''.join(self)
.
Unfortunately, implementing this naively results in test failures.
This code is sensitive, tricky, and lacks comment. A refactoring would be useful.
See also #4969, #16494, #18764, [5f2d9cdbb1], [2892cb0ec4], and [536b030363].
To sum up: this ticket is about cleaning up the conversion of HTTP response content to bytes.
Attachments (1)
Change History (7)
comment:2 by , 12 years ago
Owner: | changed from | to
---|
comment:3 by , 12 years ago
This comment was made on #7581 but it actually falls in the scope of this ticket:
Returning self._iterator
directly in HttpStreamingResponse.streaming_content
, instead of a new iterator that yields bytestrings means that middleware authors can't expect consistent bytestring chunks when wrapping streaming_content in a new iterator? They may have to type check or coerce each chunk (potentially reproducing something like make_bytes()
? For comparison, HttpResponse.content
always returns a bytestring.
comment:4 by , 12 years ago
Description: | modified (diff) |
---|
by , 12 years ago
Attachment: | 18796.patch added |
---|
comment:6 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
When
HttpResponse
is instantiated with an iterator, the contents of the iterator should be consumed and saved as soon asresponse.content
is accessed (or other methods requiring knowledge of the response's content).We probably want to keep the current (undocumented) streaming behavior of
HttpResponse
and not unconditionally consume the iterator in__init__
.This was initially part of #7581 — but I'm reducing the scope of that ticket to something manageable.