Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically close underlying file object when response closes #12

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

devmonkey22
Copy link

@devmonkey22 devmonkey22 commented Feb 3, 2020

Expected behaviour

Underlying file object should get closed when the response gets closed. The underlying StreamingHttpResponse should add the RangedFileReader to the _closable_objects list.

Actual behaviour

Without a RangedFileReader.close() method, the StreamingHttpResponse doesn't see the RangedFileReader (and its underlying file) as a closable object.

Description of fix

  • Added RangedFileReader.close() method, which allows underlying StreamingHttpResponse to add to _closable_objects.
  • Also initialized RangedFileReader.size using file_like.size property (if available, i.e. Django File/FieldFile) rather than reading entire file contents.

This supercedes the changes in #11

chugcup and others added 4 commits August 20, 2019 12:03
This commit fixes ResourceWarnings in Python 3 where file objects
do not appear to be closed at the end of the request, and the
Django server will emit ResourceWarnings about unclosed files.
The `_closable_objects` list is managed by the base FileResponse
class in Django and will call `.close()` on objects at the end
of the request.
Added RangedFileReader.close() method, which allows underlying StreamingHttpResponse to add to `_closable_objects` more naturally to fix unclosed files. Also initialized `RangedFileReader.size` using `file_like.size` property (if available, i.e. Django File/FieldFile) rather than reading entire file contents.
No longer need to add file to `_closable_objects` explicitly with new reader close() method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants