[Django] #34392: Allow using test client response.json() with StreamingHttpResponse
Django
---------------------------------------------+------------------------
Reporter: vainu-arto | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
This doesn't work since the ClientMixin._parse_json() method uses
response.content directly instead of using the response.getvalue() wrapper
which abstracts away the difference between the Response implementations.
--
Ticket URL: <https://code.djangoproject.com/ticket/34392>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: vainu-arto | Owner: vainu-arto
Type: New feature | Status: assigned
Component: Testing framework | Version: dev
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 vainu-arto):
* owner: nobody => vainu-arto
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34392#comment:1>
Django
-----------------------------------+--------------------------------------
Reporter: vainu-arto | Owner: vainu-arto
Type: New feature | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
* stage: Unreviewed => Accepted
Comment:
OK, I think this is likely an ok addition.
`getvalue()` should allow for an async iterator if we're going to use it
this way. (Consuming `self` rather than `self.streaming_content` would be
one way, but the warning would need to be caught.)
--
Ticket URL: <https://code.djangoproject.com/ticket/34392#comment:2>
Django
-----------------------------------+--------------------------------------
Reporter: vainu-arto | Owner: vainu-arto
Type: New feature | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
Changes (by Carlton Gibson):
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34392#comment:3>
Django
-----------------------------------+--------------------------------------
Reporter: vainu-arto | Owner: vainu-arto
Type: New feature | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
Comment (by vainu-arto):
Replying to [comment:2 Carlton Gibson]:
> OK, I think this is likely an ok addition.
>
> `getvalue()` should allow for an async iterator if we're going to use it
this way. (Consuming `self` rather than `self.streaming_content` would be
one way, but the warning would need to be caught.)
Huh. Apparently trac chose not to notify me on your comment. Sorry about
the delay.
I don't really see the difference you are talking about here. "Consuming
self" means calling StreamingHttpResponse.__iter__, right? That just
returns self.streaming_content, ending up in the same place as far as I
can tell. Can you be more specific on what you are asking me to do
instead?
--
Ticket URL: <https://code.djangoproject.com/ticket/34392#comment:4>
Django
-----------------------------------+--------------------------------------
Reporter: vainu-arto | Owner: vainu-arto
Type: New feature | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
* cc: Carlton Gibson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34392#comment:5>
Django
-----------------------------------+--------------------------------------
Reporter: vainu-arto | Owner: vainu-arto
Type: New feature | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
Comment (by Carlton Gibson):
>"Consuming self" means calling StreamingHttpResponse.iter, right? That
just returns self.streaming_content, ending up in the same place as far as
I can tell.
No, that's not quite right. Rather `__iter__` and `__aiter__` both handle
mapping the ''wrong'' kind of iterator, and issue a warning, to let you
know that's likely not what you're after:
{{{
>>> import asyncio
>>> from django.conf import settings
>>> from django.http import StreamingHttpResponse
>>>
>>> settings.configure()
>>>
>>> async def content():
... yield 1
... await asyncio.sleep(1)
... yield 2
...
>>> r = StreamingHttpResponse(content())
>>> list(r)
/Users/carlton/Projects/Django/django/django/http/response.py:497:
Warning: StreamingHttpResponse must consume asynchronous iterators in
order to serve them synchronously. Use a synchronous iterator instead.
warnings.warn(
[b'1', b'2']
>>> r = StreamingHttpResponse(content())
>>> list(r.streaming_content)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: 'async_generator' object is not iterable
}}}
That is, calling `list` (or `join`, in the case at hand here) on the
response itself allows accessing the content, where doing the same on
`streaming_content` does not.
`getvalue()` is currently going via `streaming_content` and so (currently)
would hit the type error here. Moving to `…join(self)` would resolve that,
at the cost of the warning which would need to be captured.
Make sense?
--
Ticket URL: <https://code.djangoproject.com/ticket/34392#comment:6>
Django
-----------------------------------+--------------------------------------
Reporter: vainu-arto | Owner: vainu-arto
Type: New feature | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
Comment (by vainu-arto):
Replying to [comment:6 Carlton Gibson]:
resolve that, at the cost of the warning which would need to be captured,
in `ClientMixin`.
>
> Make sense?
So you are discussing a bug that currently exists in
StreamingHttpResponse.getvalue() when the content is an async iterator?
StreamingHttpResponse.json() doesn't currently work at all (regardless of
async/sync) and as such this issue isn't caused or made worse by the code
change suggested in this PR. Is fixing this only somewhat-related bug
considered a prerequisite to merging this change?
--
Ticket URL: <https://code.djangoproject.com/ticket/34392#comment:7>