diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 6f36acc68..6d24482ec 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -68,7 +68,8 @@ defines one or more of these methods: .. class:: DownloaderMiddleware - .. note:: Any of the downloader middleware methods may also return a deferred. + .. note:: Any of the downloader middleware methods may be defined as a + coroutine function (``async def``). .. method:: process_request(request) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 0d23aa85c..109a6eaec 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -23,6 +23,7 @@ from scrapy.utils.decorators import _warn_spider_arg from scrapy.utils.defer import ( _defer_sleep_async, _schedule_coro, + deferred_from_coro, maybe_deferred_to_future, ) from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute @@ -139,7 +140,11 @@ class Downloader: ) -> Generator[Deferred[Any], Any, Response | Request]: self.active.add(request) try: - return (yield self.middleware.download(self._enqueue_request, request)) + return ( + yield deferred_from_coro( + self.middleware.download_async(self._enqueue_request, request) + ) + ) finally: self.active.remove(request) @@ -178,10 +183,7 @@ class Downloader: return key # passed as download_func into self.middleware.download() in self.fetch() - @inlineCallbacks - def _enqueue_request( - self, request: Request - ) -> Generator[Deferred[Any], Any, Response]: + async def _enqueue_request(self, request: Request) -> Response: key, slot = self._get_slot(request) request.meta[self.DOWNLOAD_SLOT] = key slot.active.add(request) @@ -194,7 +196,7 @@ class Downloader: slot.queue.append((request, d)) self._process_queue(slot) try: - return (yield d) # fired in _wait_for_download() + return await maybe_deferred_to_future(d) # fired in _wait_for_download() finally: slot.active.remove(request) diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index a822ba811..431579856 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -7,19 +7,25 @@ See documentation in docs/topics/downloader-middleware.rst from __future__ import annotations import warnings +from functools import wraps from typing import TYPE_CHECKING, Any, cast -from twisted.internet.defer import Deferred, inlineCallbacks - from scrapy.exceptions import ScrapyDeprecationWarning, _InvalidOutput from scrapy.http import Request, Response from scrapy.middleware import MiddlewareManager from scrapy.utils.conf import build_component_list -from scrapy.utils.defer import _defer_sleep, deferred_from_coro -from scrapy.utils.deprecate import argument_is_required +from scrapy.utils.defer import ( + _defer_sleep_async, + deferred_from_coro, + ensure_awaitable, + maybe_deferred_to_future, +) +from scrapy.utils.python import global_object_name if TYPE_CHECKING: - from collections.abc import Callable, Generator + from collections.abc import Callable, Coroutine + + from twisted.internet.defer import Deferred from scrapy import Spider from scrapy.settings import BaseSettings @@ -43,36 +49,42 @@ class DownloaderMiddlewareManager(MiddlewareManager): self.methods["process_exception"].appendleft(mw.process_exception) self._check_mw_method_spider_arg(mw.process_exception) - @inlineCallbacks def download( self, - download_func: Callable[[Request], Deferred[Response]], + download_func: Callable[[Request, Spider], Deferred[Response]], request: Request, - spider: Spider | None = None, - ) -> Generator[Deferred[Any], Any, Response | Request]: - if argument_is_required(download_func, "spider"): - warnings.warn( - "The spider argument of download_func is deprecated" - " and will not be passed in future Scrapy versions.", - ScrapyDeprecationWarning, - stacklevel=2, - ) - need_spider_arg = True - else: - need_spider_arg = False + spider: Spider, + ) -> Deferred[Response | Request]: + warnings.warn( + "DownloaderMiddlewareManager.download() is deprecated, use download_async() instead", + ScrapyDeprecationWarning, + stacklevel=2, + ) - @inlineCallbacks - def process_request( - request: Request, - ) -> Generator[Deferred[Any], Any, Response | Request]: + @wraps(download_func) + async def download_func_wrapped(request: Request) -> Response: + return await maybe_deferred_to_future(download_func(request, spider)) + + self._set_compat_spider(spider) + return deferred_from_coro(self.download_async(download_func_wrapped, request)) + + async def download_async( + self, + download_func: Callable[[Request], Coroutine[Any, Any, Response]], + request: Request, + ) -> Response | Request: + async def process_request(request: Request) -> Response | Request: for method in self.methods["process_request"]: method = cast("Callable", method) if method in self._mw_methods_requiring_spider: - response = yield deferred_from_coro( - method(request=request, spider=self._spider) + response = await ensure_awaitable( + method(request=request, spider=self._spider), + _warn=global_object_name(method), ) else: - response = yield deferred_from_coro(method(request=request)) + response = await ensure_awaitable( + method(request=request), _warn=global_object_name(method) + ) if response is not None and not isinstance( response, (Response, Request) ): @@ -82,14 +94,9 @@ class DownloaderMiddlewareManager(MiddlewareManager): ) if response: return response - if need_spider_arg: - return (yield download_func(request, self._spider)) # type: ignore[call-arg] - return (yield download_func(request)) + return await download_func(request) - @inlineCallbacks - def process_response( - response: Response | Request, - ) -> Generator[Deferred[Any], Any, Response | Request]: + async def process_response(response: Response | Request) -> Response | Request: if response is None: raise TypeError("Received None in process_response") if isinstance(response, Request): @@ -98,12 +105,14 @@ class DownloaderMiddlewareManager(MiddlewareManager): for method in self.methods["process_response"]: method = cast("Callable", method) if method in self._mw_methods_requiring_spider: - response = yield deferred_from_coro( - method(request=request, response=response, spider=self._spider) + response = await ensure_awaitable( + method(request=request, response=response, spider=self._spider), + _warn=global_object_name(method), ) else: - response = yield deferred_from_coro( - method(request=request, response=response) + response = await ensure_awaitable( + method(request=request, response=response), + _warn=global_object_name(method), ) if not isinstance(response, (Response, Request)): raise _InvalidOutput( @@ -114,21 +123,20 @@ class DownloaderMiddlewareManager(MiddlewareManager): return response return response - @inlineCallbacks - def process_exception( - exception: Exception, - ) -> Generator[Deferred[Any], Any, Response | Request]: + async def process_exception(exception: Exception) -> Response | Request: for method in self.methods["process_exception"]: method = cast("Callable", method) if method in self._mw_methods_requiring_spider: - response = yield deferred_from_coro( + response = await ensure_awaitable( method( request=request, exception=exception, spider=self._spider - ) + ), + _warn=global_object_name(method), ) else: - response = yield deferred_from_coro( - method(request=request, exception=exception) + response = await ensure_awaitable( + method(request=request, exception=exception), + _warn=global_object_name(method), ) if response is not None and not isinstance( response, (Response, Request) @@ -141,14 +149,11 @@ class DownloaderMiddlewareManager(MiddlewareManager): return response raise exception - if spider: - self._warn_spider_arg("download") - self._set_compat_spider(spider) try: - result: Response | Request = yield process_request(request) + result: Response | Request = await process_request(request) except Exception as ex: - yield _defer_sleep() + await _defer_sleep_async() # either returns a request or response (which we pass to process_response()) # or reraises the exception - result = yield process_exception(ex) - return (yield process_response(result)) + result = await process_exception(ex) + return await process_response(result) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 6f694753a..61fecfc60 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -436,7 +436,17 @@ class SpiderMiddlewareManager(MiddlewareManager): self, spider: Spider | None = None ) -> AsyncIterator[Any] | None: if spider: - self._warn_spider_arg("process_start") + if self.crawler: + msg = ( + "Passing a spider argument to SpiderMiddlewareManager.process_start() is deprecated" + " and the passed value is ignored." + ) + else: + msg = ( + "Passing a spider argument to SpiderMiddlewareManager.process_start() is deprecated," + " SpiderMiddlewareManager should be instantiated with a Crawler instance instead." + ) + warn(msg, category=ScrapyDeprecationWarning, stacklevel=2) self._set_compat_spider(spider) self._check_deprecated_start_requests_use() if self._use_start_requests: diff --git a/scrapy/middleware.py b/scrapy/middleware.py index 21a2ebfa3..83362784b 100644 --- a/scrapy/middleware.py +++ b/scrapy/middleware.py @@ -82,19 +82,6 @@ class MiddlewareManager(ABC): f" {self._compat_spider} and {spider}" ) - def _warn_spider_arg(self, method_name: str) -> None: - if self.crawler: - msg = ( - f"Passing a spider argument to {type(self).__name__}.{method_name}() is deprecated" - " and the passed value is ignored." - ) - else: - msg = ( - f"Passing a spider argument to {type(self).__name__}.{method_name}() is deprecated," - f" {type(self).__name__} should be instantiated with a Crawler instance instead." - ) - warnings.warn(msg, category=ScrapyDeprecationWarning, stacklevel=3) - @classmethod @abstractmethod def _get_mwlist_from_settings(cls, settings: Settings) -> list[Any]: diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 8ca3bc0bd..c27f8bb34 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -517,14 +517,14 @@ def _schedule_coro(coro: Coroutine[Any, Any, Any]) -> None: @overload -def ensure_awaitable(o: Awaitable[_T]) -> Awaitable[_T]: ... +def ensure_awaitable(o: Awaitable[_T], _warn: str | None = None) -> Awaitable[_T]: ... @overload -def ensure_awaitable(o: _T) -> Awaitable[_T]: ... +def ensure_awaitable(o: _T, _warn: str | None = None) -> Awaitable[_T]: ... -def ensure_awaitable(o: _T | Awaitable[_T]) -> Awaitable[_T]: +def ensure_awaitable(o: _T | Awaitable[_T], _warn: str | None = None) -> Awaitable[_T]: """Convert any value to an awaitable object. For a :class:`~twisted.internet.defer.Deferred` object, use @@ -535,6 +535,13 @@ def ensure_awaitable(o: _T | Awaitable[_T]) -> Awaitable[_T]: .. versionadded:: VERSION """ if isinstance(o, Deferred): + if _warn: + warnings.warn( + f"{_warn} returned a Deferred, this is deprecated." + f" Please refactor this function to return a coroutine.", + ScrapyDeprecationWarning, + stacklevel=2, + ) return maybe_deferred_to_future(o) if inspect.isawaitable(o): return o diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index 03b941e4f..bc1748dfb 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -50,10 +50,10 @@ class TestManagerBase: if not response: response = Response(request.url) - def download_func(request: Request) -> Deferred[Response]: - return succeed(response) + async def download_func(request: Request) -> Response: + return response - return await maybe_deferred_to_future(mwman.download(download_func, request)) + return await mwman.download_async(download_func, request) class TestDefaults(TestManagerBase): @@ -135,7 +135,7 @@ class TestResponseFromProcessRequest(TestManagerBase): async with self.get_mwman() as mwman: mwman._add_middleware(ResponseMiddleware()) - result = await maybe_deferred_to_future(mwman.download(download_func, req)) + result = await mwman.download_async(download_func, req) assert result is resp assert not download_func.called @@ -163,7 +163,7 @@ class TestResponseFromProcessException(TestManagerBase): async with self.get_mwman() as mwman: mwman._add_middleware(ResponseMiddleware()) - result = await maybe_deferred_to_future(mwman.download(download_func, req)) + result = await mwman.download_async(download_func, req) assert result is resp assert calls == [ "process_exception", @@ -219,7 +219,7 @@ class TestInvalidOutput(TestManagerBase): class TestMiddlewareUsingDeferreds(TestManagerBase): - """Middlewares using Deferreds should work""" + """Middlewares using Deferreds (deprecated) should work""" @deferred_f_from_coro_f async def test_deferred(self): @@ -239,7 +239,11 @@ class TestMiddlewareUsingDeferreds(TestManagerBase): async with self.get_mwman() as mwman: mwman._add_middleware(DeferredMiddleware()) - result = await maybe_deferred_to_future(mwman.download(download_func, req)) + with pytest.warns( + ScrapyDeprecationWarning, + match="returned a Deferred, this is deprecated", + ): + result = await mwman.download_async(download_func, req) assert result is resp assert not download_func.called @@ -260,7 +264,7 @@ class TestMiddlewareUsingCoro(TestManagerBase): async with self.get_mwman() as mwman: mwman._add_middleware(CoroMiddleware()) - result = await maybe_deferred_to_future(mwman.download(download_func, req)) + result = await mwman.download_async(download_func, req) assert result is resp assert not download_func.called @@ -278,14 +282,14 @@ class TestMiddlewareUsingCoro(TestManagerBase): async with self.get_mwman() as mwman: mwman._add_middleware(CoroMiddleware()) - result = await maybe_deferred_to_future(mwman.download(download_func, req)) + result = await mwman.download_async(download_func, req) assert result is resp assert not download_func.called class TestDownloadDeprecated(TestManagerBase): @deferred_f_from_coro_f - async def test_download_func_spider_arg(self): + async def test_mwman_download(self): req = Request("http://example.com/index.html") resp = Response(req.url, status=200) @@ -295,24 +299,7 @@ class TestDownloadDeprecated(TestManagerBase): async with self.get_mwman() as mwman: with pytest.warns( ScrapyDeprecationWarning, - match="The spider argument of download_func is deprecated", - ): - ret = await maybe_deferred_to_future(mwman.download(download_func, req)) - assert isinstance(ret, Response) - - @deferred_f_from_coro_f - async def test_mwman_download_spider_arg(self): - req = Request("http://example.com/index.html") - resp = Response(req.url, status=200) - - def download_func(request: Request) -> Deferred[Response]: - return succeed(resp) - - async with self.get_mwman() as mwman: - with pytest.warns( - ScrapyDeprecationWarning, - match=r"Passing a spider argument to DownloaderMiddlewareManager.download\(\)" - r" is deprecated and the passed value is ignored.", + match=r"DownloaderMiddlewareManager.download\(\) is deprecated, use download_async\(\) instead", ): ret = await maybe_deferred_to_future( mwman.download(download_func, req, mwman.crawler.spider) @@ -353,6 +340,6 @@ class TestDeprecatedSpiderArg(TestManagerBase): ), ): mwman._add_middleware(DeprecatedSpiderArgMiddleware()) - result = await maybe_deferred_to_future(mwman.download(download_func, req)) + result = await mwman.download_async(download_func, req) assert result is resp assert not download_func.called