mirror of https://github.com/scrapy/scrapy.git
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 <wrar@wrar.name>
This commit is contained in:
parent
1e8de24380
commit
11073c8680
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue