mirror of https://github.com/scrapy/scrapy.git
Added download_async() to ExecutionEngine and test cases for download() and download_async() (#6842)
* added async_download()
* add case tests for ExecutionEngine download() and async_download()
* change async_download() to download_async() in Engine class and subsequent tests
* fixed reactor import in test_engine.py
* Simplify TestEngineDownloadAsync.
* replaced tearDown by self.engine.downloader.close in test_engine.py and replace @defer.inlineCallbacks by @inlineCallbacks
* Simplify ExecutionEngine.download().
* Remove an unneeded import.
* Refactor ExecutionEngine.download{,_async}() tests.
* Deprecate ExecutionEngine.download().
* Update doc examples to use download_async().
* Replace uses of download() with download_async().
* Fix test_downloadermiddleware_robotstxt.py.
* Remove a stray file.
---------
Co-authored-by: Andrey Rakhmatullin <wrar@wrar.name>
This commit is contained in:
parent
6ce0c4c855
commit
89f53f0555
|
|
@ -266,7 +266,6 @@ within a spider callback:
|
|||
.. code-block:: python
|
||||
|
||||
from scrapy import Spider, Request
|
||||
from scrapy.utils.defer import maybe_deferred_to_future
|
||||
|
||||
|
||||
class SingleRequestSpider(Spider):
|
||||
|
|
@ -275,8 +274,9 @@ within a spider callback:
|
|||
|
||||
async def parse(self, response, **kwargs):
|
||||
additional_request = Request("https://example.org/price")
|
||||
deferred = self.crawler.engine.download(additional_request)
|
||||
additional_response = await maybe_deferred_to_future(deferred)
|
||||
additional_response = await self.crawler.engine.download_async(
|
||||
additional_request
|
||||
)
|
||||
yield {
|
||||
"h1": response.css("h1").get(),
|
||||
"price": additional_response.css("#price").get(),
|
||||
|
|
@ -286,9 +286,9 @@ You can also send multiple requests in parallel:
|
|||
|
||||
.. code-block:: python
|
||||
|
||||
import asyncio
|
||||
|
||||
from scrapy import Spider, Request
|
||||
from scrapy.utils.defer import maybe_deferred_to_future
|
||||
from twisted.internet.defer import DeferredList
|
||||
|
||||
|
||||
class MultipleRequestsSpider(Spider):
|
||||
|
|
@ -300,11 +300,11 @@ You can also send multiple requests in parallel:
|
|||
Request("https://example.com/price"),
|
||||
Request("https://example.com/color"),
|
||||
]
|
||||
deferreds = []
|
||||
tasks = []
|
||||
for r in additional_requests:
|
||||
deferred = self.crawler.engine.download(r)
|
||||
deferreds.append(deferred)
|
||||
responses = await maybe_deferred_to_future(DeferredList(deferreds))
|
||||
task = self.crawler.engine.download_async(r)
|
||||
tasks.append(task)
|
||||
responses = await asyncio.gather(*tasks)
|
||||
yield {
|
||||
"h1": response.css("h1::text").get(),
|
||||
"price": responses[0][1].css(".price::text").get(),
|
||||
|
|
|
|||
|
|
@ -190,7 +190,6 @@ item.
|
|||
import scrapy
|
||||
from itemadapter import ItemAdapter
|
||||
from scrapy.http.request import NO_CALLBACK
|
||||
from scrapy.utils.defer import maybe_deferred_to_future
|
||||
|
||||
|
||||
class ScreenshotPipeline:
|
||||
|
|
@ -204,9 +203,7 @@ item.
|
|||
encoded_item_url = quote(adapter["url"])
|
||||
screenshot_url = self.SPLASH_URL.format(encoded_item_url)
|
||||
request = scrapy.Request(screenshot_url, callback=NO_CALLBACK)
|
||||
response = await maybe_deferred_to_future(
|
||||
spider.crawler.engine.download(request)
|
||||
)
|
||||
response = await spider.crawler.engine.download_async(request)
|
||||
|
||||
if response.status != 200:
|
||||
# Error happened, return item.
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ from __future__ import annotations
|
|||
|
||||
import asyncio
|
||||
import logging
|
||||
import warnings
|
||||
from time import time
|
||||
from traceback import format_exc
|
||||
from typing import TYPE_CHECKING, Any, cast
|
||||
|
|
@ -19,7 +20,12 @@ from twisted.python.failure import Failure
|
|||
from scrapy import signals
|
||||
from scrapy.core.scheduler import BaseScheduler
|
||||
from scrapy.core.scraper import Scraper
|
||||
from scrapy.exceptions import CloseSpider, DontCloseSpider, IgnoreRequest
|
||||
from scrapy.exceptions import (
|
||||
CloseSpider,
|
||||
DontCloseSpider,
|
||||
IgnoreRequest,
|
||||
ScrapyDeprecationWarning,
|
||||
)
|
||||
from scrapy.http import Request, Response
|
||||
from scrapy.utils.asyncio import AsyncioLoopingCall, create_looping_call
|
||||
from scrapy.utils.defer import (
|
||||
|
|
@ -372,18 +378,28 @@ class ExecutionEngine:
|
|||
signals.request_dropped, request=request, spider=self.spider
|
||||
)
|
||||
|
||||
@inlineCallbacks
|
||||
def download(self, request: Request) -> Generator[Deferred[Any], Any, Response]:
|
||||
def download(self, request: Request) -> Deferred[Response]:
|
||||
"""Return a Deferred which fires with a Response as result, only downloader middlewares are applied"""
|
||||
warnings.warn(
|
||||
"ExecutionEngine.download() is deprecated, use download_async() instead",
|
||||
ScrapyDeprecationWarning,
|
||||
stacklevel=2,
|
||||
)
|
||||
return deferred_from_coro(self.download_async(request))
|
||||
|
||||
async def download_async(self, request: Request) -> Response:
|
||||
"""Asynchronous version of download() that returns a Response."""
|
||||
if self.spider is None:
|
||||
raise RuntimeError(f"No open spider to crawl: {request}")
|
||||
try:
|
||||
response_or_request = yield self._download(request)
|
||||
response_or_request = await maybe_deferred_to_future(
|
||||
self._download(request)
|
||||
)
|
||||
finally:
|
||||
assert self._slot is not None
|
||||
self._slot.remove_request(request)
|
||||
if isinstance(response_or_request, Request):
|
||||
return (yield self.download(response_or_request))
|
||||
return await self.download_async(response_or_request)
|
||||
return response_or_request
|
||||
|
||||
@inlineCallbacks
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from twisted.internet.defer import Deferred, maybeDeferred
|
|||
from scrapy.exceptions import IgnoreRequest, NotConfigured
|
||||
from scrapy.http import Request, Response
|
||||
from scrapy.http.request import NO_CALLBACK
|
||||
from scrapy.utils.defer import deferred_from_coro
|
||||
from scrapy.utils.httpobj import urlparse_cached
|
||||
from scrapy.utils.log import failure_to_exc_info
|
||||
from scrapy.utils.misc import load_object
|
||||
|
|
@ -105,7 +106,7 @@ class RobotsTxtMiddleware:
|
|||
)
|
||||
assert self.crawler.engine
|
||||
assert self.crawler.stats
|
||||
dfd = self.crawler.engine.download(robotsreq)
|
||||
dfd = deferred_from_coro(self.crawler.engine.download_async(robotsreq))
|
||||
dfd.addCallback(self._parse_robots, netloc, spider)
|
||||
dfd.addErrback(self._logerror, robotsreq, spider)
|
||||
dfd.addErrback(self._robots_error, netloc)
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ from scrapy.http.request import NO_CALLBACK, Request
|
|||
from scrapy.settings import Settings
|
||||
from scrapy.utils.asyncio import call_later
|
||||
from scrapy.utils.datatypes import SequenceExclude
|
||||
from scrapy.utils.defer import _DEFER_DELAY, _defer_sleep
|
||||
from scrapy.utils.defer import _DEFER_DELAY, _defer_sleep, deferred_from_coro
|
||||
from scrapy.utils.log import failure_to_exc_info
|
||||
from scrapy.utils.misc import arg_to_iter
|
||||
from scrapy.utils.python import get_func_args, global_object_name
|
||||
|
|
@ -246,7 +246,9 @@ class MediaPipeline(ABC):
|
|||
else:
|
||||
self._modify_media_request(request)
|
||||
assert self.crawler.engine
|
||||
response = yield self.crawler.engine.download(request)
|
||||
response = yield deferred_from_coro(
|
||||
self.crawler.engine.download_async(request)
|
||||
)
|
||||
return self.media_downloaded(response, request, info, item=item)
|
||||
except Exception:
|
||||
failure = self.media_failed(Failure(), request, info)
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from scrapy.exceptions import IgnoreRequest, NotConfigured
|
|||
from scrapy.http import Request, Response, TextResponse
|
||||
from scrapy.http.request import NO_CALLBACK
|
||||
from scrapy.settings import Settings
|
||||
from scrapy.utils.asyncio import call_later
|
||||
from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future
|
||||
from tests.test_robotstxt_interface import rerp_available
|
||||
|
||||
|
|
@ -25,7 +26,7 @@ class TestRobotsTxtMiddleware:
|
|||
def setup_method(self):
|
||||
self.crawler = mock.MagicMock()
|
||||
self.crawler.settings = Settings()
|
||||
self.crawler.engine.download = mock.MagicMock()
|
||||
self.crawler.engine.download_async = mock.AsyncMock()
|
||||
|
||||
def teardown_method(self):
|
||||
del self.crawler
|
||||
|
|
@ -51,14 +52,12 @@ Disallow: /some/randome/page.html
|
|||
""".encode()
|
||||
response = TextResponse("http://site.local/robots.txt", body=ROBOTS)
|
||||
|
||||
def return_response(request):
|
||||
from twisted.internet import reactor
|
||||
|
||||
async def return_response(request):
|
||||
deferred = Deferred()
|
||||
reactor.callFromThread(deferred.callback, response)
|
||||
return deferred
|
||||
call_later(0, deferred.callback, response)
|
||||
return await maybe_deferred_to_future(deferred)
|
||||
|
||||
crawler.engine.download.side_effect = return_response
|
||||
crawler.engine.download_async.side_effect = return_response
|
||||
return crawler
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
|
|
@ -102,14 +101,12 @@ Disallow: /some/randome/page.html
|
|||
"http://site.local/robots.txt", body=b"GIF89a\xd3\x00\xfe\x00\xa2"
|
||||
)
|
||||
|
||||
def return_response(request):
|
||||
from twisted.internet import reactor
|
||||
|
||||
async def return_response(request):
|
||||
deferred = Deferred()
|
||||
reactor.callFromThread(deferred.callback, response)
|
||||
return deferred
|
||||
call_later(0, deferred.callback, response)
|
||||
return await maybe_deferred_to_future(deferred)
|
||||
|
||||
crawler.engine.download.side_effect = return_response
|
||||
crawler.engine.download_async.side_effect = return_response
|
||||
return crawler
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
|
|
@ -126,14 +123,12 @@ Disallow: /some/randome/page.html
|
|||
crawler.settings.set("ROBOTSTXT_OBEY", True)
|
||||
response = Response("http://site.local/robots.txt")
|
||||
|
||||
def return_response(request):
|
||||
from twisted.internet import reactor
|
||||
|
||||
async def return_response(request):
|
||||
deferred = Deferred()
|
||||
reactor.callFromThread(deferred.callback, response)
|
||||
return deferred
|
||||
call_later(0, deferred.callback, response)
|
||||
return await maybe_deferred_to_future(deferred)
|
||||
|
||||
crawler.engine.download.side_effect = return_response
|
||||
crawler.engine.download_async.side_effect = return_response
|
||||
return crawler
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
|
|
@ -149,14 +144,12 @@ Disallow: /some/randome/page.html
|
|||
self.crawler.settings.set("ROBOTSTXT_OBEY", True)
|
||||
err = error.DNSLookupError("Robotstxt address not found")
|
||||
|
||||
def return_failure(request):
|
||||
from twisted.internet import reactor
|
||||
|
||||
async def return_failure(request):
|
||||
deferred = Deferred()
|
||||
reactor.callFromThread(deferred.errback, failure.Failure(err))
|
||||
return deferred
|
||||
call_later(0, deferred.errback, failure.Failure(err))
|
||||
return await maybe_deferred_to_future(deferred)
|
||||
|
||||
self.crawler.engine.download.side_effect = return_failure
|
||||
self.crawler.engine.download_async.side_effect = return_failure
|
||||
|
||||
middleware = RobotsTxtMiddleware(self.crawler)
|
||||
middleware._logerror = mock.MagicMock(side_effect=middleware._logerror)
|
||||
|
|
@ -170,12 +163,10 @@ Disallow: /some/randome/page.html
|
|||
self.crawler.settings.set("ROBOTSTXT_OBEY", True)
|
||||
err = error.DNSLookupError("Robotstxt address not found")
|
||||
|
||||
def immediate_failure(request):
|
||||
deferred = Deferred()
|
||||
deferred.errback(failure.Failure(err))
|
||||
return deferred
|
||||
async def immediate_failure(request):
|
||||
raise err
|
||||
|
||||
self.crawler.engine.download.side_effect = immediate_failure
|
||||
self.crawler.engine.download_async.side_effect = immediate_failure
|
||||
|
||||
middleware = RobotsTxtMiddleware(self.crawler)
|
||||
await self.assertNotIgnored(Request("http://site.local"), middleware)
|
||||
|
|
@ -184,14 +175,12 @@ Disallow: /some/randome/page.html
|
|||
async def test_ignore_robotstxt_request(self):
|
||||
self.crawler.settings.set("ROBOTSTXT_OBEY", True)
|
||||
|
||||
def ignore_request(request):
|
||||
from twisted.internet import reactor
|
||||
|
||||
async def ignore_request(request):
|
||||
deferred = Deferred()
|
||||
reactor.callFromThread(deferred.errback, failure.Failure(IgnoreRequest()))
|
||||
return deferred
|
||||
call_later(0, deferred.errback, failure.Failure(IgnoreRequest()))
|
||||
return await maybe_deferred_to_future(deferred)
|
||||
|
||||
self.crawler.engine.download.side_effect = ignore_request
|
||||
self.crawler.engine.download_async.side_effect = ignore_request
|
||||
|
||||
middleware = RobotsTxtMiddleware(self.crawler)
|
||||
mw_module_logger.error = mock.MagicMock()
|
||||
|
|
@ -240,7 +229,7 @@ Disallow: /some/randome/page.html
|
|||
)
|
||||
|
||||
def assertRobotsTxtRequested(self, base_url: str) -> None:
|
||||
calls = self.crawler.engine.download.call_args_list
|
||||
calls = self.crawler.engine.download_async.call_args_list
|
||||
request = calls[0][0][0]
|
||||
assert request.url == f"{base_url}/robots.txt"
|
||||
assert request.callback == NO_CALLBACK
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ import sys
|
|||
from collections import defaultdict
|
||||
from dataclasses import dataclass
|
||||
from logging import DEBUG
|
||||
from unittest.mock import Mock
|
||||
from unittest.mock import Mock, call
|
||||
from urllib.parse import urlparse
|
||||
|
||||
import attr
|
||||
|
|
@ -31,7 +31,7 @@ from scrapy import signals
|
|||
from scrapy.core.engine import ExecutionEngine, _Slot
|
||||
from scrapy.core.scheduler import BaseScheduler
|
||||
from scrapy.exceptions import CloseSpider, IgnoreRequest
|
||||
from scrapy.http import Request
|
||||
from scrapy.http import Request, Response
|
||||
from scrapy.item import Field, Item
|
||||
from scrapy.linkextractors import LinkExtractor
|
||||
from scrapy.signals import request_scheduled
|
||||
|
|
@ -489,6 +489,99 @@ class TestEngine(TestEngineBase):
|
|||
assert "AssertionError" not in stderr_str, stderr_str
|
||||
|
||||
|
||||
class TestEngineDownloadAsync:
|
||||
"""Test cases for ExecutionEngine.download_async()."""
|
||||
|
||||
@pytest.fixture
|
||||
def engine(self) -> ExecutionEngine:
|
||||
crawler = get_crawler(MySpider)
|
||||
engine = ExecutionEngine(crawler, lambda _: None)
|
||||
engine.downloader.close()
|
||||
engine.downloader = Mock()
|
||||
engine._slot = Mock()
|
||||
engine._slot.inprogress = set()
|
||||
return engine
|
||||
|
||||
@staticmethod
|
||||
async def _download(engine: ExecutionEngine, request: Request) -> Response:
|
||||
return await engine.download_async(request)
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
async def test_download_async_success(self, engine):
|
||||
"""Test basic successful async download of a request."""
|
||||
request = Request("http://example.com")
|
||||
response = Response("http://example.com", body=b"test body")
|
||||
engine.spider = Mock()
|
||||
engine.downloader.fetch.return_value = defer.succeed(response)
|
||||
engine._slot.add_request = Mock()
|
||||
engine._slot.remove_request = Mock()
|
||||
|
||||
result = await self._download(engine, request)
|
||||
assert result == response
|
||||
engine._slot.add_request.assert_called_once_with(request)
|
||||
engine._slot.remove_request.assert_called_once_with(request)
|
||||
engine.downloader.fetch.assert_called_once_with(request, engine.spider)
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
async def test_download_async_redirect(self, engine):
|
||||
"""Test async download with a redirect request."""
|
||||
# Arrange
|
||||
original_request = Request("http://example.com")
|
||||
redirect_request = Request("http://example.com/redirect")
|
||||
final_response = Response("http://example.com/redirect", body=b"redirected")
|
||||
|
||||
# First call returns redirect request, second call returns final response
|
||||
engine.downloader.fetch.side_effect = [
|
||||
defer.succeed(redirect_request),
|
||||
defer.succeed(final_response),
|
||||
]
|
||||
engine.spider = Mock()
|
||||
engine._slot.add_request = Mock()
|
||||
engine._slot.remove_request = Mock()
|
||||
|
||||
result = await self._download(engine, original_request)
|
||||
assert result == final_response
|
||||
assert engine.downloader.fetch.call_count == 2
|
||||
engine._slot.add_request.assert_has_calls(
|
||||
[call(original_request), call(redirect_request)]
|
||||
)
|
||||
engine._slot.remove_request.assert_has_calls(
|
||||
[call(original_request), call(redirect_request)]
|
||||
)
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
async def test_download_async_no_spider(self, engine):
|
||||
"""Test async download attempt when no spider is available."""
|
||||
request = Request("http://example.com")
|
||||
engine.spider = None
|
||||
with pytest.raises(RuntimeError, match="No open spider to crawl:"):
|
||||
await self._download(engine, request)
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
async def test_download_async_failure(self, engine):
|
||||
"""Test async download when the downloader raises an exception."""
|
||||
request = Request("http://example.com")
|
||||
error = RuntimeError("Download failed")
|
||||
engine.spider = Mock()
|
||||
engine.downloader.fetch.return_value = defer.fail(error)
|
||||
engine._slot.add_request = Mock()
|
||||
engine._slot.remove_request = Mock()
|
||||
|
||||
with pytest.raises(RuntimeError, match="Download failed"):
|
||||
await self._download(engine, request)
|
||||
engine._slot.add_request.assert_called_once_with(request)
|
||||
engine._slot.remove_request.assert_called_once_with(request)
|
||||
|
||||
|
||||
@pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning")
|
||||
class TestEngineDownload(TestEngineDownloadAsync):
|
||||
"""Test cases for ExecutionEngine.download()."""
|
||||
|
||||
@staticmethod
|
||||
async def _download(engine: ExecutionEngine, request: Request) -> Response:
|
||||
return await maybe_deferred_to_future(engine.download(request))
|
||||
|
||||
|
||||
def test_request_scheduled_signal(caplog):
|
||||
class TestScheduler(BaseScheduler):
|
||||
def __init__(self):
|
||||
|
|
|
|||
Loading…
Reference in New Issue