From 11073c86802d82a3e9d16400ae8f55289fdb9ef4 Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Sun, 30 Nov 2025 17:26:17 -0300 Subject: [PATCH] Deprecate spider attributes that can be replaced by settings, round 2 (#7039) * Move duplicate code to utils * move new function to end * draft * update docs * Update tests * Update test name * rename test * rollback signature * leftover * sort * Rollback some changes * Rollback download_delay warning * Rollback * fix checks * Remove unused imports * Add pragma: no cover. --------- Co-authored-by: Andrey Rakhmatullin --- scrapy/core/downloader/__init__.py | 13 ++---- scrapy/core/downloader/handlers/http11.py | 8 ++++ scrapy/core/http2/protocol.py | 8 ++++ .../downloadermiddlewares/downloadtimeout.py | 3 ++ .../downloadermiddlewares/httpcompression.py | 19 +++----- scrapy/downloadermiddlewares/useragent.py | 4 ++ scrapy/utils/deprecate.py | 10 +++++ .../test_downloader_handler_twisted_http2.py | 12 ++--- tests/test_downloader_handlers_http_base.py | 45 +++++++++---------- ...st_downloadermiddleware_downloadtimeout.py | 8 ++-- tests/test_downloadermiddleware_useragent.py | 19 -------- 11 files changed, 74 insertions(+), 75 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 77b17287d..0d23aa85c 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -1,7 +1,6 @@ from __future__ import annotations import random -import warnings from collections import deque from datetime import datetime from time import time @@ -13,7 +12,6 @@ from twisted.python.failure import Failure from scrapy import Request, Spider, signals from scrapy.core.downloader.handlers import DownloadHandlers from scrapy.core.downloader.middleware import DownloaderMiddlewareManager -from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.resolver import dnscache from scrapy.utils.asyncio import ( AsyncioLoopingCall, @@ -27,6 +25,7 @@ from scrapy.utils.defer import ( _schedule_coro, maybe_deferred_to_future, ) +from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute from scrapy.utils.httpobj import urlparse_cached if TYPE_CHECKING: @@ -97,13 +96,9 @@ def _get_concurrency_delay( if hasattr(spider, "download_delay"): delay = spider.download_delay - if hasattr(spider, "max_concurrent_requests"): - warnings.warn( - "The 'max_concurrent_requests' spider attribute is deprecated. " - "Use Spider.custom_settings or Spider.update_settings() instead. " - "The corresponding setting name is 'CONCURRENT_REQUESTS'.", - category=ScrapyDeprecationWarning, - stacklevel=2, + if hasattr(spider, "max_concurrent_requests"): # pragma: no cover + warn_on_deprecated_spider_attribute( + "max_concurrent_requests", "CONCURRENT_REQUESTS" ) concurrency = spider.max_concurrent_requests diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index d8965c130..02aaf7c5c 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -35,6 +35,7 @@ from scrapy.core.downloader.contextfactory import load_context_factory_from_sett from scrapy.exceptions import StopDownload from scrapy.http import Headers, Response from scrapy.responsetypes import responsetypes +from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.python import to_bytes, to_unicode from scrapy.utils.url import add_http_if_no_scheme @@ -92,6 +93,13 @@ class HTTP11DownloadHandler: def download_request(self, request: Request, spider: Spider) -> Deferred[Response]: """Return a deferred for the HTTP download""" + if hasattr(spider, "download_maxsize"): # pragma: no cover + warn_on_deprecated_spider_attribute("download_maxsize", "DOWNLOAD_MAXSIZE") + if hasattr(spider, "download_warnsize"): # pragma: no cover + warn_on_deprecated_spider_attribute( + "download_warnsize", "DOWNLOAD_WARNSIZE" + ) + agent = ScrapyAgent( contextFactory=self._contextFactory, pool=self._pool, diff --git a/scrapy/core/http2/protocol.py b/scrapy/core/http2/protocol.py index cf2742de6..ee9211efc 100644 --- a/scrapy/core/http2/protocol.py +++ b/scrapy/core/http2/protocol.py @@ -34,6 +34,7 @@ from zope.interface import implementer from scrapy.core.http2.stream import Stream, StreamCloseReason from scrapy.http import Request, Response +from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute if TYPE_CHECKING: from ipaddress import IPv4Address, IPv6Address @@ -191,6 +192,13 @@ class H2ClientProtocol(Protocol, TimeoutMixin): def _new_stream(self, request: Request, spider: Spider) -> Stream: """Instantiates a new Stream object""" + if hasattr(spider, "download_maxsize"): # pragma: no cover + warn_on_deprecated_spider_attribute("download_maxsize", "DOWNLOAD_MAXSIZE") + if hasattr(spider, "download_warnsize"): # pragma: no cover + warn_on_deprecated_spider_attribute( + "download_warnsize", "DOWNLOAD_WARNSIZE" + ) + stream = Stream( stream_id=next(self._stream_id_generator), request=request, diff --git a/scrapy/downloadermiddlewares/downloadtimeout.py b/scrapy/downloadermiddlewares/downloadtimeout.py index b57d5c2a9..bccfb230c 100644 --- a/scrapy/downloadermiddlewares/downloadtimeout.py +++ b/scrapy/downloadermiddlewares/downloadtimeout.py @@ -10,6 +10,7 @@ from typing import TYPE_CHECKING from scrapy import Request, Spider, signals from scrapy.utils.decorators import _warn_spider_arg +from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute if TYPE_CHECKING: # typing.Self requires Python 3.11 @@ -30,6 +31,8 @@ class DownloadTimeoutMiddleware: return o def spider_opened(self, spider: Spider) -> None: + if hasattr(spider, "download_timeout"): # pragma: no cover + warn_on_deprecated_spider_attribute("download_timeout", "DOWNLOAD_TIMEOUT") self._timeout = getattr(spider, "download_timeout", self._timeout) @_warn_spider_arg diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index e81888d9b..d4fa2d4d7 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -6,7 +6,7 @@ from logging import getLogger from typing import TYPE_CHECKING, Any from scrapy import Request, Spider, signals -from scrapy.exceptions import IgnoreRequest, NotConfigured, ScrapyDeprecationWarning +from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes from scrapy.utils._compression import ( @@ -16,6 +16,7 @@ from scrapy.utils._compression import ( _unzstd, ) from scrapy.utils.decorators import _warn_spider_arg +from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute from scrapy.utils.gz import gunzip if TYPE_CHECKING: @@ -85,21 +86,11 @@ class HttpCompressionMiddleware: def open_spider(self, spider: Spider) -> None: if hasattr(spider, "download_maxsize"): - warnings.warn( - "The 'download_maxsize' spider attribute is deprecated. " - "Use Spider.custom_settings or Spider.update_settings() instead. " - "The corresponding setting name is 'DOWNLOAD_MAXSIZE'.", - category=ScrapyDeprecationWarning, - stacklevel=2, - ) + warn_on_deprecated_spider_attribute("download_maxsize", "DOWNLOAD_MAXSIZE") self._max_size = spider.download_maxsize if hasattr(spider, "download_warnsize"): - warnings.warn( - "The 'download_warnsize' spider attribute is deprecated. " - "Use Spider.custom_settings or Spider.update_settings() instead. " - "The corresponding setting name is 'DOWNLOAD_WARNSIZE'.", - category=ScrapyDeprecationWarning, - stacklevel=2, + warn_on_deprecated_spider_attribute( + "download_warnsize", "DOWNLOAD_WARNSIZE" ) self._warn_size = spider.download_warnsize diff --git a/scrapy/downloadermiddlewares/useragent.py b/scrapy/downloadermiddlewares/useragent.py index c43a0195c..61f84b518 100644 --- a/scrapy/downloadermiddlewares/useragent.py +++ b/scrapy/downloadermiddlewares/useragent.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING from scrapy import Request, Spider, signals from scrapy.utils.decorators import _warn_spider_arg +from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute if TYPE_CHECKING: # typing.Self requires Python 3.11 @@ -28,6 +29,9 @@ class UserAgentMiddleware: return o def spider_opened(self, spider: Spider) -> None: + if hasattr(spider, "user_agent"): # pragma: no cover + warn_on_deprecated_spider_attribute("user_agent", "USER_AGENT") + self.user_agent = getattr(spider, "user_agent", self.user_agent) @_warn_spider_arg diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 1f529b2cb..e6e6becd6 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -219,3 +219,13 @@ def argument_is_required(func: Callable[..., Any], arg_name: str) -> bool: args = get_func_args_dict(func) param = args.get(arg_name) return param is not None and param.default is inspect.Parameter.empty + + +def warn_on_deprecated_spider_attribute(attribute_name: str, setting_name: str) -> None: + warnings.warn( + f"The '{attribute_name}' spider attribute is deprecated. " + "Use Spider.custom_settings or Spider.update_settings() instead. " + f"The corresponding setting name is '{setting_name}'.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 3e3e677a2..1e638392f 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -15,6 +15,8 @@ from twisted.web.http import H2_ENABLED from scrapy.http import Request from scrapy.spiders import Spider from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future +from scrapy.utils.misc import build_from_crawler +from scrapy.utils.test import get_crawler from tests.test_downloader_handlers_http_base import ( TestHttpProxyBase, TestHttps11Base, @@ -31,7 +33,6 @@ if TYPE_CHECKING: from tests.mockserver.http import MockServer from tests.mockserver.proxy_echo import ProxyEchoMockServer - pytestmark = pytest.mark.skipif( not H2_ENABLED, reason="HTTP/2 support in Twisted is not enabled" ) @@ -63,10 +64,13 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): @deferred_f_from_coro_f async def test_download_with_maxsize_very_large_file( - self, mockserver: MockServer, download_handler: DownloadHandlerProtocol + self, mockserver: MockServer ) -> None: from twisted.internet import reactor + crawler = get_crawler(settings_dict={"DOWNLOAD_MAXSIZE": 1_500}) + download_handler = build_from_crawler(self.download_handler_cls, crawler) + with mock.patch("scrapy.core.http2.stream.logger") as logger: request = Request( mockserver.url("/largechunkedfile", is_secure=self.is_secure) @@ -76,9 +80,7 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): logger.error.assert_called_once_with(mock.ANY) with pytest.raises((defer.CancelledError, error.ConnectionAborted)): - await download_request( - download_handler, request, Spider("foo", download_maxsize=1500) - ) + await download_request(download_handler, request, Spider("foo")) # As the error message is logged in the dataReceived callback, we # have to give a bit of time to the reactor to process the queue diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index 8fefc0dd7..a2459911f 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -453,28 +453,29 @@ class TestHttp11Base(TestHttpBase): assert type(response) is TextResponse # pylint: disable=unidiomatic-typecheck @deferred_f_from_coro_f - async def test_download_with_maxsize( - self, mockserver: MockServer, download_handler: DownloadHandlerProtocol - ) -> None: + async def test_download_with_maxsize(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/text", is_secure=self.is_secure)) # 10 is minimal size for this request and the limit is only counted on # response body. (regardless of headers) - response = await download_request( - download_handler, request, Spider("foo", download_maxsize=5) - ) + crawler = get_crawler(settings_dict={"DOWNLOAD_MAXSIZE": 5}) + download_handler = build_from_crawler(self.download_handler_cls, crawler) + response = await download_request(download_handler, request, Spider("foo")) assert response.body == b"Works" + crawler = get_crawler(settings_dict={"DOWNLOAD_MAXSIZE": 4}) + download_handler = build_from_crawler(self.download_handler_cls, crawler) + with pytest.raises((defer.CancelledError, error.ConnectionAborted)): - await download_request( - download_handler, request, Spider("foo", download_maxsize=4) - ) + await download_request(download_handler, request, Spider("foo")) @deferred_f_from_coro_f async def test_download_with_maxsize_very_large_file( - self, mockserver: MockServer, download_handler: DownloadHandlerProtocol + self, mockserver: MockServer ) -> None: # TODO: the logger check is specific to scrapy.core.downloader.handlers.http11 + crawler = get_crawler(settings_dict={"DOWNLOAD_MAXSIZE": 1_500}) + download_handler = build_from_crawler(self.download_handler_cls, crawler) with mock.patch("scrapy.core.downloader.handlers.http11.logger") as logger: request = Request( mockserver.url("/largechunkedfile", is_secure=self.is_secure) @@ -484,9 +485,7 @@ class TestHttp11Base(TestHttpBase): logger.warning.assert_called_once_with(mock.ANY, mock.ANY) with pytest.raises((defer.CancelledError, error.ConnectionAborted)): - await download_request( - download_handler, request, Spider("foo", download_maxsize=1500) - ) + await download_request(download_handler, request, Spider("foo")) # As the error message is logged in the dataReceived callback, we # have to give a bit of time to the reactor to process the queue @@ -506,23 +505,23 @@ class TestHttp11Base(TestHttpBase): await download_request(download_handler, request) @deferred_f_from_coro_f - async def test_download_with_small_maxsize_per_spider( - self, mockserver: MockServer, download_handler: DownloadHandlerProtocol + async def test_download_with_small_maxsize_via_setting( + self, mockserver: MockServer ) -> None: + crawler = get_crawler(settings_dict={"DOWNLOAD_MAXSIZE": 2}) + download_handler = build_from_crawler(self.download_handler_cls, crawler) request = Request(mockserver.url("/text", is_secure=self.is_secure)) with pytest.raises((defer.CancelledError, error.ConnectionAborted)): - await download_request( - download_handler, request, Spider("foo", download_maxsize=2) - ) + await download_request(download_handler, request, Spider("foo")) @deferred_f_from_coro_f - async def test_download_with_large_maxsize_per_spider( - self, mockserver: MockServer, download_handler: DownloadHandlerProtocol + async def test_download_with_large_maxsize_via_setting( + self, mockserver: MockServer ) -> None: + crawler = get_crawler(settings_dict={"DOWNLOAD_MAXSIZE": 5}) + download_handler = build_from_crawler(self.download_handler_cls, crawler) request = Request(mockserver.url("/text", is_secure=self.is_secure)) - response = await download_request( - download_handler, request, Spider("foo", download_maxsize=100) - ) + response = await download_request(download_handler, request, Spider("foo")) assert response.body == b"Works" @deferred_f_from_coro_f diff --git a/tests/test_downloadermiddleware_downloadtimeout.py b/tests/test_downloadermiddleware_downloadtimeout.py index 3707cee18..c744d259c 100644 --- a/tests/test_downloadermiddleware_downloadtimeout.py +++ b/tests/test_downloadermiddleware_downloadtimeout.py @@ -23,16 +23,14 @@ class TestDownloadTimeoutMiddleware: assert mw.process_request(req) is None assert req.meta.get("download_timeout") == 20.1 - def test_spider_has_download_timeout(self): - req, spider, mw = self.get_request_spider_mw() - spider.download_timeout = 2 + def test_setting_has_download_timeout(self): + req, spider, mw = self.get_request_spider_mw({"DOWNLOAD_TIMEOUT": 2}) mw.spider_opened(spider) assert mw.process_request(req) is None assert req.meta.get("download_timeout") == 2 def test_request_has_download_timeout(self): - req, spider, mw = self.get_request_spider_mw() - spider.download_timeout = 2 + req, spider, mw = self.get_request_spider_mw({"DOWNLOAD_TIMEOUT": 2}) mw.spider_opened(spider) req.meta["download_timeout"] = 1 assert mw.process_request(req) is None diff --git a/tests/test_downloadermiddleware_useragent.py b/tests/test_downloadermiddleware_useragent.py index 60dc2ae7a..539183cd6 100644 --- a/tests/test_downloadermiddleware_useragent.py +++ b/tests/test_downloadermiddleware_useragent.py @@ -16,26 +16,8 @@ class TestUserAgentMiddleware: assert mw.process_request(req) is None assert req.headers["User-Agent"] == b"default_useragent" - def test_remove_agent(self): - # settings USER_AGENT to None should remove the user agent - spider, mw = self.get_spider_and_mw("default_useragent") - spider.user_agent = None - mw.spider_opened(spider) - req = Request("http://scrapytest.org/") - assert mw.process_request(req) is None - assert req.headers.get("User-Agent") is None - - def test_spider_agent(self): - spider, mw = self.get_spider_and_mw("default_useragent") - spider.user_agent = "spider_useragent" - mw.spider_opened(spider) - req = Request("http://scrapytest.org/") - assert mw.process_request(req) is None - assert req.headers["User-Agent"] == b"spider_useragent" - def test_header_agent(self): spider, mw = self.get_spider_and_mw("default_useragent") - spider.user_agent = "spider_useragent" mw.spider_opened(spider) req = Request( "http://scrapytest.org/", headers={"User-Agent": "header_useragent"} @@ -45,7 +27,6 @@ class TestUserAgentMiddleware: def test_no_agent(self): spider, mw = self.get_spider_and_mw(None) - spider.user_agent = None mw.spider_opened(spider) req = Request("http://scrapytest.org/") assert mw.process_request(req) is None