#30565: Close StreamingHttpResponse content immediately after iterating it
-------------------------------------+-------------------------------------
     Reporter:  Chris Jerdonek       |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  HTTP handling        |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  HttpResponse,        |             Triage Stage:
  streaming, StreamingHttpResponse   |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Chris Jerdonek):

 * version:  2.2 => master
 * type:  Uncategorized => Cleanup/optimization


Old description:

> This ticket is to suggest doing for `StreamingHttpResponse` what #25725
> did for `HttpReponse`, namely to close the underlying content iterator
> after it has been iterated over.
>
> Currently, if creating a `StreamingHttpResponse` from a file-like object,
> it doesn't seem like there's an obvious way to close the underlying file
> after the file has been streamed. And as
> [https://code.djangoproject.com/ticket/25725#comment:1 one of the
> comments] in #25725 pointed out, trying to do this in
> `StreamingHttpResponse.close()` isn't a good solution because WSGI
> servers can't be relied upon to call `close()`.
>
> I believe an alternative, more reliable solution may be to call `close()`
> immediately after the iterator has been exhausted (if `hasattr(value,
> 'close')` is true, as #25725 does). This is essentially what #25725 did
> for non-streaming `HttpResponse` objects. Here is that code:
>
> {{{
> #!python
> def content(self, value):
>     # Consume iterators upon assignment to allow repeated iteration.
>     if hasattr(value, '__iter__') and not isinstance(value, (bytes,
> str)):
>         content = b''.join(self.make_bytes(chunk) for chunk in value)
>         if hasattr(value, 'close'):
>             try:
>                 value.close()
>             except Exception:
>                 pass
>     else:
>         content = self.make_bytes(value)
> }}}
>

> (from
> https://github.com/django/django/blob/1564e42ad397021093585147875a21dae1a3b3fc/django/http/response.py#L310-L319
> )
>
> In the streaming case, the content `value` argument could be wrapped
> something like so (inside
> `StreamingHttpResponse._set_streaming_content(value)`):
>
> {{{
> #!python
> def iter_content():
>     yield from value
>     if hasattr(value, 'close'):
>         try:
>             value.close()
>         except Exception:
>             pass
>
> new_value = iter_content()
> }}}

New description:

 This ticket is to suggest doing for `StreamingHttpResponse` what #25725
 did for `HttpReponse`, namely to close the underlying content iterator
 after it has been iterated over.

 Currently, if creating a `StreamingHttpResponse` from a file-like object,
 it doesn't seem like there's an obvious way to close the underlying file
 after the file has been streamed. And as
 [https://code.djangoproject.com/ticket/25725#comment:1 one of the
 comments] in #25725 pointed out, trying to do this in
 `StreamingHttpResponse.close()` isn't a good solution because WSGI servers
 can't be relied upon to call `close()`.

 I believe an alternative, more reliable solution may be to call `close()`
 immediately after the iterator has been exhausted (if `hasattr(value,
 'close')` is true, as #25725 does). This is essentially what #25725 did
 for non-streaming `HttpResponse` objects. Here is that code:

 {{{
 #!python
 def content(self, value):
     # Consume iterators upon assignment to allow repeated iteration.
     if hasattr(value, '__iter__') and not isinstance(value, (bytes, str)):
         content = b''.join(self.make_bytes(chunk) for chunk in value)
         if hasattr(value, 'close'):
             try:
                 value.close()
             except Exception:
                 pass
     else:
         content = self.make_bytes(value)
 }}}


 (from
 
https://github.com/django/django/blob/1564e42ad397021093585147875a21dae1a3b3fc/django/http/response.py#L310-L319
 )

 In the streaming case, the content `value` argument could be wrapped
 something like so (inside
 `StreamingHttpResponse._set_streaming_content(value)`):

 {{{
 #!python
 def iter_content():
     yield from value
     if hasattr(value, 'close'):
         try:
             value.close()
         except Exception:
             pass

 new_value = iter_content()
 }}}

 Here is the current code for
 `StreamingHttpResponse._set_streaming_content()`:

 {{{
 #!python
 def _set_streaming_content(self, value):
     # Ensure we can never iterate on "value" more than once.
     self._iterator = iter(value)
     if hasattr(value, 'close'):
         self._closable_objects.append(value)
 }}}

 (from:
 
https://github.com/django/django/blob/1564e42ad397021093585147875a21dae1a3b3fc/django/http/response.py#L376-L380)

--

-- 
Ticket URL: <https://code.djangoproject.com/ticket/30565#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.f71d16658bfc5a597f329f87269e7cb7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to