feat: add FileDownloadCrawler#2043
Conversation
4669391 to
db4ded0
Compare
Pijukatel
left a comment
There was a problem hiding this comment.
For large files, it would be great to have something like https://pypi.org/project/smart-open/ , but our storages lacks interface to support such chunked uploads
vdusek
left a comment
There was a problem hiding this comment.
Otherwise it looks great
| ) | ||
|
|
||
| self._statistics.register_status_code(response.status_code) | ||
| context.request.state = RequestState.AFTER_NAV |
There was a problem hiding this comment.
From Claude:
The streamed path never sets
context.request.loaded_url, whereas the bufferedcrawl()path does (request.loaded_url = str(response.url)). As a result, in stream mode_check_url_after_redirectsis effectively skipped (it's guarded byif loaded_url is not None), so off-domain redirects aren't filtered by the enqueue strategy, andcontext.request.loaded_urlstaysNonefor user code. It's a silent divergence between the two modes. Thestream()API doesn't expose the final URL, so this may be hard to fix cleanly, but worth a note/follow-up.
| # been read yet. | ||
| crawler = FileDownloadCrawler( | ||
| stream=True, | ||
| # The streamed request runs under this timeout, so raise it for large downloads. |
There was a problem hiding this comment.
Claude's nit:
In stream mode
navigation_timeoutonly bounds connection establishment + headers, not the body download (that runs underrequest_handler_timeout, as the next comment correctly says). This wording reads as if the whole download is bounded bynavigation_timeout, so a reader tuning only this value for a large file would still hit the handler timeout.
| content = await context.http_response.read() | ||
| kvs = await context.get_key_value_store() | ||
| await kvs.set_value( | ||
| key=context.request.url.split('/')[-1], |
There was a problem hiding this comment.
Claude's nit:
context.request.url.split('/')[-1]yields''for a trailing-slash URL, andkvs.set_value(key='')would raise. The runnable example (file_download.py) uses the saferURL(context.request.url).name— worth matching here (and at line 73) so the docstring doesn't teach the more fragile pattern.
Description
FileDownloadCrawlerwith support for streaming the response body in the request handler.Issues
FileDownloadcrawler #875Testing
FileDownloadCrawler