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

FileResponse works with files asynchronously #3476

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/3313.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FileResponse from web_fileresponse.py uses a ThreadPoolExecutor to work with files asynchronously.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
@@ -82,6 +82,7 @@ Eric Sheng
Erich Healy
Eugene Chernyshov
Eugene Naydenov
Eugene Tolmachev
Evert Lammerts
FichteFoll
Frederik Gladhorn
19 changes: 11 additions & 8 deletions aiohttp/web_fileresponse.py
Original file line number Diff line number Diff line change
@@ -179,22 +179,24 @@ async def _sendfile_fallback(self, request: 'BaseRequest',
# os.sendfile() system call. This should be used on systems
# that don't support the os.sendfile().

# To avoid blocking the event loop & to keep memory usage low,
# fobj is transferred in chunks controlled by the
# constructor's chunk_size argument.
# To keep memory usage low,fobj is transferred in chunks
# controlled by the constructor's chunk_size argument.

writer = await super().prepare(request)
assert writer is not None

chunk_size = self._chunk_size
loop = asyncio.get_event_loop()

chunk = fobj.read(chunk_size)
chunk = await loop.run_in_executor(None, fobj.read, chunk_size)
while chunk:
await writer.write(chunk)
count = count - chunk_size
if count <= 0:
break
chunk = fobj.read(min(chunk_size, count))
chunk = await loop.run_in_executor(
None, fobj.read, min(chunk_size, count)
)

await writer.drain()
return writer
@@ -218,7 +220,8 @@ async def prepare(
filepath = gzip_path
gzip = True

st = filepath.stat()
loop = asyncio.get_event_loop()
st = await loop.run_in_executor(None, filepath.stat)

modsince = request.if_modified_since
if modsince is not None and st.st_mtime <= modsince.timestamp():
@@ -334,8 +337,8 @@ async def prepare(
self.headers[hdrs.CONTENT_RANGE] = 'bytes {0}-{1}/{2}'.format(
real_start, real_start + count - 1, file_size)

with filepath.open('rb') as fobj:
with (await loop.run_in_executor(None, filepath.open, 'rb')) as fobj:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tolmachofof blocking close of the fobj

Copy link
Contributor Author

@Tolmachofof Tolmachofof Apr 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aamalev thanks for your comment!
I think that something like _FileContextManager would solve this problem.
Simplified example:

class _FileContextManager:

    def __init__(self, filepath, flags, start):
        self._filepath = filepath
        self._flags = flags
        self._start = start
        self._fobj = None

    async def __aenter__(self):
        loop = asyncio.get_event_loop()
        self._fobj = await loop.run_in_executor(
            None, self._open, self._filepath, self._start
        )
        return self._fobj

    def _open(self, filepath, start):
        fobj = filepath.open(self._flags)
        if start:
            fobj.seek(start)
        return fobj

    async def __aexit__(self, exc_type, exc_val, exc_tb):
        loop = asyncio.get_event_loop()
        await loop.run_in_executor(None, self._fobj.close)
        self._fobj = None

If this acceptable i can make a PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the conversion with to try..finally is more suitable. _FileContextManager need members loop and executor, and this is too much for a one-time code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. try..finally statement sounds good. I will make a PR.

if start: # be aware that start could be None or int=0 here.
fobj.seek(start)
await loop.run_in_executor(None, fobj.seek, start)

return await self._sendfile(request, fobj, count)