diff --git a/docs/topics/coroutines.rst b/docs/topics/coroutines.rst index 2c0df5e0f..b9d780528 100644 --- a/docs/topics/coroutines.rst +++ b/docs/topics/coroutines.rst @@ -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(), diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index dc27ce6ca..13a5b88f7 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -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. diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 970a250ef..669005e32 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -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 diff --git a/scrapy/downloadermiddlewares/robotstxt.py b/scrapy/downloadermiddlewares/robotstxt.py index fbd737970..48e4875a7 100644 --- a/scrapy/downloadermiddlewares/robotstxt.py +++ b/scrapy/downloadermiddlewares/robotstxt.py @@ -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) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 04e1d14fa..a50150c59 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -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) diff --git a/tests/test_downloadermiddleware_robotstxt.py b/tests/test_downloadermiddleware_robotstxt.py index 12e43800b..2bec56e41 100644 --- a/tests/test_downloadermiddleware_robotstxt.py +++ b/tests/test_downloadermiddleware_robotstxt.py @@ -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 diff --git a/tests/test_engine.py b/tests/test_engine.py index 9590859bb..14c8f8fee 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -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):