[Django] #34556: StreamingHttpResponse documentation inaccuracy
Django
--------------------------------------------+------------------------
Reporter: Alexandre Spaeth | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: 4.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------
While working on adding typing in django-stubs for the async version of
StreamingHttpResponse, we noticed that the documentation for
`StreamingHttpResponse` mentions
> It should be given an iterator that yields bytestrings as content.
But reading at the code (and the tests), it’s clear that the content can
actually also be a string (or anything that can be converted to
bytestrings by `make_bytes`). If that’s the expected behaviour (and I’m
assuming it is, looking at the `content` documentation for
`HttpResponse`), I suggest we fix the documentation.
--
Ticket URL: <https://code.djangoproject.com/ticket/34556>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
----------------------------------+--------------------------------------
Reporter: Alexandre Spaeth | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: 4.2
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Alexandre Spaeth):
* has_patch: 0 => 1
Comment:
Created https://github.com/django/django/pull/16844
--
Ticket URL: <https://code.djangoproject.com/ticket/34556#comment:1>
Django
----------------------------------+--------------------------------------
Reporter: Alexandre Spaeth | Owner: nobody
Component: Documentation | Version: 4.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
Hello Alexandre!
I believe we'd have to close this ticket as invalid, see my reasoning
below:
On the one hand, a string *is* an iterator, so strictly speaking, a string
can be passed to `StreamingHttpResponse` because a string is an iterable
on its own.
On the other hand, the usage of `make_bytes` in `streaming_content` is
focused on ensuring that the `part` being returned is, in fact, bytes and
not other type.
Lastly, the documentation clearly says that `StreamingHttpResponse` is not
a subclass of `HttpResponse` so concepts that apply to the latter would
not apply to the former. Specifically, a `StreamingHttpResponse` has no
`content` attibute as described [https://docs.djangoproject.com/en/4.2/ref
/request-response/#django.http.StreamingHttpResponse here].
What tests are you referring to that pass a string to
`StreamingHttpResponse`?
Based on the above and after re-reading the docs for
`StreamingHttpResponse`, I think that the current text is correct and
complete so I'll close as invalid but please re-open if I misread or
misunderstood your report.
--
Ticket URL: <https://code.djangoproject.com/ticket/34556#comment:2>
Django
----------------------------------+--------------------------------------
Reporter: Alexandre Spaeth | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 4.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------
Comment (by Alexandre Spaeth):
Hello Natalia, I guess I wasn’t clear enough :-)
The `streaming_content` passed to the class constructor is an
`Iterable[bytes]` (or an `AsyncIterable[bytes]`), but according to
https://github.com/django/django/blob/6e32d1fa1dafd0c9cd9f93997ecebb26cd9a1b62/tests/httpwrappers/tests.py#L672,
it can also be an `Iterable[str]` or even `Iterable[object]` in the case
of a `memoryview` (and their respective async ones).
The documentation, as written now, assumes that only `Iterable[bytes]` are
acceptable, which is not the case.
I totally agree about the fact that the `streaming_content` is an iterable
of bytes though and wouldn’t touch this.
--
Ticket URL: <https://code.djangoproject.com/ticket/34556#comment:3>
Django
----------------------------------+------------------------------------
Reporter: Alexandre Spaeth | Owner: nobody
Component: Documentation | Version: 4.2
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
* status: closed => new
* resolution: invalid =>
* stage: Unreviewed => Accepted
Comment:
Thanks for the clarification, I now understand your point. Accepting then
following the linked examples and the implementation code that in fact
allows for iterator of: bytes, memoryviews, strings or other str-
convertable types.
--
Ticket URL: <https://code.djangoproject.com/ticket/34556#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Alexandre Spaeth | Owner: Alexandre
| Spaeth
Type: Uncategorized | Status: assigned
Component: Documentation | Version: 4.2
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Natalia Bidart):
* owner: nobody => Alexandre Spaeth
* status: new => assigned
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34556#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Alexandre Spaeth | Owner: Alexandre
| Spaeth
Component: Documentation | Version: 4.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"599f3e2cda50ab084915ffd08edb5ad6cad61415" 599f3e2c]:
{{{
#!CommitTicketReference repository=""
revision="599f3e2cda50ab084915ffd08edb5ad6cad61415"
Fixed #34556 -- Doc'd that StreamingHttpResponse accepts memoryviews and
strings iterators.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34556#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Alexandre Spaeth | Owner: Alexandre
| Spaeth
Type: Uncategorized | Status: closed
Component: Documentation | Version: 4.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):
In [changeset:"ddccecee91601ce790c82b0aa03b2fa751580361" ddccece]:
{{{
#!CommitTicketReference repository=""
revision="ddccecee91601ce790c82b0aa03b2fa751580361"
[4.2.x] Fixed #34556 -- Doc'd that StreamingHttpResponse accepts
memoryviews and strings iterators.
Backport of 599f3e2cda50ab084915ffd08edb5ad6cad61415 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34556#comment:7>