mirror of https://github.com/scrapy/scrapy.git
Add `DownloaderMiddlewareManager.download_async()`, deprecate returning Deferreds from downloader mws (#7069)
* Restore coverage in MiddlewareManager spider arg handling. * Move MiddlewareManager._warn_spider_arg() tests. * Add DownloaderMiddlewareManager.download_async(). * Make download_func async. * Inline MiddlewareManager._warn_spider_arg(). * Deprecate returning deferreds from downloader middlewares. * Update the downloader mw doc about async def. * Use global_object_name() instead of repr(). * Update docs/topics/downloader-middleware.rst Co-authored-by: Adrián Chaves <adrian@chaves.io> * Update downloader-middleware.rst --------- Co-authored-by: Adrián Chaves <adrian@chaves.io>
This commit is contained in:
parent
7ed20ee7f3
commit
5105f55a98
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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]:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue