FileResponse documentation should warn against using context managers

[Mike DePalatis]

Idiomatic Python usually opens file objects using a context manager like this:

with open("filename.txt", "rb") as fo:
    # do stuff with fo

This is problematic when using a FileResponse which requires a file-like object rather than a path. Doing something like:

with open("filename.txt", "rb") as fo:
    return FileResponse(fo)

results in an error due to trying to operate on a closed file:

Exception happened during processing of request from ('127.0.0.1', 60169)
Traceback (most recent call last):
  File "/Users/zduey/anaconda3/envs/cml/lib/python3.5/wsgiref/handlers.py", line 138, in run
    self.finish_response()
  File "/Users/zduey/anaconda3/envs/cml/lib/python3.5/wsgiref/handlers.py", line 179, in finish_response
    for data in self.result:
  File "/Users/zduey/anaconda3/envs/cml/lib/python3.5/wsgiref/util.py", line 30, in __next__
    data = self.filelike.read(self.blksize)
ValueError: read of closed file

The documentation for FileResponse should be updated to explicitly warn users against passing in file objects opened with a context manager and make clear that the file will be closed automatically.

[Windson yang]

​PR

[andrei kulakov]

I don't think this requires a warning because the sole function and purpose of with construct is to close file objects / clean up state when done with the block. By the same logic we would have to document all Python features in Django docs.

[Mike DePalatis]

Replying to andrei kulakov:

I don't think this requires a warning because the sole function and purpose of with construct is to close file objects / clean up state when done with the block. By the same logic we would have to document all Python features in Django docs.

I don't think all Python features need to be documented. However, this is a special case where the required way of opening a file goes against the usual best practices and thus it makes sense that this should be explicitly noted.

[andrei kulakov]

Replying to Mike DePalatis:

I don't think all Python features need to be documented. However, this is a special case where the required way of opening a file goes against the usual best practices and thus it makes sense that this should be explicitly noted.

In theory, perhaps, but in this case someone using this best practice needs to understand the reason for 'with' construct. It's a fairly common construct and its
use is much more general than just file objects. Also the way it works is neither very obscure nor complex to understand.

You could also argue that the best practice is "use with construct and complete all work on file within the with block", and so this best practice does not apply
here because obviously no work is completed in the example.

[Claude Paroz]

I think it's useful for users to know FileResponse will close the file itself, and that it's not possible to close the file before the response has finished streaming it.

[Tim Graham <timograham@…>]

In 4fe5d846:

Fixed #29278 -- Doc'd that a context manager can't be used with FileResponse.

[Tim Graham <timograham@…>]

In a390cc71:

[2.0.x] Fixed #29278 -- Doc'd that a context manager can't be used with FileResponse.

Backport of 4fe5d846666d46a5395a5f0ea2845a96b6837a75 from master