From 7188fa80b7feb74d9d3284ce5922aadb9fdbd0ae Mon Sep 17 00:00:00 2001 From: Stuart Mumford Date: Wed, 15 Dec 2021 17:00:20 +0000 Subject: [PATCH] Remove the default total download timeout. Also close the request which gets the headers before opening new ones when transferring data. --- parfive/downloader.py | 29 +++++++++++++++-------------- parfive/tests/test_downloader.py | 2 +- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index 5d64f0ec..06a0060c 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -265,9 +265,9 @@ async def run_download(self, timeouts=None): ---------- timeouts : `dict`, optional Overrides for the default timeouts for http downloads. Supported - keys are any accepted by the `aiohttp.ClientTimeout` class. Defaults - to 5 minutes for total session timeout and 90 seconds for socket - read timeout. + keys are any accepted by the `aiohttp.ClientTimeout` class. + Defaults to no timeout for total session timeout (overriding the + aiohttp 5 minute default) and 90 seconds for socket read timeout. Returns ------- @@ -284,7 +284,7 @@ async def run_download(self, timeouts=None): if "PARFIVE_DEBUG" in os.environ: self._configure_debug() - timeouts = timeouts or {"total": float(os.environ.get("PARFIVE_TOTAL_TIMEOUT", 5 * 60)), + timeouts = timeouts or {"total": float(os.environ.get("PARFIVE_TOTAL_TIMEOUT", 0)), "sock_read": float(os.environ.get("PARFIVE_SOCK_READ_TIMEOUT", 90))} total_files = self.queued_downloads @@ -322,9 +322,9 @@ def download(self, timeouts=None): ---------- timeouts : `dict`, optional Overrides for the default timeouts for http downloads. Supported - keys are any accepted by the `aiohttp.ClientTimeout` class. Defaults - to 5 minutes for total session timeout and 90 seconds for socket - read timeout. + keys are any accepted by the `aiohttp.ClientTimeout` class. + Defaults to no timeout for total session timeout (overriding the + aiohttp 5 minute default) and 90 seconds for socket read timeout. Returns ------- @@ -553,13 +553,14 @@ async def _get_http(self, session, *, url, filepath_partial, chunksize=None, session, url, chunksize, None, timeout, downloaded_chunk_queue, **kwargs )) ) + # Close the initial request here before we start transferring data. - # run all the download workers - await asyncio.gather(*download_workers) - # join() waits till all the items in the queue have been processed - await downloaded_chunk_queue.join() - writer.cancel() - return str(filepath) + # run all the download workers + await asyncio.gather(*download_workers) + # join() waits till all the items in the queue have been processed + await downloaded_chunk_queue.join() + writer.cancel() + return str(filepath) except Exception as e: raise FailedDownload(filepath_partial, url, e) @@ -650,7 +651,7 @@ async def _http_download_worker(self, session, url, chunksize, http_range, timeo offset = 0 async with session.get(url, timeout=timeout, headers=headers, **kwargs) as resp: - parfive.log.debug("%s request made to %s with headers=%s", + parfive.log.debug("%s request made for download to %s with headers=%s", resp.request_info.method, resp.request_info.url, resp.request_info.headers) diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index d2e6c56f..425f2d54 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -460,7 +460,7 @@ def test_proxy_passed_as_kwargs_to_get(tmpdir, url, proxy): assert patched.called, "`ClientSession._request` not called" assert list(patched.call_args) == [('GET', url), {'allow_redirects': True, - 'timeout': ClientTimeout(total=300, connect=None, sock_read=90, sock_connect=None), + 'timeout': ClientTimeout(total=0, connect=None, sock_read=90, sock_connect=None), 'proxy': proxy }]