From 4bbe6f55d3ccde4a673d32ed2bfbd767d694848b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 5 Jul 2024 17:29:49 +0200 Subject: [PATCH 01/42] Initial implementation --- scrapy/core/engine.py | 68 ++++++++++++++++++++++++++++---- scrapy/core/scraper.py | 19 +-------- scrapy/http/response/__init__.py | 12 ++++++ 3 files changed, 74 insertions(+), 25 deletions(-) diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 5318cbd64..5aae35f0c 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -23,6 +23,7 @@ from typing import ( Union, cast, ) +from warnings import warn from twisted.internet.defer import Deferred, inlineCallbacks, succeed from twisted.internet.task import LoopingCall @@ -31,7 +32,12 @@ from twisted.python.failure import Failure from scrapy import signals from scrapy.core.downloader import Downloader 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.logformatter import LogFormatter from scrapy.settings import Settings @@ -117,6 +123,24 @@ class ExecutionEngine: ) self.start_time: Optional[float] = None + default_response_max_active_size = 5000000 + scraper_max_active_size = self.settings.getint( + "SCRAPER_MAX_ACTIVE_SIZE", 5000000 + ) + if scraper_max_active_size != default_response_max_active_size: + warn( + ( + "The SCRAPER_MAX_ACTIVE_SIZE setting is deprecated, use " + "RESPONSE_MAX_ACTIVE_SIZE instead." + ), + ScrapyDeprecationWarning, + ) + default_response_max_active_size = scraper_max_active_size + self._response_max_active_size = self.settings.getint( + "RESPONSE_MAX_ACTIVE_SIZE", default_response_max_active_size + ) + self._response_max_active_size_warned = False + def _get_scheduler_class(self, settings: BaseSettings) -> Type[BaseScheduler]: from scrapy.core.scheduler import BaseScheduler @@ -210,15 +234,43 @@ class ExecutionEngine: if self.spider_is_idle() and self.slot.close_if_idle: self._spider_idle() + def _count_backout(self, reason): + self.crawler.stats.inc_value("request_backouts/total") + self.crawler.stats.inc_value(f"request_backouts/{reason}") + def _needs_backout(self) -> bool: assert self.slot is not None # typing - assert self.scraper.slot is not None # typing - return ( - not self.running - or bool(self.slot.closing) - or self.downloader.needs_backout() - or self.scraper.slot.needs_backout() - ) + if not self.running or bool(self.slot.closing): + return True + if self.downloader.needs_backout(): + self._count_backout("concurrent_requests") + return True + if ( + self._response_max_active_size + and Response._ACTIVE_SIZE >= self._response_max_active_size + ): + if not self._response_max_active_size_warned: + self._response_max_active_size_warned = True + logger.info( + f"The total size of all bodies from active responses " + f"({Response._ACTIVE_SIZE} B) has surpassed its maximum " + f"({self._response_max_active_size} B) configured through " + f"the RESPONSE_MAX_ACTIVE_SIZE setting; no more requests " + f"will be processed until that changes. If your memory " + f"allows, increase RESPONSE_MAX_ACTIVE_SIZE to increase " + f"your crawl speed. If your code keeps non-weak " + f"references to Response objects, your crawl might get " + f"stuck indefinitely; you can set " + f"RESPONSE_MAX_ACTIVE_SIZE to 0 to disable this pause of " + f"request processing, but then your code might run out of " + f"memory. This message will only appear the first time " + f"this happens. To learn how often request processing has " + f"been paused during a crawl for this reason, see the " + f"request_backouts/response_max_active_size stat." + ) + self._count_backout("response_max_active_size") + return True + return False def _next_request_from_scheduler(self) -> Optional[Deferred[None]]: assert self.slot is not None # typing diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index a7d65e1e3..d61040e7a 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -65,13 +65,9 @@ if TYPE_CHECKING: class Slot: """Scraper slot (one per running spider)""" - MIN_RESPONSE_SIZE = 1024 - - def __init__(self, max_active_size: int = 5000000): - self.max_active_size = max_active_size + def __init__(self): self.queue: Deque[QueueTuple] = deque() self.active: Set[Request] = set() - self.active_size: int = 0 self.itemproc_size: int = 0 self.closing: Optional[Deferred[Spider]] = None @@ -80,10 +76,6 @@ class Slot: ) -> _HandleOutputDeferred: deferred: _HandleOutputDeferred = Deferred() self.queue.append((result, request, deferred)) - if isinstance(result, Response): - self.active_size += max(len(result.body), self.MIN_RESPONSE_SIZE) - else: - self.active_size += self.MIN_RESPONSE_SIZE return deferred def next_response_request_deferred(self) -> QueueTuple: @@ -95,17 +87,10 @@ class Slot: self, result: Union[Response, Failure], request: Request ) -> None: self.active.remove(request) - if isinstance(result, Response): - self.active_size -= max(len(result.body), self.MIN_RESPONSE_SIZE) - else: - self.active_size -= self.MIN_RESPONSE_SIZE def is_idle(self) -> bool: return not (self.queue or self.active) - def needs_backout(self) -> bool: - return self.active_size > self.max_active_size - class Scraper: def __init__(self, crawler: Crawler) -> None: @@ -126,7 +111,7 @@ class Scraper: @inlineCallbacks def open_spider(self, spider: Spider) -> Generator[Deferred[Any], Any, None]: """Open the given spider for scraping and allocate resources for it""" - self.slot = Slot(self.crawler.settings.getint("SCRAPER_SLOT_MAX_ACTIVE_SIZE")) + self.slot = Slot() yield self.itemproc.open_spider(spider) def close_spider(self, spider: Spider) -> Deferred[Spider]: diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index 92e4852b6..f94a87e77 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -7,6 +7,7 @@ See documentation in docs/topics/request-response.rst from __future__ import annotations +from functools import partial from typing import ( TYPE_CHECKING, Any, @@ -24,6 +25,7 @@ from typing import ( overload, ) from urllib.parse import urljoin +from weakref import finalize from scrapy.exceptions import NotSupported from scrapy.http.headers import Headers @@ -70,6 +72,12 @@ class Response(object_ref): Currently used by :meth:`Response.replace`. """ + _ACTIVE_SIZE = 0 + + @classmethod + def _finalize(cls, size): + Response._ACTIVE_SIZE -= size + def __init__( self, url: str, @@ -92,6 +100,10 @@ class Response(object_ref): self.ip_address: Union[IPv4Address, IPv6Address, None] = ip_address self.protocol: Optional[str] = protocol + size = len(self._body) + Response._ACTIVE_SIZE += size + finalize(self, partial(Response._finalize, size)) + @property def cb_kwargs(self) -> Dict[str, Any]: try: From 2621b97c941fad3673e096a4bae436aa21da58eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 08:17:11 +0200 Subject: [PATCH 02/42] =?UTF-8?q?SCRAPER=5FMAX=5FACTIVE=5FSIZE=20=E2=86=92?= =?UTF-8?q?=20SCRAPER=5FSLOT=5FMAX=5FACTIVE=5FSIZE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/core/engine.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 5aae35f0c..af78f1545 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -125,13 +125,13 @@ class ExecutionEngine: default_response_max_active_size = 5000000 scraper_max_active_size = self.settings.getint( - "SCRAPER_MAX_ACTIVE_SIZE", 5000000 + "SCRAPER_SLOT_MAX_ACTIVE_SIZE", 5000000 ) if scraper_max_active_size != default_response_max_active_size: warn( ( - "The SCRAPER_MAX_ACTIVE_SIZE setting is deprecated, use " - "RESPONSE_MAX_ACTIVE_SIZE instead." + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, " + "use RESPONSE_MAX_ACTIVE_SIZE instead." ), ScrapyDeprecationWarning, ) From 8e534f708a72c6aa11c46b4ee51a8fa68197b3db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 09:17:24 +0200 Subject: [PATCH 03/42] Move the implementation to the downloader --- scrapy/core/downloader/__init__.py | 58 +++++++++++++++++++- scrapy/core/downloader/middleware.py | 80 +++++++++++++++++++--------- scrapy/core/engine.py | 66 +++-------------------- scrapy/http/response/__init__.py | 12 ----- 4 files changed, 119 insertions(+), 97 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 6786d7acf..a37436f6f 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -4,6 +4,7 @@ import random import warnings from collections import deque from datetime import datetime +from logging import getLogger from time import time from typing import ( TYPE_CHECKING, @@ -17,6 +18,7 @@ from typing import ( Union, cast, ) +from warnings import warn from twisted.internet import task from twisted.internet.defer import Deferred @@ -35,6 +37,7 @@ if TYPE_CHECKING: from scrapy.http import Response from scrapy.settings import BaseSettings +logger = getLogger(__name__) _T = TypeVar("_T") @@ -129,6 +132,25 @@ class Downloader: self.per_slot_settings: Dict[str, Dict[str, Any]] = self.settings.getdict( "DOWNLOAD_SLOTS", {} ) + self._stats = crawler.stats + + default_response_max_active_size = 5000000 + scraper_max_active_size = self.settings.getint( + "SCRAPER_MAX_ACTIVE_SIZE", 5000000 + ) + if scraper_max_active_size != default_response_max_active_size: + warn( + ( + "The SCRAPER_MAX_ACTIVE_SIZE setting is deprecated, use " + "RESPONSE_MAX_ACTIVE_SIZE instead." + ), + ScrapyDeprecationWarning, + ) + default_response_max_active_size = scraper_max_active_size + self._response_max_active_size = self.settings.getint( + "RESPONSE_MAX_ACTIVE_SIZE", default_response_max_active_size + ) + self._response_max_active_size_warned = False def fetch( self, request: Request, spider: Spider @@ -143,8 +165,42 @@ class Downloader: ) return dfd.addBoth(_deactivate) + def _count_backout(self, reason): + self._stats.inc_value("request_backouts/total") + self._stats.inc_value(f"request_backouts/{reason}") + def needs_backout(self) -> bool: - return len(self.active) >= self.total_concurrency + if len(self.active) >= self.total_concurrency: + self._count_backout("concurrent_requests") + return True + if ( + self._response_max_active_size + and self.middleware.response_active_size >= self._response_max_active_size + ): + if not self._response_max_active_size_warned: + self._response_max_active_size_warned = True + logger.info( + f"The active response size, i.e. the total size of all " + f"bodies from responses that have been processed by " + f"downloader middlewares and remain in memory, is " + f"{self.middleware.response_active_size} B. The " + f"RESPONSE_MAX_ACTIVE_SIZE setting sets its maximum value " + f"at {self._response_max_active_size} B. No more requests " + f"will be processed until active response size lowers. If " + f"your memory allows it, you may increase " + f"RESPONSE_MAX_ACTIVE_SIZE, which should increase your " + f"crawl speed. If your code keeps non-weak references to " + f"Response objects, your crawl might get stuck " + f"indefinitely; you can set RESPONSE_MAX_ACTIVE_SIZE to 0 " + f"to disable this limit, but then your code might run out " + f"of memory. This message will only appear the first time " + f"this happens. To learn how often request processing has " + f"been paused during a crawl for this reason, see the " + f"request_backouts/response_max_active_size stat." + ) + self._count_backout("response_max_active_size") + return True + return False def _get_slot(self, request: Request, spider: Spider) -> Tuple[str, Slot]: key = self.get_slot_key(request) diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 0bdb756c8..1ad1752f7 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -6,7 +6,9 @@ See documentation in docs/topics/downloader-middleware.rst from __future__ import annotations +from functools import partial from typing import TYPE_CHECKING, Any, Callable, Generator, List, Union, cast +from weakref import WeakSet, finalize from twisted.internet.defer import Deferred, inlineCallbacks @@ -26,6 +28,11 @@ if TYPE_CHECKING: class DownloaderMiddlewareManager(MiddlewareManager): component_name = "downloader middleware" + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.response_active_size = 0 + self._tracked_responses = WeakSet() + @classmethod def _get_mwlist_from_settings(cls, settings: BaseSettings) -> List[Any]: return build_component_list(settings.getwithbase("DOWNLOADER_MIDDLEWARES")) @@ -38,6 +45,29 @@ class DownloaderMiddlewareManager(MiddlewareManager): if hasattr(mw, "process_exception"): self.methods["process_exception"].appendleft(mw.process_exception) + def _count_response_size(self, response: Response) -> None: + if response in self._tracked_responses: + return + self._tracked_responses.add(response) + size = len(response.body) + from logging import getLogger + + logger = getLogger(__name__) + logger.debug( + f"{self.response_active_size=} += {size=} → {self.response_active_size + size}" + ) + self.response_active_size += size + finalize(response, partial(self._discount_response_size, size)) + + def _discount_response_size(self, size: int) -> None: + from logging import getLogger + + logger = getLogger(__name__) + logger.debug( + f"{self.response_active_size=} -= {size=} → {self.response_active_size - size}" + ) + self.response_active_size -= size + def download( self, download_func: Callable[[Request, Spider], Deferred[Response]], @@ -50,42 +80,44 @@ class DownloaderMiddlewareManager(MiddlewareManager): ) -> Generator[Deferred[Any], Any, Union[Response, Request]]: for method in self.methods["process_request"]: method = cast(Callable, method) - response = yield deferred_from_coro( + result = yield deferred_from_coro( method(request=request, spider=spider) ) - if response is not None and not isinstance( - response, (Response, Request) - ): + if result is not None and not isinstance(result, (Response, Request)): raise _InvalidOutput( f"Middleware {method.__qualname__} must return None, Response or " - f"Request, got {response.__class__.__name__}" + f"Request, got {result.__class__.__name__}" ) - if response: - return response + if isinstance(result, Response): + self._count_response_size(result) + if result: + return result return (yield download_func(request, spider)) @inlineCallbacks def process_response( response: Union[Response, Request] ) -> Generator[Deferred[Any], Any, Union[Response, Request]]: - if response is None: + result = response + if result is None: raise TypeError("Received None in process_response") - elif isinstance(response, Request): - return response + elif isinstance(result, Request): + return result for method in self.methods["process_response"]: method = cast(Callable, method) - response = yield deferred_from_coro( - method(request=request, response=response, spider=spider) + result = yield deferred_from_coro( + method(request=request, response=result, spider=spider) ) - if not isinstance(response, (Response, Request)): + if not isinstance(result, (Response, Request)): raise _InvalidOutput( f"Middleware {method.__qualname__} must return Response or Request, " - f"got {type(response)}" + f"got {type(result)}" ) - if isinstance(response, Request): - return response - return response + if isinstance(result, Request): + return result + self._count_response_size(result) + return result @inlineCallbacks def process_exception( @@ -94,18 +126,18 @@ class DownloaderMiddlewareManager(MiddlewareManager): exception = failure.value for method in self.methods["process_exception"]: method = cast(Callable, method) - response = yield deferred_from_coro( + result = yield deferred_from_coro( method(request=request, exception=exception, spider=spider) ) - if response is not None and not isinstance( - response, (Response, Request) - ): + if result is not None and not isinstance(result, (Response, Request)): raise _InvalidOutput( f"Middleware {method.__qualname__} must return None, Response or " - f"Request, got {type(response)}" + f"Request, got {type(result)}" ) - if response: - return response + if isinstance(result, Response): + self._count_response_size(result) + if result: + return result return failure deferred: Deferred[Union[Response, Request]] = mustbe_deferred( diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index af78f1545..4ce77c315 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -23,7 +23,6 @@ from typing import ( Union, cast, ) -from warnings import warn from twisted.internet.defer import Deferred, inlineCallbacks, succeed from twisted.internet.task import LoopingCall @@ -32,12 +31,7 @@ from twisted.python.failure import Failure from scrapy import signals from scrapy.core.downloader import Downloader from scrapy.core.scraper import Scraper -from scrapy.exceptions import ( - CloseSpider, - DontCloseSpider, - IgnoreRequest, - ScrapyDeprecationWarning, -) +from scrapy.exceptions import CloseSpider, DontCloseSpider, IgnoreRequest from scrapy.http import Request, Response from scrapy.logformatter import LogFormatter from scrapy.settings import Settings @@ -123,24 +117,6 @@ class ExecutionEngine: ) self.start_time: Optional[float] = None - default_response_max_active_size = 5000000 - scraper_max_active_size = self.settings.getint( - "SCRAPER_SLOT_MAX_ACTIVE_SIZE", 5000000 - ) - if scraper_max_active_size != default_response_max_active_size: - warn( - ( - "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, " - "use RESPONSE_MAX_ACTIVE_SIZE instead." - ), - ScrapyDeprecationWarning, - ) - default_response_max_active_size = scraper_max_active_size - self._response_max_active_size = self.settings.getint( - "RESPONSE_MAX_ACTIVE_SIZE", default_response_max_active_size - ) - self._response_max_active_size_warned = False - def _get_scheduler_class(self, settings: BaseSettings) -> Type[BaseScheduler]: from scrapy.core.scheduler import BaseScheduler @@ -234,43 +210,13 @@ class ExecutionEngine: if self.spider_is_idle() and self.slot.close_if_idle: self._spider_idle() - def _count_backout(self, reason): - self.crawler.stats.inc_value("request_backouts/total") - self.crawler.stats.inc_value(f"request_backouts/{reason}") - def _needs_backout(self) -> bool: assert self.slot is not None # typing - if not self.running or bool(self.slot.closing): - return True - if self.downloader.needs_backout(): - self._count_backout("concurrent_requests") - return True - if ( - self._response_max_active_size - and Response._ACTIVE_SIZE >= self._response_max_active_size - ): - if not self._response_max_active_size_warned: - self._response_max_active_size_warned = True - logger.info( - f"The total size of all bodies from active responses " - f"({Response._ACTIVE_SIZE} B) has surpassed its maximum " - f"({self._response_max_active_size} B) configured through " - f"the RESPONSE_MAX_ACTIVE_SIZE setting; no more requests " - f"will be processed until that changes. If your memory " - f"allows, increase RESPONSE_MAX_ACTIVE_SIZE to increase " - f"your crawl speed. If your code keeps non-weak " - f"references to Response objects, your crawl might get " - f"stuck indefinitely; you can set " - f"RESPONSE_MAX_ACTIVE_SIZE to 0 to disable this pause of " - f"request processing, but then your code might run out of " - f"memory. This message will only appear the first time " - f"this happens. To learn how often request processing has " - f"been paused during a crawl for this reason, see the " - f"request_backouts/response_max_active_size stat." - ) - self._count_backout("response_max_active_size") - return True - return False + return ( + not self.running + or bool(self.slot.closing) + or self.downloader.needs_backout() + ) def _next_request_from_scheduler(self) -> Optional[Deferred[None]]: assert self.slot is not None # typing diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index f94a87e77..92e4852b6 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -7,7 +7,6 @@ See documentation in docs/topics/request-response.rst from __future__ import annotations -from functools import partial from typing import ( TYPE_CHECKING, Any, @@ -25,7 +24,6 @@ from typing import ( overload, ) from urllib.parse import urljoin -from weakref import finalize from scrapy.exceptions import NotSupported from scrapy.http.headers import Headers @@ -72,12 +70,6 @@ class Response(object_ref): Currently used by :meth:`Response.replace`. """ - _ACTIVE_SIZE = 0 - - @classmethod - def _finalize(cls, size): - Response._ACTIVE_SIZE -= size - def __init__( self, url: str, @@ -100,10 +92,6 @@ class Response(object_ref): self.ip_address: Union[IPv4Address, IPv6Address, None] = ip_address self.protocol: Optional[str] = protocol - size = len(self._body) - Response._ACTIVE_SIZE += size - finalize(self, partial(Response._finalize, size)) - @property def cb_kwargs(self) -> Dict[str, Any]: try: From 58302cab38277836f57c7be47e214ad8ba701156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 09:37:24 +0200 Subject: [PATCH 04/42] Add a request_backouts/total_per_second stat --- scrapy/extensions/corestats.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scrapy/extensions/corestats.py b/scrapy/extensions/corestats.py index 6ef2d0382..ab131a7b2 100644 --- a/scrapy/extensions/corestats.py +++ b/scrapy/extensions/corestats.py @@ -47,6 +47,13 @@ class CoreStats: ) self.stats.set_value("finish_time", finish_time, spider=spider) self.stats.set_value("finish_reason", reason, spider=spider) + if elapsed_time_seconds > 0: + request_backouts = self.stats.get_value("request_backouts/total", 0) + self.stats.set_value( + "request_backouts/total_per_second", + request_backouts / elapsed_time_seconds, + spider=spider, + ) def item_scraped(self, item: Any, spider: Spider) -> None: self.stats.inc_value("item_scraped_count", spider=spider) From 1d254948617d90ee9af7dfe56622bf1f54283efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 10:05:11 +0200 Subject: [PATCH 05/42] Restore backward compatibility in the scraper --- scrapy/core/scraper.py | 111 +++++++++++++++++++++++++++++++++++++++-- setup.py | 1 + tox.ini | 1 + 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index d61040e7a..6a350c236 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -22,14 +22,21 @@ from typing import ( Union, cast, ) +from warnings import warn +from classutilities import ClassPropertiesMixin, classproperty from itemadapter import is_item from twisted.internet.defer import Deferred, inlineCallbacks from twisted.python.failure import Failure from scrapy import Spider, signals from scrapy.core.spidermw import SpiderMiddlewareManager -from scrapy.exceptions import CloseSpider, DropItem, IgnoreRequest +from scrapy.exceptions import ( + CloseSpider, + DropItem, + IgnoreRequest, + ScrapyDeprecationWarning, +) from scrapy.http import Request, Response from scrapy.logformatter import LogFormatter from scrapy.pipelines import ItemPipelineManager @@ -62,20 +69,107 @@ if TYPE_CHECKING: QueueTuple = Tuple[Union[Response, Failure], Request, _HandleOutputDeferred] -class Slot: +_UNSET = object() + + +class Slot(ClassPropertiesMixin): """Scraper slot (one per running spider)""" - def __init__(self): + _MIN_RESPONSE_SIZE = 1024 + + @classproperty + def MIN_RESPONSE_SIZE(cls): + warn( + "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", + ScrapyDeprecationWarning, + ) + return cls._MIN_RESPONSE_SIZE + + @MIN_RESPONSE_SIZE.setter + def MIN_RESPONSE_SIZE(cls, value): + warn( + "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", + ScrapyDeprecationWarning, + ) + cls._MIN_RESPONSE_SIZE = value + + def __init__(self, max_active_size: int = _UNSET): + if max_active_size is not _UNSET: + warn( + ( + "The max_active_size parameter of " + "scrapy.core.scraper.Slot is deprecated. Use the " + "RESPONSE_MAX_ACTIVE_SIZE setting instead." + ), + ScrapyDeprecationWarning, + ) + self._max_active_size = max_active_size self.queue: Deque[QueueTuple] = deque() self.active: Set[Request] = set() self.itemproc_size: int = 0 self.closing: Optional[Deferred[Spider]] = None + self._active_size: int = 0 + + @property + def active_size(self): + warn( + ( + "scrapy.core.scraper.Slot.active_size is deprecated. Read " + "scrapy.core.downloader.DownloaderMiddlewareManager.response_active_size " + "instead." + ), + ScrapyDeprecationWarning, + ) + return self._active_size + + @active_size.setter + def active_size(self, value): + warn( + ( + "scrapy.core.scraper.Slot.active_size is deprecated. " + "scrapy.core.downloader.DownloaderMiddlewareManager.response_active_size " + "might work as a replacement, but modifying that attribute " + "might not be a good idea. If you have a use case for it, you " + "might want to bring it up in a GitHub issue, to discuss with " + "Scrapy developers if there is a better approach, or some " + "change we could implement in Scrapy to improve support for " + "your use case." + ), + ScrapyDeprecationWarning, + ) + self._active_size = value + + @property + def max_active_size(self): + warn( + ( + "scrapy.core.scraper.Slot.max_active_size is deprecated. Read " + "the RESPONSE_MAX_ACTIVE_SIZE setting instead." + ), + ScrapyDeprecationWarning, + ) + return self._max_active_size + + @max_active_size.setter + def max_active_size(self, value): + warn( + ( + "scrapy.core.scraper.Slot.max_active_size is deprecated. Set " + "the RESPONSE_MAX_ACTIVE_SIZE setting instead." + ), + ScrapyDeprecationWarning, + ) + self._max_active_size = value def add_response_request( self, result: Union[Response, Failure], request: Request ) -> _HandleOutputDeferred: deferred: _HandleOutputDeferred = Deferred() self.queue.append((result, request, deferred)) + if isinstance(result, Response): + self._active_size += max(len(result.body), self._MIN_RESPONSE_SIZE) + else: + self._active_size += self._MIN_RESPONSE_SIZE return deferred def next_response_request_deferred(self) -> QueueTuple: @@ -87,10 +181,21 @@ class Slot: self, result: Union[Response, Failure], request: Request ) -> None: self.active.remove(request) + if isinstance(result, Response): + self._active_size -= max(len(result.body), self._MIN_RESPONSE_SIZE) + else: + self._active_size -= self._MIN_RESPONSE_SIZE def is_idle(self) -> bool: return not (self.queue or self.active) + def needs_backout(self) -> bool: + warn( + "scrapy.core.scraper.Slot.needs_backout is deprecated.", + ScrapyDeprecationWarning, + ) + return self._active_size > self._max_active_size + class Scraper: def __init__(self, crawler: Crawler) -> None: diff --git a/setup.py b/setup.py index 2d6d26b0c..28a1dab89 100644 --- a/setup.py +++ b/setup.py @@ -7,6 +7,7 @@ version = (Path(__file__).parent / "scrapy/VERSION").read_text("ascii").strip() install_requires = [ "Twisted>=18.9.0", + "classutilities>=0.2.1", "cryptography>=36.0.0", "cssselect>=0.9.1", "itemloaders>=1.0.1", diff --git a/tox.ini b/tox.ini index c325064d9..f85c58f04 100644 --- a/tox.ini +++ b/tox.ini @@ -97,6 +97,7 @@ commands = [pinned] basepython = python3.8 deps = + classutilities==0.2.1 cryptography==36.0.0 cssselect==0.9.1 h2==3.0 From 4b0c23194f21db4faa7d35af69e0c16e7902eeb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 10:15:58 +0200 Subject: [PATCH 06/42] Restore Python 3.8 support --- scrapy/_classutilities.py | 94 +++++++++++++++++++++++++++++++++++++++ scrapy/core/scraper.py | 2 +- setup.py | 1 - tox.ini | 1 - 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 scrapy/_classutilities.py diff --git a/scrapy/_classutilities.py b/scrapy/_classutilities.py new file mode 100644 index 000000000..ee58ea62f --- /dev/null +++ b/scrapy/_classutilities.py @@ -0,0 +1,94 @@ +# https://github.com/david-salac/classutilities/issues/1 +# Unmodified copy of +# https://github.com/david-salac/classutilities/blob/a6e4a86331936d432afaa454ed4c963528165a61/src/classutilities/classproperty.py + +# Allows creating a class level property +from typing import Any, Callable + + +class ClassPropertyContainer: + """ + Allows creating a class level property (functionality for + decorator). + """ + + def __init__(self, prop_get: Any, prop_set: Any = None): + """ + Container that allows having a class property decorator. + :param prop_get: Class property getter. + :param prop_set: Class property setter. + """ + self.prop_get: Any = prop_get + self.prop_set: Any = prop_set + + def __get__(self, obj: Any, cls: type = None) -> Callable: + """ + Get the property getter. + :param obj: Instance of the class. + :param cls: Type of the class. + :return: Class property getter. + """ + if cls is None: + cls = type(obj) + return self.prop_get.__get__(obj, cls)() + + def __set__(self, obj, value) -> Callable: + """ + Get the property setter. + :param obj: Instance of the class. + :param value: A value to be set. + :return: Class property setter. + """ + if not self.prop_set: + raise AttributeError("cannot set attribute") + _type: type = type(obj) + if _type == ClassPropertyMetaClass: + _type = obj + return self.prop_set.__get__(obj, _type)(value) + + def setter(self, func: Callable) -> "ClassPropertyContainer": + """ + Allows creating setter in a property like way. + :param func: Getter function. + :return: Setter object for the decorator. + """ + if not isinstance(func, (classmethod, staticmethod)): + func = classmethod(func) + self.prop_set = func + return self + + +def classproperty(func): + """ + Create a decorator for a class level property. + :param func: This class method is decorated. + :return: Modified class method behaving like a class property. + """ + if not isinstance(func, (classmethod, staticmethod)): + # The method must be a classmethod (or staticmethod) + func = classmethod(func) + return ClassPropertyContainer(func) + + +class ClassPropertyMetaClass(type): + """ + Metaclass that allows creating a standard setter. + """ + + def __setattr__(self, key, value): + """Overloads setter for class""" + if key in self.__dict__: + obj = self.__dict__.get(key) + if obj and type(obj) is ClassPropertyContainer: + return obj.__set__(self, value) + + return super().__setattr__(key, value) + + +class ClassPropertiesMixin(metaclass=ClassPropertyMetaClass): + """ + This mixin allows using class properties setter (getter works + correctly even without this mixin) + """ + + pass diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 6a350c236..02ab8d3e4 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -24,12 +24,12 @@ from typing import ( ) from warnings import warn -from classutilities import ClassPropertiesMixin, classproperty from itemadapter import is_item from twisted.internet.defer import Deferred, inlineCallbacks from twisted.python.failure import Failure from scrapy import Spider, signals +from scrapy._classutilities import ClassPropertiesMixin, classproperty from scrapy.core.spidermw import SpiderMiddlewareManager from scrapy.exceptions import ( CloseSpider, diff --git a/setup.py b/setup.py index 28a1dab89..2d6d26b0c 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,6 @@ version = (Path(__file__).parent / "scrapy/VERSION").read_text("ascii").strip() install_requires = [ "Twisted>=18.9.0", - "classutilities>=0.2.1", "cryptography>=36.0.0", "cssselect>=0.9.1", "itemloaders>=1.0.1", diff --git a/tox.ini b/tox.ini index f85c58f04..c325064d9 100644 --- a/tox.ini +++ b/tox.ini @@ -97,7 +97,6 @@ commands = [pinned] basepython = python3.8 deps = - classutilities==0.2.1 cryptography==36.0.0 cssselect==0.9.1 h2==3.0 From c22b450d0cb99c4e823a89ab5a8242790987afba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 10:25:11 +0200 Subject: [PATCH 07/42] Address typing issues --- scrapy/_classutilities.py | 8 +++++--- scrapy/core/scraper.py | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/scrapy/_classutilities.py b/scrapy/_classutilities.py index ee58ea62f..3545ba7d0 100644 --- a/scrapy/_classutilities.py +++ b/scrapy/_classutilities.py @@ -3,7 +3,7 @@ # https://github.com/david-salac/classutilities/blob/a6e4a86331936d432afaa454ed4c963528165a61/src/classutilities/classproperty.py # Allows creating a class level property -from typing import Any, Callable +from typing import Any, Callable, Optional, Union class ClassPropertyContainer: @@ -21,7 +21,7 @@ class ClassPropertyContainer: self.prop_get: Any = prop_get self.prop_set: Any = prop_set - def __get__(self, obj: Any, cls: type = None) -> Callable: + def __get__(self, obj: Any, cls: Optional[type] = None) -> Callable: """ Get the property getter. :param obj: Instance of the class. @@ -46,7 +46,9 @@ class ClassPropertyContainer: _type = obj return self.prop_set.__get__(obj, _type)(value) - def setter(self, func: Callable) -> "ClassPropertyContainer": + def setter( + self, func: Union[Callable, classmethod, staticmethod] + ) -> "ClassPropertyContainer": """ Allows creating setter in a property like way. :param func: Getter function. diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 02ab8d3e4..a5d392870 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -85,7 +85,7 @@ class Slot(ClassPropertiesMixin): ) return cls._MIN_RESPONSE_SIZE - @MIN_RESPONSE_SIZE.setter + @MIN_RESPONSE_SIZE.setter # type: ignore[no-redef] def MIN_RESPONSE_SIZE(cls, value): warn( "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", @@ -93,7 +93,7 @@ class Slot(ClassPropertiesMixin): ) cls._MIN_RESPONSE_SIZE = value - def __init__(self, max_active_size: int = _UNSET): + def __init__(self, max_active_size: Any = _UNSET): if max_active_size is not _UNSET: warn( ( From 035ae28886819a3c555b199395a633800cd744b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 10:29:33 +0200 Subject: [PATCH 08/42] Minor cleanup --- scrapy/core/downloader/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index a37436f6f..71cfb73ce 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -134,9 +134,9 @@ class Downloader: ) self._stats = crawler.stats - default_response_max_active_size = 5000000 + default_response_max_active_size = 5_000_000 scraper_max_active_size = self.settings.getint( - "SCRAPER_MAX_ACTIVE_SIZE", 5000000 + "SCRAPER_MAX_ACTIVE_SIZE", default_response_max_active_size ) if scraper_max_active_size != default_response_max_active_size: warn( From f5779c96249515d642df20bf13df74fde5c8f9d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 10:30:22 +0200 Subject: [PATCH 09/42] =?UTF-8?q?SCRAPER=5FMAX=5FACTIVE=5FSIZE=20=E2=86=92?= =?UTF-8?q?=20SCRAPER=5FSLOT=5FMAX=5FACTIVE=5FSIZE=20(again)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/core/downloader/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 71cfb73ce..6fda7b502 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -136,13 +136,13 @@ class Downloader: default_response_max_active_size = 5_000_000 scraper_max_active_size = self.settings.getint( - "SCRAPER_MAX_ACTIVE_SIZE", default_response_max_active_size + "SCRAPER_SLOT_MAX_ACTIVE_SIZE", default_response_max_active_size ) if scraper_max_active_size != default_response_max_active_size: warn( ( - "The SCRAPER_MAX_ACTIVE_SIZE setting is deprecated, use " - "RESPONSE_MAX_ACTIVE_SIZE instead." + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, " + "use RESPONSE_MAX_ACTIVE_SIZE instead." ), ScrapyDeprecationWarning, ) From 784179ad2bea0185290b2a35d776868d8d6350b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 10:32:49 +0200 Subject: [PATCH 10/42] Remove debugging leftovers --- scrapy/core/downloader/middleware.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 1ad1752f7..5d4830113 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -50,22 +50,10 @@ class DownloaderMiddlewareManager(MiddlewareManager): return self._tracked_responses.add(response) size = len(response.body) - from logging import getLogger - - logger = getLogger(__name__) - logger.debug( - f"{self.response_active_size=} += {size=} → {self.response_active_size + size}" - ) self.response_active_size += size finalize(response, partial(self._discount_response_size, size)) def _discount_response_size(self, size: int) -> None: - from logging import getLogger - - logger = getLogger(__name__) - logger.debug( - f"{self.response_active_size=} -= {size=} → {self.response_active_size - size}" - ) self.response_active_size -= size def download( From 73ac493d4a602a6fa77dfd3761809b62b788df8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 10:36:15 +0200 Subject: [PATCH 11/42] Use stacklevel=2 for scraper warnings --- scrapy/core/scraper.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index a5d392870..5c878ec61 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -82,6 +82,7 @@ class Slot(ClassPropertiesMixin): warn( "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", ScrapyDeprecationWarning, + stacklevel=2, ) return cls._MIN_RESPONSE_SIZE @@ -90,6 +91,7 @@ class Slot(ClassPropertiesMixin): warn( "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", ScrapyDeprecationWarning, + stacklevel=2, ) cls._MIN_RESPONSE_SIZE = value @@ -102,6 +104,7 @@ class Slot(ClassPropertiesMixin): "RESPONSE_MAX_ACTIVE_SIZE setting instead." ), ScrapyDeprecationWarning, + stacklevel=2, ) self._max_active_size = max_active_size self.queue: Deque[QueueTuple] = deque() @@ -119,6 +122,7 @@ class Slot(ClassPropertiesMixin): "instead." ), ScrapyDeprecationWarning, + stacklevel=2, ) return self._active_size @@ -136,6 +140,7 @@ class Slot(ClassPropertiesMixin): "your use case." ), ScrapyDeprecationWarning, + stacklevel=2, ) self._active_size = value @@ -147,6 +152,7 @@ class Slot(ClassPropertiesMixin): "the RESPONSE_MAX_ACTIVE_SIZE setting instead." ), ScrapyDeprecationWarning, + stacklevel=2, ) return self._max_active_size @@ -158,6 +164,7 @@ class Slot(ClassPropertiesMixin): "the RESPONSE_MAX_ACTIVE_SIZE setting instead." ), ScrapyDeprecationWarning, + stacklevel=2, ) self._max_active_size = value @@ -193,6 +200,7 @@ class Slot(ClassPropertiesMixin): warn( "scrapy.core.scraper.Slot.needs_backout is deprecated.", ScrapyDeprecationWarning, + stacklevel=2, ) return self._active_size > self._max_active_size From fdc79f9536ec4d3f90a08b7c595f4fd9b57a1f80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 10:42:58 +0200 Subject: [PATCH 12/42] Solve issues reported by pylint --- scrapy/_classutilities.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scrapy/_classutilities.py b/scrapy/_classutilities.py index 3545ba7d0..652655b01 100644 --- a/scrapy/_classutilities.py +++ b/scrapy/_classutilities.py @@ -77,12 +77,12 @@ class ClassPropertyMetaClass(type): Metaclass that allows creating a standard setter. """ - def __setattr__(self, key, value): + def __setattr__(cls, key, value): """Overloads setter for class""" - if key in self.__dict__: - obj = self.__dict__.get(key) - if obj and type(obj) is ClassPropertyContainer: - return obj.__set__(self, value) + if key in cls.__dict__: + obj = cls.__dict__.get(key) + if obj and isinstance(obj, ClassPropertyContainer): + return obj.__set__(cls, value) return super().__setattr__(key, value) From 30c98924e35f3ccac4b433c123ed8cc6e77bac8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 12:06:31 +0200 Subject: [PATCH 13/42] Test RESPONSE_MAX_ACTIVE_SIZE and SCRAPER_SLOT_MAX_ACTIVE_SIZE setting behavior --- scrapy/core/downloader/__init__.py | 20 +++-- scrapy/settings/default_settings.py | 3 +- tests/test_core_downloader.py | 12 --- tests/test_downloader.py | 116 ++++++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 21 deletions(-) delete mode 100644 tests/test_core_downloader.py create mode 100644 tests/test_downloader.py diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 6fda7b502..dc76a6971 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -134,11 +134,10 @@ class Downloader: ) self._stats = crawler.stats - default_response_max_active_size = 5_000_000 - scraper_max_active_size = self.settings.getint( - "SCRAPER_SLOT_MAX_ACTIVE_SIZE", default_response_max_active_size + deprecated_setting_priority = self.settings.getpriority( + "SCRAPER_SLOT_MAX_ACTIVE_SIZE" ) - if scraper_max_active_size != default_response_max_active_size: + if deprecated_setting_priority > 0: warn( ( "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, " @@ -146,10 +145,15 @@ class Downloader: ), ScrapyDeprecationWarning, ) - default_response_max_active_size = scraper_max_active_size - self._response_max_active_size = self.settings.getint( - "RESPONSE_MAX_ACTIVE_SIZE", default_response_max_active_size - ) + setting_priority = self.settings.getpriority("RESPONSE_MAX_ACTIVE_SIZE") + if setting_priority >= deprecated_setting_priority: + self._response_max_active_size = self.settings.getint( + "RESPONSE_MAX_ACTIVE_SIZE" + ) + else: + self._response_max_active_size = self.settings.getint( + "SCRAPER_SLOT_MAX_ACTIVE_SIZE" + ) self._response_max_active_size_warned = False def fetch( diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 932475fb5..ed1ddf16c 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -292,7 +292,8 @@ SCHEDULER_DISK_QUEUE = "scrapy.squeues.PickleLifoDiskQueue" SCHEDULER_MEMORY_QUEUE = "scrapy.squeues.LifoMemoryQueue" SCHEDULER_PRIORITY_QUEUE = "scrapy.pqueues.ScrapyPriorityQueue" -SCRAPER_SLOT_MAX_ACTIVE_SIZE = 5000000 +SCRAPER_SLOT_MAX_ACTIVE_SIZE = 5_000_000 +RESPONSE_MAX_ACTIVE_SIZE = 5_000_000 SPIDER_LOADER_CLASS = "scrapy.spiderloader.SpiderLoader" SPIDER_LOADER_WARN_ONLY = False diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py deleted file mode 100644 index 81cff4947..000000000 --- a/tests/test_core_downloader.py +++ /dev/null @@ -1,12 +0,0 @@ -from twisted.trial import unittest - -from scrapy.core.downloader import Slot - - -class SlotTest(unittest.TestCase): - def test_repr(self): - slot = Slot(concurrency=8, delay=0.1, randomize_delay=True) - self.assertEqual( - repr(slot), - "Slot(concurrency=8, delay=0.10, randomize_delay=True, throttle=None)", - ) diff --git a/tests/test_downloader.py b/tests/test_downloader.py new file mode 100644 index 000000000..e12c6f29a --- /dev/null +++ b/tests/test_downloader.py @@ -0,0 +1,116 @@ +import warnings + +import pytest +from twisted.trial import unittest + +from scrapy import Spider +from scrapy.core.downloader import Slot +from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.defer import deferred_f_from_coro_f +from scrapy.utils.test import get_crawler + + +class SlotTest(unittest.TestCase): + def test_repr(self): + slot = Slot(concurrency=8, delay=0.1, randomize_delay=True) + self.assertEqual( + repr(slot), + "Slot(concurrency=8, delay=0.10, randomize_delay=True, throttle=None)", + ) + + +class OfflineSpider(Spider): + name = "offline" + start_urls = ["data:,"] + + def parse(self, response): + pass + + +class ResponseMaxActiveSizeTest(unittest.TestCase): + + @deferred_f_from_coro_f + async def test_default(self): + """A crawl without custom settings has its effective response max + active size set to 5 000 000, and triggers no deprecation warning.""" + crawler = get_crawler(OfflineSpider) + with warnings.catch_warnings(): + warnings.simplefilter("error") + await crawler.crawl() + self.assertEqual(crawler.engine.downloader._response_max_active_size, 5_000_000) + + @deferred_f_from_coro_f + async def test_custom(self): + """Setting RESPONSE_MAX_ACTIVE_SIZE to a custom value changes the + effective response max active size.""" + crawler = get_crawler( + OfflineSpider, settings_dict={"RESPONSE_MAX_ACTIVE_SIZE": 0} + ) + with warnings.catch_warnings(): + warnings.simplefilter("error") + await crawler.crawl() + self.assertEqual(crawler.engine.downloader._response_max_active_size, 0) + + @deferred_f_from_coro_f + async def test_deprecated_default(self): + """Setting SCRAPER_SLOT_MAX_ACTIVE_SIZE triggers a deprecation warning, + even if it is the default value.""" + crawler = get_crawler( + OfflineSpider, settings_dict={"SCRAPER_SLOT_MAX_ACTIVE_SIZE": 5_000_000} + ) + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + await crawler.crawl() + self.assertEqual(crawler.engine.downloader._response_max_active_size, 5_000_000) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " + "RESPONSE_MAX_ACTIVE_SIZE instead." + ), + ) + + @deferred_f_from_coro_f + async def test_deprecated_custom(self): + """Setting SCRAPER_SLOT_MAX_ACTIVE_SIZE to a custom value triggers a + deprecation warning, and changes the effective response max active + size.""" + crawler = get_crawler( + OfflineSpider, settings_dict={"SCRAPER_SLOT_MAX_ACTIVE_SIZE": 0} + ) + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + await crawler.crawl() + self.assertEqual(crawler.engine.downloader._response_max_active_size, 0) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " + "RESPONSE_MAX_ACTIVE_SIZE instead." + ), + ) + + @deferred_f_from_coro_f + async def test_both(self): + """Setting RESPONSE_MAX_ACTIVE_SIZE and SCRAPER_SLOT_MAX_ACTIVE_SIZE to + different values with the same setting priority triggers a deprecation + warning about SCRAPER_SLOT_MAX_ACTIVE_SIZE and makes the value of + RESPONSE_MAX_ACTIVE_SIZE the effective response max active size.""" + crawler = get_crawler( + OfflineSpider, + settings_dict={ + "RESPONSE_MAX_ACTIVE_SIZE": 1, + "SCRAPER_SLOT_MAX_ACTIVE_SIZE": 2, + }, + ) + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + await crawler.crawl() + self.assertEqual(crawler.engine.downloader._response_max_active_size, 1) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " + "RESPONSE_MAX_ACTIVE_SIZE instead." + ), + ) From 4479b97895547db27baf0b71c3688e3396cd7ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 14:59:00 +0200 Subject: [PATCH 14/42] Add tests for request backout --- scrapy/core/downloader/__init__.py | 2 +- scrapy/extensions/corestats.py | 11 +- tests/test_downloader.py | 178 +++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 6 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index dc76a6971..8db71c3fb 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -175,7 +175,7 @@ class Downloader: def needs_backout(self) -> bool: if len(self.active) >= self.total_concurrency: - self._count_backout("concurrent_requests") + self._count_backout("concurrency") return True if ( self._response_max_active_size diff --git a/scrapy/extensions/corestats.py b/scrapy/extensions/corestats.py index ab131a7b2..d8b285ca3 100644 --- a/scrapy/extensions/corestats.py +++ b/scrapy/extensions/corestats.py @@ -49,11 +49,12 @@ class CoreStats: self.stats.set_value("finish_reason", reason, spider=spider) if elapsed_time_seconds > 0: request_backouts = self.stats.get_value("request_backouts/total", 0) - self.stats.set_value( - "request_backouts/total_per_second", - request_backouts / elapsed_time_seconds, - spider=spider, - ) + if request_backouts: + self.stats.set_value( + "request_backouts/total_per_second", + request_backouts / elapsed_time_seconds, + spider=spider, + ) def item_scraped(self, item: Any, spider: Spider) -> None: self.stats.inc_value("item_scraped_count", spider=spider) diff --git a/tests/test_downloader.py b/tests/test_downloader.py index e12c6f29a..8283a934c 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -27,6 +27,18 @@ class OfflineSpider(Spider): pass +class gt: + + def __init__(self, value): + self.value = value + + def __eq__(self, other): + return other > self.value + + def __repr__(self): + return f">{self.value}" + + class ResponseMaxActiveSizeTest(unittest.TestCase): @deferred_f_from_coro_f @@ -114,3 +126,169 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): "RESPONSE_MAX_ACTIVE_SIZE instead." ), ) + + @deferred_f_from_coro_f + async def test_both_deprecated_priority(self): + """Setting RESPONSE_MAX_ACTIVE_SIZE and SCRAPER_SLOT_MAX_ACTIVE_SIZE to + different values and SCRAPER_SLOT_MAX_ACTIVE_SIZE with a higher + priority triggers a deprecation warning about + SCRAPER_SLOT_MAX_ACTIVE_SIZE but also makes the value of + SCRAPER_SLOT_MAX_ACTIVE_SIZE the effective response max active size.""" + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + + @classmethod + def update_settings(cls, settings): + settings.set("RESPONSE_MAX_ACTIVE_SIZE", 1, priority=100) + settings.set("SCRAPER_SLOT_MAX_ACTIVE_SIZE", 2, priority=101) + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + await crawler.crawl() + self.assertEqual(crawler.engine.downloader._response_max_active_size, 2) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " + "RESPONSE_MAX_ACTIVE_SIZE instead." + ), + ) + + +class RequestBackoutTest(unittest.TestCase): + + @pytest.fixture(autouse=True) + def use_caplog(self, caplog): + self.caplog = caplog + + @deferred_f_from_coro_f + async def test_none(self): + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await crawler.crawl() + + matching_log_count = 0 + for log_record in self.caplog.records: + if ( + str(log_record.msg).startswith("The active response size") + and log_record.levelname == "INFO" + ): + matching_log_count += 1 + self.assertEqual(matching_log_count, 0) + + stats = { + k: v + for k, v in crawler.stats.get_stats().items() + if k.startswith("request_backouts/") + } + self.assertEqual(stats, {}) + + @deferred_f_from_coro_f + async def test_concurrency(self): + + class SlowDown: + """Downloader middleware that returns a non-instant deferred from + process_request, to force need_backout calls to happen at that + point.""" + + def process_request(self, request, spider): + from twisted.internet import reactor + from twisted.internet.defer import Deferred + + d = Deferred() + reactor.callLater(0, d.callback, None) + return d + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + custom_settings = { + "CONCURRENT_REQUESTS": 1, + "DOWNLOADER_MIDDLEWARES": {SlowDown: 0}, + } + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await crawler.crawl() + + matching_log_count = 0 + for log_record in self.caplog.records: + if ( + str(log_record.msg).startswith("The active response size") + and log_record.levelname == "INFO" + ): + matching_log_count += 1 + self.assertEqual(matching_log_count, 0) + + expected_stats = { + "request_backouts/concurrency": gt(0), + "request_backouts/total": gt(0), + "request_backouts/total_per_second": gt(0), + } + actual_stats = { + k: v + for k, v in crawler.stats.get_stats().items() + if k.startswith("request_backouts/") + } + self.assertEqual(expected_stats, actual_stats) + + @deferred_f_from_coro_f + async def test_response_size(self): + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,a"] + custom_settings = { + "RESPONSE_MAX_ACTIVE_SIZE": 1, + } + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await crawler.crawl() + + matching_log_count = 0 + for log_record in self.caplog.records: + if ( + str(log_record.msg).startswith("The active response size") + and log_record.levelname == "INFO" + ): + matching_log_count += 1 + self.assertEqual(matching_log_count, 1) + + expected_stats = { + # Test > 1, if 1 then we are not really making sure that the INFO + # message above is logged only once in a scenario where active size + # is checked more than once. + "request_backouts/response_max_active_size": gt(1), + "request_backouts/total": gt(0), + "request_backouts/total_per_second": gt(0), + } + actual_stats = { + k: v + for k, v in crawler.stats.get_stats().items() + if k.startswith("request_backouts/") + } + self.assertEqual(expected_stats, actual_stats) From 4b58358fbb230de6281d84cebdb71c62a329f2ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 15:21:11 +0200 Subject: [PATCH 15/42] Test response override from downloader middlewares --- tests/test_downloader.py | 144 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 8283a934c..e998eb7d9 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -6,6 +6,7 @@ from twisted.trial import unittest from scrapy import Spider from scrapy.core.downloader import Slot from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.http import Response from scrapy.utils.defer import deferred_f_from_coro_f from scrapy.utils.test import get_crawler @@ -292,3 +293,146 @@ class RequestBackoutTest(unittest.TestCase): if k.startswith("request_backouts/") } self.assertEqual(expected_stats, actual_stats) + + @deferred_f_from_coro_f + async def test_response_size_process_request(self): + + class DownloaderMiddleware: + + def process_request(self, request, spider): + return Response("https://example.com", body=b"a") + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + custom_settings = { + "DOWNLOADER_MIDDLEWARES": {DownloaderMiddleware: 0}, + "RESPONSE_MAX_ACTIVE_SIZE": 1, + } + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await crawler.crawl() + + matching_log_count = 0 + for log_record in self.caplog.records: + if ( + str(log_record.msg).startswith("The active response size") + and log_record.levelname == "INFO" + ): + matching_log_count += 1 + self.assertEqual(matching_log_count, 1) + + expected_stats = { + "request_backouts/response_max_active_size": gt(0), + "request_backouts/total": gt(0), + "request_backouts/total_per_second": gt(0), + } + actual_stats = { + k: v + for k, v in crawler.stats.get_stats().items() + if k.startswith("request_backouts/") + } + self.assertEqual(expected_stats, actual_stats) + + @deferred_f_from_coro_f + async def test_response_size_process_response(self): + + class DownloaderMiddleware: + + def process_response(self, request, response, spider): + return Response("https://example.com", body=b"a") + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + custom_settings = { + "DOWNLOADER_MIDDLEWARES": {DownloaderMiddleware: 0}, + "RESPONSE_MAX_ACTIVE_SIZE": 1, + } + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await crawler.crawl() + + matching_log_count = 0 + for log_record in self.caplog.records: + if ( + str(log_record.msg).startswith("The active response size") + and log_record.levelname == "INFO" + ): + matching_log_count += 1 + self.assertEqual(matching_log_count, 1) + + expected_stats = { + "request_backouts/response_max_active_size": gt(0), + "request_backouts/total": gt(0), + "request_backouts/total_per_second": gt(0), + } + actual_stats = { + k: v + for k, v in crawler.stats.get_stats().items() + if k.startswith("request_backouts/") + } + self.assertEqual(expected_stats, actual_stats) + + @deferred_f_from_coro_f + async def test_response_size_process_exception(self): + + class DownloaderMiddleware1: + + def process_exception(self, request, exception, spider): + return Response("https://example.com", body=b"a") + + class DownloaderMiddleware2: + + def process_request(self, request, spider): + raise ValueError + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + custom_settings = { + "DOWNLOADER_MIDDLEWARES": { + DownloaderMiddleware1: 0, + DownloaderMiddleware2: 1, + }, + "RESPONSE_MAX_ACTIVE_SIZE": 1, + } + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await crawler.crawl() + + matching_log_count = 0 + for log_record in self.caplog.records: + if ( + str(log_record.msg).startswith("The active response size") + and log_record.levelname == "INFO" + ): + matching_log_count += 1 + self.assertEqual(matching_log_count, 1) + + expected_stats = { + "request_backouts/response_max_active_size": gt(0), + "request_backouts/total": gt(0), + "request_backouts/total_per_second": gt(0), + } + actual_stats = { + k: v + for k, v in crawler.stats.get_stats().items() + if k.startswith("request_backouts/") + } + self.assertEqual(expected_stats, actual_stats) From f1e9acd2e1d06701135bc5a2f355a24857da3e0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 8 Jul 2024 16:48:02 +0200 Subject: [PATCH 16/42] Test the scraper --- scrapy/core/scraper.py | 6 +- tests/test_scraper.py | 187 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 tests/test_scraper.py diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 5c878ec61..3b3bcf64f 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -96,7 +96,9 @@ class Slot(ClassPropertiesMixin): cls._MIN_RESPONSE_SIZE = value def __init__(self, max_active_size: Any = _UNSET): - if max_active_size is not _UNSET: + if max_active_size is _UNSET: + max_active_size = 5_000_000 + else: warn( ( "The max_active_size parameter of " @@ -106,7 +108,7 @@ class Slot(ClassPropertiesMixin): ScrapyDeprecationWarning, stacklevel=2, ) - self._max_active_size = max_active_size + self._max_active_size = max_active_size self.queue: Deque[QueueTuple] = deque() self.active: Set[Request] = set() self.itemproc_size: int = 0 diff --git a/tests/test_scraper.py b/tests/test_scraper.py new file mode 100644 index 000000000..7bfcfa0b8 --- /dev/null +++ b/tests/test_scraper.py @@ -0,0 +1,187 @@ +import warnings + +import pytest +from twisted.trial import unittest + +from scrapy import Spider +from scrapy.core.scraper import Slot +from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.defer import deferred_f_from_coro_f +from scrapy.utils.test import get_crawler + + +class DummySpider(Spider): + name = "test" + start_urls = ["data:,"] + + def parse(self, response): + pass + + +class ScraperTest(unittest.TestCase): + + @deferred_f_from_coro_f + async def test_crawl(self): + """A crawl should not trigger any deprecation warning.""" + outcome = {} + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + + def parse(self, response): + with pytest.warns(ScrapyDeprecationWarning): + outcome["active_size"] = ( + self.crawler.engine.scraper.slot.active_size + ) + + crawler = get_crawler(TestSpider) + with warnings.catch_warnings(): + warnings.simplefilter("error") + await crawler.crawl() + + with pytest.warns(ScrapyDeprecationWarning): + expected = crawler.engine.scraper.slot.MIN_RESPONSE_SIZE + self.assertEqual(outcome["active_size"], expected) + + def test_min_response_time_read(self): + slot = Slot() + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + actual = slot.MIN_RESPONSE_SIZE + self.assertEqual(actual, 1024) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", + ) + + def test_min_response_time_write(self): + slot = Slot() + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + slot.MIN_RESPONSE_SIZE = 0 + with pytest.warns(ScrapyDeprecationWarning): + self.assertEqual(slot.MIN_RESPONSE_SIZE, 0) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", + ) + + def test_slot_init_max_active_size_default(self): + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + slot = Slot(max_active_size=5_000_000) + with pytest.warns(ScrapyDeprecationWarning): + self.assertEqual(slot.max_active_size, 5_000_000) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "The max_active_size parameter of scrapy.core.scraper.Slot is " + "deprecated. Use the RESPONSE_MAX_ACTIVE_SIZE setting instead." + ), + ) + + def test_slot_init_max_active_size_custom(self): + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + slot = Slot(max_active_size=0) + with pytest.warns(ScrapyDeprecationWarning): + self.assertEqual(slot.max_active_size, 0) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "The max_active_size parameter of scrapy.core.scraper.Slot is " + "deprecated. Use the RESPONSE_MAX_ACTIVE_SIZE setting instead." + ), + ) + + def test_max_active_size_read(self): + slot = Slot() + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + actual = slot.max_active_size + self.assertEqual(actual, 5_000_000) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "scrapy.core.scraper.Slot.max_active_size is deprecated. Read " + "the RESPONSE_MAX_ACTIVE_SIZE setting instead." + ), + ) + + def test_max_active_size_write(self): + slot = Slot() + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + slot.max_active_size = 0 + with pytest.warns(ScrapyDeprecationWarning): + self.assertEqual(slot.max_active_size, 0) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "scrapy.core.scraper.Slot.max_active_size is deprecated. Set " + "the RESPONSE_MAX_ACTIVE_SIZE setting instead." + ), + ) + + def test_active_size_read(self): + slot = Slot() + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + actual = slot.active_size + self.assertEqual(actual, 0) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "scrapy.core.scraper.Slot.active_size is deprecated. Read " + "scrapy.core.downloader.DownloaderMiddlewareManager.response_active_size " + "instead." + ), + ) + + def test_active_size_write(self): + slot = Slot() + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + slot.active_size = 1 + with pytest.warns(ScrapyDeprecationWarning): + self.assertEqual(slot.active_size, 1) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + ( + "scrapy.core.scraper.Slot.active_size is deprecated. " + "scrapy.core.downloader.DownloaderMiddlewareManager.response_active_size " + "might work as a replacement, but modifying that attribute " + "might not be a good idea. If you have a use case for it, you " + "might want to bring it up in a GitHub issue, to discuss with " + "Scrapy developers if there is a better approach, or some " + "change we could implement in Scrapy to improve support for " + "your use case." + ), + ) + + def test_needs_backout_false(self): + slot = Slot() + with pytest.warns(ScrapyDeprecationWarning): + slot.active_size = 5_000_000 + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + actual = slot.needs_backout() + self.assertEqual(actual, False) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + "scrapy.core.scraper.Slot.needs_backout is deprecated.", + ) + + def test_needs_backout_true(self): + slot = Slot() + with pytest.warns(ScrapyDeprecationWarning): + slot.active_size = 5_000_001 + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + actual = slot.needs_backout() + self.assertEqual(actual, True) + self.assertEqual(len(warning_messages), 1) + self.assertEqual( + str(warning_messages[0].message), + "scrapy.core.scraper.Slot.needs_backout is deprecated.", + ) From c054e9a008c5f1cad08a93efbf03b1cfea67f38b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 9 Jul 2024 12:08:49 +0200 Subject: [PATCH 17/42] Test engine.download --- tests/test_downloader.py | 57 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/tests/test_downloader.py b/tests/test_downloader.py index e998eb7d9..52909c06d 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -3,7 +3,7 @@ import warnings import pytest from twisted.trial import unittest -from scrapy import Spider +from scrapy import Request, Spider from scrapy.core.downloader import Slot from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Response @@ -436,3 +436,58 @@ class RequestBackoutTest(unittest.TestCase): if k.startswith("request_backouts/") } self.assertEqual(expected_stats, actual_stats) + + @deferred_f_from_coro_f + async def test_response_size_download(self): + """Ensure that responses from engine.download calls are also taken into + account for the RESPONSE_MAX_ACTIVE_SIZE setting.""" + + class SlowDown: + """Item pipeline that returns a non-instant deferred, to force + need_backout calls to happen at that point.""" + + def process_item(self, item, spider): + from twisted.internet import reactor + from twisted.internet.defer import Deferred + + d = Deferred() + reactor.callLater(0, d.callback, {}) + return d + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,"] + custom_settings = { + "ITEM_PIPELINES": {SlowDown: 0}, + "RESPONSE_MAX_ACTIVE_SIZE": 1, + } + + async def parse(self, response): + response = await self.crawler.engine.download(Request("data:,a")) + yield {"response": response} + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await crawler.crawl() + + matching_log_count = 0 + for log_record in self.caplog.records: + if ( + str(log_record.msg).startswith("The active response size") + and log_record.levelname == "INFO" + ): + matching_log_count += 1 + self.assertEqual(matching_log_count, 1) + + expected_stats = { + "request_backouts/response_max_active_size": gt(0), + "request_backouts/total": gt(0), + "request_backouts/total_per_second": gt(0), + } + actual_stats = { + k: v + for k, v in crawler.stats.get_stats().items() + if k.startswith("request_backouts/") + } + self.assertEqual(expected_stats, actual_stats) From b02a0a0c354be20bed357d121f2d08e442fb3c79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 9 Jul 2024 12:11:02 +0200 Subject: [PATCH 18/42] Keep mypy happy --- scrapy/core/downloader/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 8db71c3fb..eac68e071 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -137,6 +137,7 @@ class Downloader: deprecated_setting_priority = self.settings.getpriority( "SCRAPER_SLOT_MAX_ACTIVE_SIZE" ) + assert deprecated_setting_priority is not None if deprecated_setting_priority > 0: warn( ( @@ -146,6 +147,7 @@ class Downloader: ScrapyDeprecationWarning, ) setting_priority = self.settings.getpriority("RESPONSE_MAX_ACTIVE_SIZE") + assert setting_priority is not None if setting_priority >= deprecated_setting_priority: self._response_max_active_size = self.settings.getint( "RESPONSE_MAX_ACTIVE_SIZE" From 11213cc44e34c465f096882509197e7a811c4e11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 9 Jul 2024 12:16:43 +0200 Subject: [PATCH 19/42] Fix test support for asyncio --- tests/test_downloader.py | 28 ++++++++++++++-------------- tests/test_scraper.py | 4 ++-- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 52909c06d..39ac15870 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -7,7 +7,7 @@ from scrapy import Request, Spider from scrapy.core.downloader import Slot from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Response -from scrapy.utils.defer import deferred_f_from_coro_f +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.test import get_crawler @@ -49,7 +49,7 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): crawler = get_crawler(OfflineSpider) with warnings.catch_warnings(): warnings.simplefilter("error") - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) self.assertEqual(crawler.engine.downloader._response_max_active_size, 5_000_000) @deferred_f_from_coro_f @@ -61,7 +61,7 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): ) with warnings.catch_warnings(): warnings.simplefilter("error") - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) self.assertEqual(crawler.engine.downloader._response_max_active_size, 0) @deferred_f_from_coro_f @@ -72,7 +72,7 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): OfflineSpider, settings_dict={"SCRAPER_SLOT_MAX_ACTIVE_SIZE": 5_000_000} ) with pytest.warns(ScrapyDeprecationWarning) as warning_messages: - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) self.assertEqual(crawler.engine.downloader._response_max_active_size, 5_000_000) self.assertEqual(len(warning_messages), 1) self.assertEqual( @@ -92,7 +92,7 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): OfflineSpider, settings_dict={"SCRAPER_SLOT_MAX_ACTIVE_SIZE": 0} ) with pytest.warns(ScrapyDeprecationWarning) as warning_messages: - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) self.assertEqual(crawler.engine.downloader._response_max_active_size, 0) self.assertEqual(len(warning_messages), 1) self.assertEqual( @@ -117,7 +117,7 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): }, ) with pytest.warns(ScrapyDeprecationWarning) as warning_messages: - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) self.assertEqual(crawler.engine.downloader._response_max_active_size, 1) self.assertEqual(len(warning_messages), 1) self.assertEqual( @@ -150,7 +150,7 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): crawler = get_crawler(TestSpider) with pytest.warns(ScrapyDeprecationWarning) as warning_messages: - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) self.assertEqual(crawler.engine.downloader._response_max_active_size, 2) self.assertEqual(len(warning_messages), 1) self.assertEqual( @@ -181,7 +181,7 @@ class RequestBackoutTest(unittest.TestCase): crawler = get_crawler(TestSpider) self.caplog.clear() with self.caplog.at_level("INFO"): - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) matching_log_count = 0 for log_record in self.caplog.records: @@ -229,7 +229,7 @@ class RequestBackoutTest(unittest.TestCase): crawler = get_crawler(TestSpider) self.caplog.clear() with self.caplog.at_level("INFO"): - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) matching_log_count = 0 for log_record in self.caplog.records: @@ -268,7 +268,7 @@ class RequestBackoutTest(unittest.TestCase): crawler = get_crawler(TestSpider) self.caplog.clear() with self.caplog.at_level("INFO"): - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) matching_log_count = 0 for log_record in self.caplog.records: @@ -316,7 +316,7 @@ class RequestBackoutTest(unittest.TestCase): crawler = get_crawler(TestSpider) self.caplog.clear() with self.caplog.at_level("INFO"): - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) matching_log_count = 0 for log_record in self.caplog.records: @@ -361,7 +361,7 @@ class RequestBackoutTest(unittest.TestCase): crawler = get_crawler(TestSpider) self.caplog.clear() with self.caplog.at_level("INFO"): - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) matching_log_count = 0 for log_record in self.caplog.records: @@ -414,7 +414,7 @@ class RequestBackoutTest(unittest.TestCase): crawler = get_crawler(TestSpider) self.caplog.clear() with self.caplog.at_level("INFO"): - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) matching_log_count = 0 for log_record in self.caplog.records: @@ -469,7 +469,7 @@ class RequestBackoutTest(unittest.TestCase): crawler = get_crawler(TestSpider) self.caplog.clear() with self.caplog.at_level("INFO"): - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) matching_log_count = 0 for log_record in self.caplog.records: diff --git a/tests/test_scraper.py b/tests/test_scraper.py index 7bfcfa0b8..9a1f261e5 100644 --- a/tests/test_scraper.py +++ b/tests/test_scraper.py @@ -6,7 +6,7 @@ from twisted.trial import unittest from scrapy import Spider from scrapy.core.scraper import Slot from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.defer import deferred_f_from_coro_f +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.test import get_crawler @@ -38,7 +38,7 @@ class ScraperTest(unittest.TestCase): crawler = get_crawler(TestSpider) with warnings.catch_warnings(): warnings.simplefilter("error") - await crawler.crawl() + await maybe_deferred_to_future(crawler.crawl()) with pytest.warns(ScrapyDeprecationWarning): expected = crawler.engine.scraper.slot.MIN_RESPONSE_SIZE From ec7dbfd6a589a35d9536f1c2c1890bd36d182015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 9 Jul 2024 13:01:52 +0200 Subject: [PATCH 20/42] Force garbage collection for PyPy support --- scrapy/core/downloader/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index eac68e071..60ec957d9 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -1,5 +1,6 @@ from __future__ import annotations +import gc import random import warnings from collections import deque @@ -205,6 +206,9 @@ class Downloader: f"request_backouts/response_max_active_size stat." ) self._count_backout("response_max_active_size") + # Force the garbage collection of response objects. Necessary for + # PyPy, which is lazier when it comes to garbage collection. + gc.collect() return True return False From 3c5acd252fb27b7ff5a0b6dbcb0b62d9b65526d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 9 Jul 2024 13:09:58 +0200 Subject: [PATCH 21/42] Mention scheduled requests as one place not to put responses in --- scrapy/core/downloader/__init__.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 60ec957d9..4199ed4e2 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -197,12 +197,14 @@ class Downloader: f"your memory allows it, you may increase " f"RESPONSE_MAX_ACTIVE_SIZE, which should increase your " f"crawl speed. If your code keeps non-weak references to " - f"Response objects, your crawl might get stuck " - f"indefinitely; you can set RESPONSE_MAX_ACTIVE_SIZE to 0 " - f"to disable this limit, but then your code might run out " - f"of memory. This message will only appear the first time " - f"this happens. To learn how often request processing has " - f"been paused during a crawl for this reason, see the " + f"Response objects, e.g. in (scheduled) requests or in a " + f"container within a component, your crawl might get " + f"stuck indefinitely; you can set " + f"RESPONSE_MAX_ACTIVE_SIZE to 0 to disable this limit, " + f"but then your code might run out of memory. This " + f"message will only appear the first time this happens. " + f"To learn how often request processing has been paused " + f"during a crawl for this reason, see the " f"request_backouts/response_max_active_size stat." ) self._count_backout("response_max_active_size") From 36cf156500c0f421546fdb9df97b99db16300e35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 9 Jul 2024 14:04:16 +0200 Subject: [PATCH 22/42] Record backout seconds instead of call counts --- scrapy/core/downloader/__init__.py | 21 +++++++++---- scrapy/extensions/corestats.py | 8 ----- tests/test_downloader.py | 47 ++++++++++++------------------ 3 files changed, 35 insertions(+), 41 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 4199ed4e2..79c2846e1 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -134,6 +134,7 @@ class Downloader: "DOWNLOAD_SLOTS", {} ) self._stats = crawler.stats + self._last_backout = (None, None) deprecated_setting_priority = self.settings.getpriority( "SCRAPER_SLOT_MAX_ACTIVE_SIZE" @@ -172,13 +173,22 @@ class Downloader: ) return dfd.addBoth(_deactivate) - def _count_backout(self, reason): - self._stats.inc_value("request_backouts/total") - self._stats.inc_value(f"request_backouts/{reason}") + def _record_backout(self, reason): + last_reason, last_reason_start_time = self._last_backout + if last_reason == reason: + return + current_time = time() + if last_reason is not None: + last_reason_seconds = current_time - last_reason_start_time + self._stats.inc_value("request_backout_seconds/total", last_reason_seconds) + self._stats.inc_value( + f"request_backout_seconds/{last_reason}", last_reason_seconds + ) + self._last_backout = (reason, current_time) def needs_backout(self) -> bool: if len(self.active) >= self.total_concurrency: - self._count_backout("concurrency") + self._record_backout("concurrency") return True if ( self._response_max_active_size @@ -207,11 +217,12 @@ class Downloader: f"during a crawl for this reason, see the " f"request_backouts/response_max_active_size stat." ) - self._count_backout("response_max_active_size") + self._record_backout("response_max_active_size") # Force the garbage collection of response objects. Necessary for # PyPy, which is lazier when it comes to garbage collection. gc.collect() return True + self._record_backout(None) return False def _get_slot(self, request: Request, spider: Spider) -> Tuple[str, Slot]: diff --git a/scrapy/extensions/corestats.py b/scrapy/extensions/corestats.py index d8b285ca3..6ef2d0382 100644 --- a/scrapy/extensions/corestats.py +++ b/scrapy/extensions/corestats.py @@ -47,14 +47,6 @@ class CoreStats: ) self.stats.set_value("finish_time", finish_time, spider=spider) self.stats.set_value("finish_reason", reason, spider=spider) - if elapsed_time_seconds > 0: - request_backouts = self.stats.get_value("request_backouts/total", 0) - if request_backouts: - self.stats.set_value( - "request_backouts/total_per_second", - request_backouts / elapsed_time_seconds, - spider=spider, - ) def item_scraped(self, item: Any, spider: Spider) -> None: self.stats.inc_value("item_scraped_count", spider=spider) diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 39ac15870..c6de213a6 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -195,7 +195,7 @@ class RequestBackoutTest(unittest.TestCase): stats = { k: v for k, v in crawler.stats.get_stats().items() - if k.startswith("request_backouts/") + if k.startswith("request_backout_seconds/") } self.assertEqual(stats, {}) @@ -241,14 +241,13 @@ class RequestBackoutTest(unittest.TestCase): self.assertEqual(matching_log_count, 0) expected_stats = { - "request_backouts/concurrency": gt(0), - "request_backouts/total": gt(0), - "request_backouts/total_per_second": gt(0), + "request_backout_seconds/concurrency": gt(0), + "request_backout_seconds/total": gt(0), } actual_stats = { k: v for k, v in crawler.stats.get_stats().items() - if k.startswith("request_backouts/") + if k.startswith("request_backout_seconds/") } self.assertEqual(expected_stats, actual_stats) @@ -280,17 +279,13 @@ class RequestBackoutTest(unittest.TestCase): self.assertEqual(matching_log_count, 1) expected_stats = { - # Test > 1, if 1 then we are not really making sure that the INFO - # message above is logged only once in a scenario where active size - # is checked more than once. - "request_backouts/response_max_active_size": gt(1), - "request_backouts/total": gt(0), - "request_backouts/total_per_second": gt(0), + "request_backout_seconds/response_max_active_size": gt(0), + "request_backout_seconds/total": gt(0), } actual_stats = { k: v for k, v in crawler.stats.get_stats().items() - if k.startswith("request_backouts/") + if k.startswith("request_backout_seconds/") } self.assertEqual(expected_stats, actual_stats) @@ -328,14 +323,13 @@ class RequestBackoutTest(unittest.TestCase): self.assertEqual(matching_log_count, 1) expected_stats = { - "request_backouts/response_max_active_size": gt(0), - "request_backouts/total": gt(0), - "request_backouts/total_per_second": gt(0), + "request_backout_seconds/response_max_active_size": gt(0), + "request_backout_seconds/total": gt(0), } actual_stats = { k: v for k, v in crawler.stats.get_stats().items() - if k.startswith("request_backouts/") + if k.startswith("request_backout_seconds/") } self.assertEqual(expected_stats, actual_stats) @@ -373,14 +367,13 @@ class RequestBackoutTest(unittest.TestCase): self.assertEqual(matching_log_count, 1) expected_stats = { - "request_backouts/response_max_active_size": gt(0), - "request_backouts/total": gt(0), - "request_backouts/total_per_second": gt(0), + "request_backout_seconds/response_max_active_size": gt(0), + "request_backout_seconds/total": gt(0), } actual_stats = { k: v for k, v in crawler.stats.get_stats().items() - if k.startswith("request_backouts/") + if k.startswith("request_backout_seconds/") } self.assertEqual(expected_stats, actual_stats) @@ -426,14 +419,13 @@ class RequestBackoutTest(unittest.TestCase): self.assertEqual(matching_log_count, 1) expected_stats = { - "request_backouts/response_max_active_size": gt(0), - "request_backouts/total": gt(0), - "request_backouts/total_per_second": gt(0), + "request_backout_seconds/response_max_active_size": gt(0), + "request_backout_seconds/total": gt(0), } actual_stats = { k: v for k, v in crawler.stats.get_stats().items() - if k.startswith("request_backouts/") + if k.startswith("request_backout_seconds/") } self.assertEqual(expected_stats, actual_stats) @@ -481,13 +473,12 @@ class RequestBackoutTest(unittest.TestCase): self.assertEqual(matching_log_count, 1) expected_stats = { - "request_backouts/response_max_active_size": gt(0), - "request_backouts/total": gt(0), - "request_backouts/total_per_second": gt(0), + "request_backout_seconds/response_max_active_size": gt(0), + "request_backout_seconds/total": gt(0), } actual_stats = { k: v for k, v in crawler.stats.get_stats().items() - if k.startswith("request_backouts/") + if k.startswith("request_backout_seconds/") } self.assertEqual(expected_stats, actual_stats) From f914cd50fbe1ba37a28cf0b753949721986cc25f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 9 Jul 2024 14:17:02 +0200 Subject: [PATCH 23/42] Ignore warnings from third-parties during tests --- pytest.ini | 2 ++ scrapy/core/downloader/__init__.py | 1 + 2 files changed, 3 insertions(+) diff --git a/pytest.ini b/pytest.ini index 16983be5e..9df4370ec 100644 --- a/pytest.ini +++ b/pytest.ini @@ -26,3 +26,5 @@ filterwarnings = ignore:Module scrapy.utils.reqser is deprecated ignore:typing.re is deprecated ignore:typing.io is deprecated + ignore:::h2 + ignore:::w3lib diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 79c2846e1..894ba8900 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -350,6 +350,7 @@ class Downloader: self._slot_gc_loop.stop() for slot in self.slots.values(): slot.close() + self._record_backout(None) def _slot_gc(self, age: float = 60) -> None: mintime = time() - age From 51a4f487942a596001bcb0cdd0ec3a0c3c147e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 9 Jul 2024 15:06:36 +0200 Subject: [PATCH 24/42] Use win-precise-time if available --- scrapy/core/downloader/__init__.py | 6 +++++- tox.ini | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 894ba8900..7368f7d9d 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -6,7 +6,6 @@ import warnings from collections import deque from datetime import datetime from logging import getLogger -from time import time from typing import ( TYPE_CHECKING, Any, @@ -21,6 +20,11 @@ from typing import ( ) from warnings import warn +try: + from win_precise_time import time +except ImportError: + from time import time + from twisted.internet import task from twisted.internet.defer import Deferred diff --git a/tox.ini b/tox.ini index c325064d9..83ad2d881 100644 --- a/tox.ini +++ b/tox.ini @@ -19,6 +19,7 @@ deps = sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 testfixtures pywin32; sys_platform == "win32" + win-precise-time; sys_platform == "win32" [testenv] deps = From 537a577268b69c94352b4f54df904f466c6fe1ce Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 15 Jun 2026 14:13:30 +0200 Subject: [PATCH 25/42] Complete the implementation --- docs/news.rst | 6 +- docs/topics/practices.rst | 40 ++++++++++ docs/topics/request-response.rst | 15 ++++ docs/topics/settings.rst | 78 ++++++++++++++++---- scrapy/core/downloader/__init__.py | 30 +++++--- scrapy/core/downloader/middleware.py | 18 +++++ scrapy/settings/default_settings.py | 3 + tests/test_downloader.py | 106 ++++++++++++++++++++++++++- 8 files changed, 266 insertions(+), 30 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index b8b976df9..a3af425ec 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -5481,9 +5481,9 @@ New features * A new ``DNS_RESOLVER`` setting allows enabling IPv6 support (:issue:`1031`, :issue:`4227`) -* A new :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE` setting allows configuring - the existing soft limit that pauses request downloads when the total - response data being processed is too high (:issue:`1410`, :issue:`3551`) +* A new ``SCRAPER_SLOT_MAX_ACTIVE_SIZE`` setting allows configuring the + existing soft limit that pauses request downloads when the total response + data being processed is too high (:issue:`1410`, :issue:`3551`) * A new :setting:`TWISTED_REACTOR` setting allows customizing the :mod:`~twisted.internet.reactor` that Scrapy uses, allowing to diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index b3c58d6d3..5253c1ed4 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -407,6 +407,46 @@ run: Because :setting:`SPIDER_MODULES` is a list setting, you can include multiple modules by separating them with commas. +.. _crawl-optimization: + +Identifying crawl bottlenecks +============================= + +Scrapy exposes ``request_backout_seconds`` stats that show how long request +scheduling was paused during a crawl, and why: + +- ``request_backout_seconds/total``: total time paused for any reason +- ``request_backout_seconds/concurrency``: time paused because + :setting:`CONCURRENT_REQUESTS` was reached +- ``request_backout_seconds/response_max_active_size``: time paused because + :setting:`RESPONSE_MAX_ACTIVE_SIZE` was reached + +For example, after a crawl you might see:: + + 2025-01-01 00:00:00 [scrapy.statscollectors] INFO: Dumping Scrapy stats: + {'request_backout_seconds/concurrency': 12.5, + 'request_backout_seconds/response_max_active_size': 45.2, + 'request_backout_seconds/total': 57.7, + ...} + +In this case, the spider spent about 45 seconds paused due to large responses +in memory. You could: + +- Increase :setting:`RESPONSE_MAX_ACTIVE_SIZE` if your machine has enough RAM. +- Reduce :setting:`CONCURRENT_REQUESTS` to let responses be processed faster. +- Check that your code doesn't hold strong references to + :class:`~scrapy.http.Response` objects longer than necessary. + +.. note:: + + On Windows, Scrapy uses the `win-precise-time + `_ package when available to + measure backout durations with higher precision. Install it with: + + .. code-block:: shell + + pip install win-precise-time + .. _bans: Avoiding getting banned diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 700238fe9..83387cd96 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -733,6 +733,7 @@ Those are: * :reqmeta:`redirect_reasons` * :reqmeta:`redirect_urls` * :reqmeta:`referrer_policy` +* :reqmeta:`response_rough_size` * :reqmeta:`verbatim_url` .. reqmeta:: bindaddress @@ -839,6 +840,20 @@ The meta key is used set retry times per request. When set, the :reqmeta:`max_retry_times` meta key takes higher precedence over the :setting:`RETRY_TIMES` setting. +.. reqmeta:: response_rough_size + +response_rough_size +------------------- + +Overrides the :setting:`RESPONSE_ROUGH_SIZE` setting for this specific request. + +The value is the estimated size (in bytes) to count toward +:setting:`RESPONSE_MAX_ACTIVE_SIZE` while this request is being downloaded, +before its actual response size is known. Set to ``0`` to disable rough-size +counting for this request. + +.. versionadded:: VERSION + .. reqmeta:: verbatim_url verbatim_url diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 455985a56..70ef3f4cf 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1709,6 +1709,71 @@ Adjust redirect request priority relative to original request: - **a positive priority adjust (default) means higher priority.** - a negative priority adjust means lower priority. +.. setting:: RESPONSE_MAX_ACTIVE_SIZE + +RESPONSE_MAX_ACTIVE_SIZE +------------------------ + +Default: ``5_000_000`` + +Soft limit (in bytes) on the total size of responses being kept in memory. + +This counts both the size of response bodies that have passed through +:ref:`downloader middlewares ` and remain in +memory, and the :setting:`rough size ` of requests +currently being downloaded. + +When the total exceeds this value, Scrapy pauses scheduling new requests until +it drops below the limit. + +If you set this to ``0``, the limit is disabled. + +Setting this to a lower value reduces memory usage at the cost of crawl speed. +Setting this to a higher value (or disabling it) improves crawl speed but may +cause memory issues when responses are large. + +When the limit is first reached, Scrapy logs an info-level message explaining +the situation. Check the ``request_backout_seconds/response_max_active_size`` +stat to see how long request processing has been paused due to this limit over +the course of a crawl. + +.. caution:: + + If your code stores strong references to :class:`~scrapy.http.Response` + objects (e.g. in a scheduled request's meta or in a component attribute), + the garbage collector cannot free them, and the total active size may not + drop below the limit. In that case your crawl might get stuck indefinitely. + Either avoid storing such references, or set this to ``0`` to disable the + limit. + + To check whether your crawl is stuck due to this, connect to the + :ref:`telnet console ` and run ``prefs()`` to see + the count of live :class:`~scrapy.http.Response` objects. If that count + is large and not decreasing, you likely have strong response references. + See :ref:`topics-leaks` for details. + +.. versionadded:: VERSION + +.. setting:: RESPONSE_ROUGH_SIZE + +RESPONSE_ROUGH_SIZE +------------------- + +Default: ``1024`` + +Estimated size (in bytes) to count toward :setting:`RESPONSE_MAX_ACTIVE_SIZE` +for each request that is currently being downloaded, before its actual response +size is known. + +This allows :setting:`RESPONSE_MAX_ACTIVE_SIZE` to provide backpressure based +on the number of concurrent in-flight requests, not just already-received +responses. Once the response arrives, its actual body size is counted instead. + +You can override this value on a per-request basis via the +:reqmeta:`response_rough_size` request meta key. + +.. versionadded:: VERSION + .. setting:: ROBOTSTXT_OBEY ROBOTSTXT_OBEY @@ -1859,19 +1924,6 @@ For available choices, see :setting:`SCHEDULER_MEMORY_QUEUE`. :start-after: queue-common-starts :end-before: queue-common-ends - -.. setting:: SCRAPER_SLOT_MAX_ACTIVE_SIZE - -SCRAPER_SLOT_MAX_ACTIVE_SIZE ----------------------------- - -Default: ``5_000_000`` - -Soft limit (in bytes) for response data being processed. - -While the sum of the sizes of all responses being processed is above this value, -Scrapy does not process new requests. - .. setting:: SPIDER_CONTRACTS SPIDER_CONTRACTS diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 002e58dd4..9c6bc27bd 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -52,7 +52,7 @@ if TYPE_CHECKING: logger = getLogger(__name__) -@dataclass(slots=True, eq=False) +@dataclass(slots=True, eq=False, repr=False) class Slot: """Downloader slot""" @@ -60,13 +60,20 @@ class Slot: delay: float randomize_delay: bool - active: set[Request] = field(default_factory=set, init=False, repr=False) + active: set[Request] = field(default_factory=set, init=False) queue: deque[tuple[Request, Deferred[Response]]] = field( - default_factory=deque, init=False, repr=False + default_factory=deque, init=False ) - transferring: set[Request] = field(default_factory=set, init=False, repr=False) - lastseen: float = field(default=0, init=False, repr=False) - latercall: CallLaterResult | None = field(default=None, init=False, repr=False) + transferring: set[Request] = field(default_factory=set, init=False) + lastseen: float = field(default=0, init=False) + latercall: CallLaterResult | None = field(default=None, init=False) + + def __repr__(self) -> str: + return ( + f"Slot(concurrency={self.concurrency!r}, " + f"delay={self.delay:.2f}, " + f"randomize_delay={self.randomize_delay!r})" + ) def free_transfer_slots(self) -> int: return self.concurrency - len(self.transferring) @@ -165,6 +172,7 @@ class Downloader: self, request: Request, spider: Spider | None = None ) -> Generator[Deferred[Any], Any, Response | Request]: self.active.add(request) + rough_size = self.middleware._count_rough_size(request) try: result: Response | Request = yield ( deferred_from_coro( @@ -174,6 +182,7 @@ class Downloader: return result finally: self.active.remove(request) + self.middleware._discount_rough_size(rough_size) def _record_backout(self, reason): last_reason, last_reason_start_time = self._last_backout @@ -194,15 +203,16 @@ class Downloader: return True if ( self._response_max_active_size - and self.middleware.response_active_size >= self._response_max_active_size + and self.middleware.total_active_size >= self._response_max_active_size ): if not self._response_max_active_size_warned: self._response_max_active_size_warned = True logger.info( f"The active response size, i.e. the total size of all " f"bodies from responses that have been processed by " - f"downloader middlewares and remain in memory, is " - f"{self.middleware.response_active_size} B. The " + f"downloader middlewares and remain in memory, plus the " + f"rough sizes of in-flight requests, is " + f"{self.middleware.total_active_size} B. The " f"RESPONSE_MAX_ACTIVE_SIZE setting sets its maximum value " f"at {self._response_max_active_size} B. No more requests " f"will be processed until active response size lowers. If " @@ -217,7 +227,7 @@ class Downloader: f"message will only appear the first time this happens. " f"To learn how often request processing has been paused " f"during a crawl for this reason, see the " - f"request_backouts/response_max_active_size stat." + f"request_backout_seconds/response_max_active_size stat." ) self._record_backout("response_max_active_size") # Force the garbage collection of response objects. Necessary for diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 2c6e1f278..560456bde 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -39,6 +39,11 @@ class DownloaderMiddlewareManager(MiddlewareManager): super().__init__(*args, **kwargs) self.response_active_size = 0 self._tracked_responses = WeakSet() + self._rough_active_size = 0 + assert self.crawler is not None + self._response_rough_size: int = self.crawler.settings.getint( + "RESPONSE_ROUGH_SIZE" + ) @classmethod def _get_mwlist_from_settings(cls, settings: BaseSettings) -> list[Any]: @@ -57,6 +62,19 @@ class DownloaderMiddlewareManager(MiddlewareManager): self.methods["process_exception"].appendleft(mw.process_exception) self._check_mw_method_spider_arg(mw.process_exception) + @property + def total_active_size(self) -> int: + """Sum of sizes of tracked responses and rough sizes of in-flight requests.""" + return self.response_active_size + self._rough_active_size + + def _count_rough_size(self, request: Request) -> int: + size: int = request.meta.get("response_rough_size", self._response_rough_size) + self._rough_active_size += size + return size + + def _discount_rough_size(self, size: int) -> None: + self._rough_active_size -= size + def _count_response_size(self, response: Response) -> None: if response in self._tracked_responses: return diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 790423d89..93b87917b 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -176,6 +176,8 @@ __all__ = [ "REFERRER_POLICIES", "REFERRER_POLICY", "REQUEST_FINGERPRINTER_CLASS", + "RESPONSE_MAX_ACTIVE_SIZE", + "RESPONSE_ROUGH_SIZE", "RETRY_ENABLED", "RETRY_EXCEPTIONS", "RETRY_GIVE_UP_LOG_LEVEL", @@ -536,6 +538,7 @@ SCHEDULER_START_MEMORY_QUEUE = "scrapy.squeues.FifoMemoryQueue" SCRAPER_SLOT_MAX_ACTIVE_SIZE = 5_000_000 RESPONSE_MAX_ACTIVE_SIZE = 5_000_000 +RESPONSE_ROUGH_SIZE = 1024 SPIDER_CONTRACTS = {} SPIDER_CONTRACTS_BASE = { diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 15485f6cc..14c49929e 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -15,10 +15,7 @@ from scrapy.utils.test import get_crawler class SlotTest(unittest.TestCase): def test_repr(self): slot = Slot(concurrency=8, delay=0.1, randomize_delay=True) - assert ( - repr(slot) - == "Slot(concurrency=8, delay=0.10, randomize_delay=True, throttle=None)" - ) + assert repr(slot) == "Slot(concurrency=8, delay=0.10, randomize_delay=True)" class OfflineSpider(Spider): @@ -151,6 +148,107 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): ) +class ResponseRoughSizeTest(unittest.TestCase): + @pytest.fixture(autouse=True) + def use_caplog(self, caplog): + self.caplog = caplog + + @deferred_f_from_coro_f + async def test_default(self): + """A crawl without custom settings has RESPONSE_ROUGH_SIZE set to 1024.""" + crawler = get_crawler(OfflineSpider) + with warnings.catch_warnings(): + warnings.simplefilter("error") + await maybe_deferred_to_future(crawler.crawl()) + assert crawler.engine.downloader.middleware._response_rough_size == 1024 + + @deferred_f_from_coro_f + async def test_custom(self): + """Setting RESPONSE_ROUGH_SIZE to a custom value changes the rough size.""" + crawler = get_crawler(OfflineSpider, settings_dict={"RESPONSE_ROUGH_SIZE": 0}) + with warnings.catch_warnings(): + warnings.simplefilter("error") + await maybe_deferred_to_future(crawler.crawl()) + assert crawler.engine.downloader.middleware._response_rough_size == 0 + + @deferred_f_from_coro_f + async def test_rough_size_per_request(self): + """response_rough_size meta key overrides RESPONSE_ROUGH_SIZE per request. + + A low RESPONSE_MAX_ACTIVE_SIZE is set so that only requests with the + custom rough size of 1 pass; without the override the default 1024 would + trigger backout and only one request would be downloaded.""" + + class TestSpider(Spider): + name = "test" + custom_settings = { + "RESPONSE_MAX_ACTIVE_SIZE": 512, + } + + async def start(self): + yield Request("data:,a", meta={"response_rough_size": 1}) + yield Request("data:,b") + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await maybe_deferred_to_future(crawler.crawl()) + + active_size_log_count = sum( + 1 + for r in self.caplog.records + if str(r.msg).startswith("The active response size") + and r.levelname == "INFO" + ) + assert active_size_log_count == 1 + + @deferred_f_from_coro_f + async def test_rough_size_triggers_backout(self): + """Rough sizes of in-flight requests count toward the backpressure limit. + + With RESPONSE_MAX_ACTIVE_SIZE=512 and RESPONSE_ROUGH_SIZE=1024, even a + response with an empty body should trigger the backout log.""" + + class TestSpider(Spider): + name = "test" + start_urls = ["data:,", "data:,"] + custom_settings = { + "RESPONSE_MAX_ACTIVE_SIZE": 512, + "RESPONSE_ROUGH_SIZE": 1024, + } + + def parse(self, response): + pass + + crawler = get_crawler(TestSpider) + self.caplog.clear() + with self.caplog.at_level("INFO"): + await maybe_deferred_to_future(crawler.crawl()) + + matching_log_count = 0 + for log_record in self.caplog.records: + if ( + str(log_record.msg).startswith("The active response size") + and log_record.levelname == "INFO" + ): + matching_log_count += 1 + assert matching_log_count == 1 + + expected_stats = { + "request_backout_seconds/response_max_active_size": gt(0), + "request_backout_seconds/total": gt(0), + } + actual_stats = { + k: v + for k, v in crawler.stats.get_stats().items() + if k.startswith("request_backout_seconds/") + } + assert expected_stats == actual_stats + + class RequestBackoutTest(unittest.TestCase): @pytest.fixture(autouse=True) def use_caplog(self, caplog): From d10fd9224503431930381a53727e32fc0790a865 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 15 Jun 2026 14:32:37 +0200 Subject: [PATCH 26/42] Set a more realistic RESPONSE_ROUGH_SIZE --- docs/topics/settings.rst | 2 +- scrapy/settings/default_settings.py | 2 +- tests/test_downloader.py | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 70ef3f4cf..c00cb1d76 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1759,7 +1759,7 @@ the course of a crawl. RESPONSE_ROUGH_SIZE ------------------- -Default: ``1024`` +Default: ``131072`` (128 kiB) Estimated size (in bytes) to count toward :setting:`RESPONSE_MAX_ACTIVE_SIZE` for each request that is currently being downloaded, before its actual response diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 93b87917b..1f6ffc902 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -538,7 +538,7 @@ SCHEDULER_START_MEMORY_QUEUE = "scrapy.squeues.FifoMemoryQueue" SCRAPER_SLOT_MAX_ACTIVE_SIZE = 5_000_000 RESPONSE_MAX_ACTIVE_SIZE = 5_000_000 -RESPONSE_ROUGH_SIZE = 1024 +RESPONSE_ROUGH_SIZE = 131072 SPIDER_CONTRACTS = {} SPIDER_CONTRACTS_BASE = { diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 14c49929e..1469517d5 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -155,12 +155,11 @@ class ResponseRoughSizeTest(unittest.TestCase): @deferred_f_from_coro_f async def test_default(self): - """A crawl without custom settings has RESPONSE_ROUGH_SIZE set to 1024.""" crawler = get_crawler(OfflineSpider) with warnings.catch_warnings(): warnings.simplefilter("error") await maybe_deferred_to_future(crawler.crawl()) - assert crawler.engine.downloader.middleware._response_rough_size == 1024 + assert crawler.engine.downloader.middleware._response_rough_size == 131072 @deferred_f_from_coro_f async def test_custom(self): @@ -176,8 +175,8 @@ class ResponseRoughSizeTest(unittest.TestCase): """response_rough_size meta key overrides RESPONSE_ROUGH_SIZE per request. A low RESPONSE_MAX_ACTIVE_SIZE is set so that only requests with the - custom rough size of 1 pass; without the override the default 1024 would - trigger backout and only one request would be downloaded.""" + custom rough size of 1 pass; without the override the default 131072 + would trigger backout and only one request would be downloaded.""" class TestSpider(Spider): name = "test" From 4d74e47cfa0f86a6be48176b51d289846af1912d Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 15 Jun 2026 15:01:41 +0200 Subject: [PATCH 27/42] Address typing issues --- scrapy/_classutilities.py | 27 +++++++++++++++------------ scrapy/core/downloader/__init__.py | 11 ++++++----- scrapy/core/downloader/middleware.py | 4 ++-- scrapy/core/scraper.py | 14 +++++++------- tests/test_downloader.py | 2 +- 5 files changed, 31 insertions(+), 27 deletions(-) diff --git a/scrapy/_classutilities.py b/scrapy/_classutilities.py index 6a7120081..720c1365e 100644 --- a/scrapy/_classutilities.py +++ b/scrapy/_classutilities.py @@ -1,5 +1,5 @@ # https://github.com/david-salac/classutilities/issues/1 -# Unmodified copy of +# Modified copy of # https://github.com/david-salac/classutilities/blob/a6e4a86331936d432afaa454ed4c963528165a61/src/classutilities/classproperty.py # Allows creating a class level property @@ -22,37 +22,37 @@ class ClassPropertyContainer: self.prop_get: Any = prop_get self.prop_set: Any = prop_set - def __get__(self, obj: Any, cls: type | None = None) -> Callable: + def __get__(self, obj: Any, cls: type | None = None) -> Any: """ - Get the property getter. + Return the value of the class property. :param obj: Instance of the class. :param cls: Type of the class. - :return: Class property getter. + :return: Value of the class property. """ if cls is None: cls = type(obj) return self.prop_get.__get__(obj, cls)() - def __set__(self, obj, value) -> Callable: + def __set__(self, obj: Any, value: Any) -> None: """ - Get the property setter. + Set the value of the class property. :param obj: Instance of the class. :param value: A value to be set. - :return: Class property setter. """ if not self.prop_set: raise AttributeError("cannot set attribute") _type: type = type(obj) if _type == ClassPropertyMetaClass: _type = obj - return self.prop_set.__get__(obj, _type)(value) + self.prop_set.__get__(obj, _type)(value) def setter( - self, func: Callable | classmethod | staticmethod + self, + func: Callable[..., Any] | classmethod[Any, Any, Any] | staticmethod[Any, Any], ) -> "ClassPropertyContainer": """ Allows creating setter in a property like way. - :param func: Getter function. + :param func: Setter function. :return: Setter object for the decorator. """ if not isinstance(func, (classmethod, staticmethod)): @@ -61,7 +61,9 @@ class ClassPropertyContainer: return self -def classproperty(func): +def classproperty( + func: Callable[..., Any] | classmethod[Any, Any, Any] | staticmethod[Any, Any], +) -> "ClassPropertyContainer": """ Create a decorator for a class level property. :param func: This class method is decorated. @@ -78,8 +80,9 @@ class ClassPropertyMetaClass(type): Metaclass that allows creating a standard setter. """ - def __setattr__(cls, key, value): + def __setattr__(cls, key: str, value: Any) -> None: """Overloads setter for class""" + obj = None if key in cls.__dict__: obj = cls.__dict__.get(key) if obj and isinstance(obj, ClassPropertyContainer): diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 9c6bc27bd..a1fab7756 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -11,9 +11,9 @@ from typing import TYPE_CHECKING, Any from warnings import warn try: - from win_precise_time import time + from win_precise_time import time # type: ignore[import-not-found] except ImportError: - from time import time + from time import time # pylint: disable=ungrouped-imports from twisted.internet.defer import Deferred, inlineCallbacks from twisted.python.failure import Failure @@ -139,7 +139,7 @@ class Downloader: "DOWNLOAD_SLOTS" ) self._stats = crawler.stats - self._last_backout = (None, None) + self._last_backout: tuple[str | None, float | None] = (None, None) deprecated_setting_priority = self.settings.getpriority( "SCRAPER_SLOT_MAX_ACTIVE_SIZE" @@ -184,12 +184,13 @@ class Downloader: self.active.remove(request) self.middleware._discount_rough_size(rough_size) - def _record_backout(self, reason): + def _record_backout(self, reason: str | None) -> None: last_reason, last_reason_start_time = self._last_backout if last_reason == reason: return current_time = time() - if last_reason is not None: + if last_reason is not None and self._stats is not None: + assert last_reason_start_time is not None last_reason_seconds = current_time - last_reason_start_time self._stats.inc_value("request_backout_seconds/total", last_reason_seconds) self._stats.inc_value( diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 560456bde..0a126191d 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -35,10 +35,10 @@ if TYPE_CHECKING: class DownloaderMiddlewareManager(MiddlewareManager): component_name = "downloader middleware" - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) self.response_active_size = 0 - self._tracked_responses = WeakSet() + self._tracked_responses: WeakSet[Response] = WeakSet() self._rough_active_size = 0 assert self.crawler is not None self._response_rough_size: int = self.crawler.settings.getint( diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 1735c67ba..ffa24b154 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -65,7 +65,7 @@ class Slot(ClassPropertiesMixin): _MIN_RESPONSE_SIZE = 1024 @classproperty - def MIN_RESPONSE_SIZE(cls): + def MIN_RESPONSE_SIZE(cls): # pylint: disable=no-self-argument warnings.warn( "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", ScrapyDeprecationWarning, @@ -74,7 +74,7 @@ class Slot(ClassPropertiesMixin): return cls._MIN_RESPONSE_SIZE @MIN_RESPONSE_SIZE.setter # type: ignore[no-redef] - def MIN_RESPONSE_SIZE(cls, value): + def MIN_RESPONSE_SIZE(cls, value): # pylint: disable=no-self-argument warnings.warn( "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated.", ScrapyDeprecationWarning, @@ -95,7 +95,7 @@ class Slot(ClassPropertiesMixin): ScrapyDeprecationWarning, stacklevel=2, ) - self._max_active_size = max_active_size + self._max_active_size: int = max_active_size self.queue: deque[QueueTuple] = deque() self.active: set[Request] = set() self.itemproc_size: int = 0 # just for scrapy.utils.engine.get_engine_status() @@ -103,7 +103,7 @@ class Slot(ClassPropertiesMixin): self._active_size: int = 0 @property - def active_size(self): + def active_size(self) -> int: warnings.warn( ( "scrapy.core.scraper.Slot.active_size is deprecated. Read " @@ -116,7 +116,7 @@ class Slot(ClassPropertiesMixin): return self._active_size @active_size.setter - def active_size(self, value): + def active_size(self, value: int) -> None: warnings.warn( ( "scrapy.core.scraper.Slot.active_size is deprecated. " @@ -134,7 +134,7 @@ class Slot(ClassPropertiesMixin): self._active_size = value @property - def max_active_size(self): + def max_active_size(self) -> int: warnings.warn( ( "scrapy.core.scraper.Slot.max_active_size is deprecated. Read " @@ -146,7 +146,7 @@ class Slot(ClassPropertiesMixin): return self._max_active_size @max_active_size.setter - def max_active_size(self, value): + def max_active_size(self, value: int) -> None: warnings.warn( ( "scrapy.core.scraper.Slot.max_active_size is deprecated. Set " diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 1469517d5..1195fd6ed 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -27,7 +27,7 @@ class OfflineSpider(Spider): class gt: - __hash__ = None + __hash__ = None # type: ignore[assignment] def __init__(self, value): self.value = value From d7a11e587539b7a1466a6d3db4a64b40ae5cab0a Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 15 Jun 2026 15:15:56 +0200 Subject: [PATCH 28/42] Only install win-precise-time in extra-deps --- tox.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index d01aacb89..77eb2dd37 100644 --- a/tox.ini +++ b/tox.ini @@ -47,7 +47,6 @@ deps = sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 testfixtures pywin32; sys_platform == "win32" - win-precise-time; sys_platform == "win32" pytest-twisted >= 1.14.3 [testenv] @@ -179,6 +178,7 @@ deps = ipython robotexclusionrulesparser uvloop; platform_system != "Windows" and implementation_name != "pypy" + win-precise-time; sys_platform == "win32" zstandard; implementation_name != "pypy" # optional for HTTP compress downloader middleware tests [testenv:min-extra-deps] @@ -196,6 +196,7 @@ deps = ipython==7.1.0 robotexclusionrulesparser==1.6.2 uvloop==0.16.0; platform_system != "Windows" and implementation_name != "pypy" + win-precise-time; sys_platform == "win32" zstandard==0.16.0; implementation_name != "pypy" setenv = {[min]setenv} From b8d47f74c1668d543b71499c518fb39a902535be Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 09:20:28 +0200 Subject: [PATCH 29/42] Address typing issues --- scrapy/_classutilities.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/scrapy/_classutilities.py b/scrapy/_classutilities.py index 720c1365e..7f82cd2cf 100644 --- a/scrapy/_classutilities.py +++ b/scrapy/_classutilities.py @@ -3,8 +3,12 @@ # https://github.com/david-salac/classutilities/blob/a6e4a86331936d432afaa454ed4c963528165a61/src/classutilities/classproperty.py # Allows creating a class level property -from collections.abc import Callable -from typing import Any +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from collections.abc import Callable class ClassPropertyContainer: @@ -49,7 +53,7 @@ class ClassPropertyContainer: def setter( self, func: Callable[..., Any] | classmethod[Any, Any, Any] | staticmethod[Any, Any], - ) -> "ClassPropertyContainer": + ) -> ClassPropertyContainer: """ Allows creating setter in a property like way. :param func: Setter function. @@ -63,7 +67,7 @@ class ClassPropertyContainer: def classproperty( func: Callable[..., Any] | classmethod[Any, Any, Any] | staticmethod[Any, Any], -) -> "ClassPropertyContainer": +) -> ClassPropertyContainer: """ Create a decorator for a class level property. :param func: This class method is decorated. From 95084bb609b7864296a64d388ec27e8eb8f548ba Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 10:45:14 +0200 Subject: [PATCH 30/42] Solve test issues? --- tests/test_scraper.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/test_scraper.py b/tests/test_scraper.py index 26347c091..3452390a3 100644 --- a/tests/test_scraper.py +++ b/tests/test_scraper.py @@ -10,14 +10,6 @@ from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.test import get_crawler -class DummySpider(Spider): - name = "test" - start_urls = ["data:,"] - - def parse(self, response): - pass - - class ScraperTest(unittest.TestCase): @deferred_f_from_coro_f async def test_crawl(self): @@ -43,6 +35,8 @@ class ScraperTest(unittest.TestCase): expected = crawler.engine.scraper.slot.MIN_RESPONSE_SIZE assert outcome["active_size"] == expected + +class TestSlot: def test_min_response_time_read(self): slot = Slot() with pytest.warns(ScrapyDeprecationWarning) as warning_messages: From d13cd45da200b7cab682f1abe6c6d935c7bed565 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 11:19:11 +0200 Subject: [PATCH 31/42] Blind attempt number 2 --- tests/test_scraper.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_scraper.py b/tests/test_scraper.py index 3452390a3..54331f9c8 100644 --- a/tests/test_scraper.py +++ b/tests/test_scraper.py @@ -26,7 +26,9 @@ class ScraperTest(unittest.TestCase): self.crawler.engine.scraper.slot.active_size ) - crawler = get_crawler(TestSpider) + crawler = get_crawler( + TestSpider, settings_dict={"TELNETCONSOLE_ENABLED": False} + ) with warnings.catch_warnings(): warnings.simplefilter("error") await maybe_deferred_to_future(crawler.crawl()) From 208ea7a39e556515577de55e33128adfdf6f5d7f Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 11:55:21 +0200 Subject: [PATCH 32/42] Let Opus do its thing --- tests/test_core_downloader.py | 8 +-- tests/test_downloader.py | 97 ++++++++++++++++++----------------- tests/test_scraper.py | 10 ++-- 3 files changed, 56 insertions(+), 59 deletions(-) diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index e348bfb7f..d644c07ea 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -14,7 +14,7 @@ from twisted.web import server, static from twisted.web.client import Agent, BrowserLikePolicyForHTTPS, readBody from twisted.web.client import Response as TxResponse -from scrapy.core.downloader import Downloader, Slot, tls +from scrapy.core.downloader import Downloader, tls from scrapy.core.downloader.contextfactory import ( _load_context_factory_from_settings, _ScrapyClientContextFactory, @@ -39,12 +39,6 @@ if TYPE_CHECKING: from twisted.web.iweb import IBodyProducer -class TestSlot: - def test_repr(self): - slot = Slot(concurrency=8, delay=0.1, randomize_delay=True) - assert repr(slot) == "Slot(concurrency=8, delay=0.1, randomize_delay=True)" - - @pytest.mark.requires_reactor # this test is related to the Twisted HTTP code class TestContextFactoryBase: @async_yield_fixture diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 1195fd6ed..2808fdc2e 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -2,17 +2,17 @@ import warnings import pytest from twisted.internet.defer import Deferred -from twisted.trial import unittest from scrapy import Request, Spider from scrapy.core.downloader import Slot from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Response -from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future +from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.test import get_crawler +from tests.utils.decorators import coroutine_test -class SlotTest(unittest.TestCase): +class TestSlot: def test_repr(self): slot = Slot(concurrency=8, delay=0.1, randomize_delay=True) assert repr(slot) == "Slot(concurrency=8, delay=0.10, randomize_delay=True)" @@ -26,6 +26,25 @@ class OfflineSpider(Spider): pass +def _assert_scraper_slot_deprecation(warning_messages): + """Assert that a crawl emitted exactly one Scrapy deprecation warning, the + one about SCRAPER_SLOT_MAX_ACTIVE_SIZE. + + Only Scrapy deprecation warnings are counted: a crawl may emit unrelated + warnings (e.g. a ResourceWarning for a socket garbage-collected while the + recorder is active), and those must not make the assertion flaky.""" + deprecations = [ + message + for message in warning_messages + if issubclass(message.category, ScrapyDeprecationWarning) + ] + assert len(deprecations) == 1 + assert str(deprecations[0].message) == ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " + "RESPONSE_MAX_ACTIVE_SIZE instead." + ) + + class gt: __hash__ = None # type: ignore[assignment] @@ -39,18 +58,18 @@ class gt: return f">{self.value}" -class ResponseMaxActiveSizeTest(unittest.TestCase): - @deferred_f_from_coro_f +class TestResponseMaxActiveSize: + @coroutine_test async def test_default(self): """A crawl without custom settings has its effective response max active size set to 5 000 000, and triggers no deprecation warning.""" crawler = get_crawler(OfflineSpider) with warnings.catch_warnings(): - warnings.simplefilter("error") + warnings.simplefilter("error", ScrapyDeprecationWarning) await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader._response_max_active_size == 5_000_000 - @deferred_f_from_coro_f + @coroutine_test async def test_custom(self): """Setting RESPONSE_MAX_ACTIVE_SIZE to a custom value changes the effective response max active size.""" @@ -58,11 +77,11 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): OfflineSpider, settings_dict={"RESPONSE_MAX_ACTIVE_SIZE": 0} ) with warnings.catch_warnings(): - warnings.simplefilter("error") + warnings.simplefilter("error", ScrapyDeprecationWarning) await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader._response_max_active_size == 0 - @deferred_f_from_coro_f + @coroutine_test async def test_deprecated_default(self): """Setting SCRAPER_SLOT_MAX_ACTIVE_SIZE triggers a deprecation warning, even if it is the default value.""" @@ -72,13 +91,9 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): with pytest.warns(ScrapyDeprecationWarning) as warning_messages: await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader._response_max_active_size == 5_000_000 - assert len(warning_messages) == 1 - assert str(warning_messages[0].message) == ( - "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " - "RESPONSE_MAX_ACTIVE_SIZE instead." - ) + _assert_scraper_slot_deprecation(warning_messages) - @deferred_f_from_coro_f + @coroutine_test async def test_deprecated_custom(self): """Setting SCRAPER_SLOT_MAX_ACTIVE_SIZE to a custom value triggers a deprecation warning, and changes the effective response max active @@ -89,13 +104,9 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): with pytest.warns(ScrapyDeprecationWarning) as warning_messages: await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader._response_max_active_size == 0 - assert len(warning_messages) == 1 - assert str(warning_messages[0].message) == ( - "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " - "RESPONSE_MAX_ACTIVE_SIZE instead." - ) + _assert_scraper_slot_deprecation(warning_messages) - @deferred_f_from_coro_f + @coroutine_test async def test_both(self): """Setting RESPONSE_MAX_ACTIVE_SIZE and SCRAPER_SLOT_MAX_ACTIVE_SIZE to different values with the same setting priority triggers a deprecation @@ -111,13 +122,9 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): with pytest.warns(ScrapyDeprecationWarning) as warning_messages: await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader._response_max_active_size == 1 - assert len(warning_messages) == 1 - assert str(warning_messages[0].message) == ( - "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " - "RESPONSE_MAX_ACTIVE_SIZE instead." - ) + _assert_scraper_slot_deprecation(warning_messages) - @deferred_f_from_coro_f + @coroutine_test async def test_both_deprecated_priority(self): """Setting RESPONSE_MAX_ACTIVE_SIZE and SCRAPER_SLOT_MAX_ACTIVE_SIZE to different values and SCRAPER_SLOT_MAX_ACTIVE_SIZE with a higher @@ -141,36 +148,32 @@ class ResponseMaxActiveSizeTest(unittest.TestCase): with pytest.warns(ScrapyDeprecationWarning) as warning_messages: await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader._response_max_active_size == 2 - assert len(warning_messages) == 1 - assert str(warning_messages[0].message) == ( - "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " - "RESPONSE_MAX_ACTIVE_SIZE instead." - ) + _assert_scraper_slot_deprecation(warning_messages) -class ResponseRoughSizeTest(unittest.TestCase): +class TestResponseRoughSize: @pytest.fixture(autouse=True) def use_caplog(self, caplog): self.caplog = caplog - @deferred_f_from_coro_f + @coroutine_test async def test_default(self): crawler = get_crawler(OfflineSpider) with warnings.catch_warnings(): - warnings.simplefilter("error") + warnings.simplefilter("error", ScrapyDeprecationWarning) await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader.middleware._response_rough_size == 131072 - @deferred_f_from_coro_f + @coroutine_test async def test_custom(self): """Setting RESPONSE_ROUGH_SIZE to a custom value changes the rough size.""" crawler = get_crawler(OfflineSpider, settings_dict={"RESPONSE_ROUGH_SIZE": 0}) with warnings.catch_warnings(): - warnings.simplefilter("error") + warnings.simplefilter("error", ScrapyDeprecationWarning) await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader.middleware._response_rough_size == 0 - @deferred_f_from_coro_f + @coroutine_test async def test_rough_size_per_request(self): """response_rough_size meta key overrides RESPONSE_ROUGH_SIZE per request. @@ -204,7 +207,7 @@ class ResponseRoughSizeTest(unittest.TestCase): ) assert active_size_log_count == 1 - @deferred_f_from_coro_f + @coroutine_test async def test_rough_size_triggers_backout(self): """Rough sizes of in-flight requests count toward the backpressure limit. @@ -248,12 +251,12 @@ class ResponseRoughSizeTest(unittest.TestCase): assert expected_stats == actual_stats -class RequestBackoutTest(unittest.TestCase): +class TestRequestBackout: @pytest.fixture(autouse=True) def use_caplog(self, caplog): self.caplog = caplog - @deferred_f_from_coro_f + @coroutine_test async def test_none(self): class TestSpider(Spider): @@ -284,7 +287,7 @@ class RequestBackoutTest(unittest.TestCase): } assert stats == {} - @deferred_f_from_coro_f + @coroutine_test async def test_concurrency(self): class SlowDown: @@ -335,7 +338,7 @@ class RequestBackoutTest(unittest.TestCase): } assert expected_stats == actual_stats - @deferred_f_from_coro_f + @coroutine_test async def test_response_size(self): class TestSpider(Spider): @@ -373,7 +376,7 @@ class RequestBackoutTest(unittest.TestCase): } assert expected_stats == actual_stats - @deferred_f_from_coro_f + @coroutine_test async def test_response_size_process_request(self): class DownloaderMiddleware: @@ -416,7 +419,7 @@ class RequestBackoutTest(unittest.TestCase): } assert expected_stats == actual_stats - @deferred_f_from_coro_f + @coroutine_test async def test_response_size_process_response(self): class DownloaderMiddleware: @@ -459,7 +462,7 @@ class RequestBackoutTest(unittest.TestCase): } assert expected_stats == actual_stats - @deferred_f_from_coro_f + @coroutine_test async def test_response_size_process_exception(self): class DownloaderMiddleware1: @@ -509,7 +512,7 @@ class RequestBackoutTest(unittest.TestCase): } assert expected_stats == actual_stats - @deferred_f_from_coro_f + @coroutine_test async def test_response_size_download(self): """Ensure that responses from engine.download calls are also taken into account for the RESPONSE_MAX_ACTIVE_SIZE setting.""" diff --git a/tests/test_scraper.py b/tests/test_scraper.py index 54331f9c8..523127ff3 100644 --- a/tests/test_scraper.py +++ b/tests/test_scraper.py @@ -1,17 +1,17 @@ import warnings import pytest -from twisted.trial import unittest from scrapy import Spider from scrapy.core.scraper import Slot from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future +from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.test import get_crawler +from tests.utils.decorators import coroutine_test -class ScraperTest(unittest.TestCase): - @deferred_f_from_coro_f +class TestScraper: + @coroutine_test async def test_crawl(self): """A crawl should not trigger any deprecation warning.""" outcome = {} @@ -30,7 +30,7 @@ class ScraperTest(unittest.TestCase): TestSpider, settings_dict={"TELNETCONSOLE_ENABLED": False} ) with warnings.catch_warnings(): - warnings.simplefilter("error") + warnings.simplefilter("error", ScrapyDeprecationWarning) await maybe_deferred_to_future(crawler.crawl()) with pytest.warns(ScrapyDeprecationWarning): From b1ccb277dad54287d4e9bcde9113a1bf429eea81 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 12:40:20 +0200 Subject: [PATCH 33/42] Make _schedule_coro() behavior the same in asyncio and non-asyncio --- scrapy/utils/defer.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 29a34d4ef..699b51505 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -531,9 +531,14 @@ def _schedule_coro(coro: Coroutine[Any, Any, Any]) -> None: alternative is calling :func:`scrapy.utils.defer.deferred_from_coro`, keeping the result, and adding proper exception handling (e.g. errbacks) to it. + + In both asyncio and non-asyncio modes the coroutine starts in the next + event-loop iteration, never in the current call stack. """ if not is_asyncio_available(): - Deferred.fromCoroutine(coro) + from twisted.internet import reactor + + reactor.callLater(0, Deferred.fromCoroutine, coro) return loop = asyncio.get_event_loop() loop.create_task(coro) # noqa: RUF006 From 464aa5b18561bebc70d6082275ece884096b5c67 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 16:27:26 +0200 Subject: [PATCH 34/42] Update a test that relied on the old behavior of _schedule_coro and would hang now --- tests/test_engine.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/test_engine.py b/tests/test_engine.py index e51eb4664..9bc0b4cb8 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -26,11 +26,7 @@ from scrapy.item import Field, Item from scrapy.linkextractors import LinkExtractor from scrapy.spiders import Spider from scrapy.statscollectors import MemoryStatsCollector -from scrapy.utils.defer import ( - _schedule_coro, - deferred_from_coro, - maybe_deferred_to_future, -) +from scrapy.utils.defer import deferred_from_coro, maybe_deferred_to_future from scrapy.utils.signal import disconnect_all from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler @@ -446,10 +442,11 @@ class TestEngine(TestEngineBase): e = ExecutionEngine(crawler, lambda _: None) crawler.engine = e yield deferred_from_coro(e.open_spider_async()) - _schedule_coro(e.start_async()) + start_deferred = deferred_from_coro(e.start_async()) with pytest.raises(RuntimeError, match="Engine already running"): yield deferred_from_coro(e.start_async()) yield deferred_from_coro(e.stop_async()) + yield start_deferred @pytest.mark.only_asyncio @coroutine_test From 0c326fdc031c7f4a4d5b7f32f6ea1b852489176f Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 16:57:48 +0200 Subject: [PATCH 35/42] Improve documentation wording --- docs/topics/practices.rst | 1 - docs/topics/settings.rst | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index 5253c1ed4..c54d0c85f 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -433,7 +433,6 @@ In this case, the spider spent about 45 seconds paused due to large responses in memory. You could: - Increase :setting:`RESPONSE_MAX_ACTIVE_SIZE` if your machine has enough RAM. -- Reduce :setting:`CONCURRENT_REQUESTS` to let responses be processed faster. - Check that your code doesn't hold strong references to :class:`~scrapy.http.Response` objects longer than necessary. diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index c00cb1d76..0fa2229a8 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1723,8 +1723,8 @@ This counts both the size of response bodies that have passed through memory, and the :setting:`rough size ` of requests currently being downloaded. -When the total exceeds this value, Scrapy pauses scheduling new requests until -it drops below the limit. +When the total exceeds this value, Scrapy pauses sending new requests to the +downloader until it drops below the limit. If you set this to ``0``, the limit is disabled. From 03081abcf66f49e4c83090f3bef3f8dd627ef2a4 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 17:27:13 +0200 Subject: [PATCH 36/42] Remove win-precise-time support, probably not worth it for this use case --- docs/topics/practices.rst | 10 ---------- scrapy/core/downloader/__init__.py | 7 +------ tox.ini | 2 -- 3 files changed, 1 insertion(+), 18 deletions(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index c54d0c85f..1721a752f 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -436,16 +436,6 @@ in memory. You could: - Check that your code doesn't hold strong references to :class:`~scrapy.http.Response` objects longer than necessary. -.. note:: - - On Windows, Scrapy uses the `win-precise-time - `_ package when available to - measure backout durations with higher precision. Install it with: - - .. code-block:: shell - - pip install win-precise-time - .. _bans: Avoiding getting banned diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index a1fab7756..50ccba7d3 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -6,15 +6,10 @@ from collections import deque from dataclasses import dataclass, field from datetime import datetime from logging import getLogger -from time import monotonic +from time import monotonic, time from typing import TYPE_CHECKING, Any from warnings import warn -try: - from win_precise_time import time # type: ignore[import-not-found] -except ImportError: - from time import time # pylint: disable=ungrouped-imports - from twisted.internet.defer import Deferred, inlineCallbacks from twisted.python.failure import Failure diff --git a/tox.ini b/tox.ini index 77eb2dd37..6ee686aaa 100644 --- a/tox.ini +++ b/tox.ini @@ -178,7 +178,6 @@ deps = ipython robotexclusionrulesparser uvloop; platform_system != "Windows" and implementation_name != "pypy" - win-precise-time; sys_platform == "win32" zstandard; implementation_name != "pypy" # optional for HTTP compress downloader middleware tests [testenv:min-extra-deps] @@ -196,7 +195,6 @@ deps = ipython==7.1.0 robotexclusionrulesparser==1.6.2 uvloop==0.16.0; platform_system != "Windows" and implementation_name != "pypy" - win-precise-time; sys_platform == "win32" zstandard==0.16.0; implementation_name != "pypy" setenv = {[min]setenv} From e746df0297b4c71b6e7b24bb9df44cba3dc20559 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 17:31:10 +0200 Subject: [PATCH 37/42] Add a comment about the content of self._last_backout --- scrapy/core/downloader/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 50ccba7d3..d0239414d 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -134,6 +134,8 @@ class Downloader: "DOWNLOAD_SLOTS" ) self._stats = crawler.stats + # (reason, start_time): current backout reason and when it began, or + # (None, None) if not backing out. self._last_backout: tuple[str | None, float | None] = (None, None) deprecated_setting_priority = self.settings.getpriority( From 0292d23d7a0ad89432c17b6920a6c9f7a01aa85a Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 17:46:22 +0200 Subject: [PATCH 38/42] Improve deprecation messaging around SCRAPER_SLOT_MAX_ACTIVE_SIZE --- scrapy/core/downloader/__init__.py | 42 ++++++++++++++++++++---------- tests/test_downloader.py | 31 +++++++++++++++------- 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index d0239414d..23a563d41 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -142,24 +142,38 @@ class Downloader: "SCRAPER_SLOT_MAX_ACTIVE_SIZE" ) assert deprecated_setting_priority is not None - if deprecated_setting_priority > 0: - warn( - ( - "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, " - "use RESPONSE_MAX_ACTIVE_SIZE instead." - ), - ScrapyDeprecationWarning, - stacklevel=2, - ) setting_priority = self.settings.getpriority("RESPONSE_MAX_ACTIVE_SIZE") assert setting_priority is not None - if setting_priority >= deprecated_setting_priority: - self._response_max_active_size = self.settings.getint( - "RESPONSE_MAX_ACTIVE_SIZE" - ) + if deprecated_setting_priority > 0: + if setting_priority >= deprecated_setting_priority: + warn( + ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated " + "and is being ignored because RESPONSE_MAX_ACTIVE_SIZE is " + "set with an equal or higher priority. Remove " + "SCRAPER_SLOT_MAX_ACTIVE_SIZE from your settings." + ), + ScrapyDeprecationWarning, + stacklevel=2, + ) + self._response_max_active_size = self.settings.getint( + "RESPONSE_MAX_ACTIVE_SIZE" + ) + else: + warn( + ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, " + "use RESPONSE_MAX_ACTIVE_SIZE instead." + ), + ScrapyDeprecationWarning, + stacklevel=2, + ) + self._response_max_active_size = self.settings.getint( + "SCRAPER_SLOT_MAX_ACTIVE_SIZE" + ) else: self._response_max_active_size = self.settings.getint( - "SCRAPER_SLOT_MAX_ACTIVE_SIZE" + "RESPONSE_MAX_ACTIVE_SIZE" ) self._response_max_active_size_warned = False diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 2808fdc2e..095873715 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -26,23 +26,35 @@ class OfflineSpider(Spider): pass -def _assert_scraper_slot_deprecation(warning_messages): +def _assert_scraper_slot_deprecation(warning_messages, *, ignored=False): """Assert that a crawl emitted exactly one Scrapy deprecation warning, the one about SCRAPER_SLOT_MAX_ACTIVE_SIZE. Only Scrapy deprecation warnings are counted: a crawl may emit unrelated warnings (e.g. a ResourceWarning for a socket garbage-collected while the - recorder is active), and those must not make the assertion flaky.""" + recorder is active), and those must not make the assertion flaky. + + Pass ``ignored=True`` when RESPONSE_MAX_ACTIVE_SIZE is set with an equal or + higher priority and therefore SCRAPER_SLOT_MAX_ACTIVE_SIZE is being + ignored.""" deprecations = [ message for message in warning_messages if issubclass(message.category, ScrapyDeprecationWarning) ] assert len(deprecations) == 1 - assert str(deprecations[0].message) == ( - "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " - "RESPONSE_MAX_ACTIVE_SIZE instead." - ) + if ignored: + assert str(deprecations[0].message) == ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated and is " + "being ignored because RESPONSE_MAX_ACTIVE_SIZE is set with an " + "equal or higher priority. Remove SCRAPER_SLOT_MAX_ACTIVE_SIZE " + "from your settings." + ) + else: + assert str(deprecations[0].message) == ( + "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use " + "RESPONSE_MAX_ACTIVE_SIZE instead." + ) class gt: @@ -110,8 +122,9 @@ class TestResponseMaxActiveSize: async def test_both(self): """Setting RESPONSE_MAX_ACTIVE_SIZE and SCRAPER_SLOT_MAX_ACTIVE_SIZE to different values with the same setting priority triggers a deprecation - warning about SCRAPER_SLOT_MAX_ACTIVE_SIZE and makes the value of - RESPONSE_MAX_ACTIVE_SIZE the effective response max active size.""" + warning about SCRAPER_SLOT_MAX_ACTIVE_SIZE being ignored, and makes + the value of RESPONSE_MAX_ACTIVE_SIZE the effective response max active + size.""" crawler = get_crawler( OfflineSpider, settings_dict={ @@ -122,7 +135,7 @@ class TestResponseMaxActiveSize: with pytest.warns(ScrapyDeprecationWarning) as warning_messages: await maybe_deferred_to_future(crawler.crawl()) assert crawler.engine.downloader._response_max_active_size == 1 - _assert_scraper_slot_deprecation(warning_messages) + _assert_scraper_slot_deprecation(warning_messages, ignored=True) @coroutine_test async def test_both_deprecated_priority(self): From 1bcb76c7cc2611ac4348acbd4dfa2c6c4728213e Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 18:29:28 +0200 Subject: [PATCH 39/42] Solve typing issues --- scrapy/statscollectors.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/scrapy/statscollectors.py b/scrapy/statscollectors.py index 1f9894519..69108fa32 100644 --- a/scrapy/statscollectors.py +++ b/scrapy/statscollectors.py @@ -69,7 +69,11 @@ class StatsCollector: self._stats = stats def inc_value( - self, key: str, count: int = 1, start: int = 0, spider: Spider | None = None + self, + key: str, + count: float = 1, + start: float = 0, + spider: Spider | None = None, ) -> None: d = self._stats d[key] = d.setdefault(key, start) + count @@ -123,7 +127,11 @@ class DummyStatsCollector(StatsCollector): pass def inc_value( - self, key: str, count: int = 1, start: int = 0, spider: Spider | None = None + self, + key: str, + count: float = 1, + start: float = 0, + spider: Spider | None = None, ) -> None: pass From f0e78aa677e2302310ec88be7b824bea3e3ea78a Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 20:06:32 +0200 Subject: [PATCH 40/42] Improve test reliability --- tests/test_downloader.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 095873715..3890ab5ae 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -306,18 +306,25 @@ class TestRequestBackout: class SlowDown: """Downloader middleware that returns a non-instant deferred from process_request, to force need_backout calls to happen at that - point.""" + point. + + The delay is deliberately non-zero so that the concurrency backout + state lasts a measurable amount of wall-clock time, which keeps the + request_backout_seconds/concurrency stat reliably above 0.""" def process_request(self, request, spider): from twisted.internet import reactor d = Deferred() - reactor.callLater(0, d.callback, None) + reactor.callLater(0.01, d.callback, None) return d class TestSpider(Spider): name = "test" - start_urls = ["data:,"] + # Several start URLs so that, with CONCURRENT_REQUESTS=1, the engine + # reliably attempts to schedule a second request while the first one + # is still active, which is what triggers the concurrency backout. + start_urls = ["data:,"] * 5 custom_settings = { "CONCURRENT_REQUESTS": 1, "DOWNLOADER_MIDDLEWARES": {SlowDown: 0}, From 73e7353c00b99a47a0535a68d4c121df58ee7472 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 21:00:20 +0200 Subject: [PATCH 41/42] Complete test coverage --- scrapy/_classutilities.py | 4 ++-- tests/test_scraper.py | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/scrapy/_classutilities.py b/scrapy/_classutilities.py index 7f82cd2cf..b788898fa 100644 --- a/scrapy/_classutilities.py +++ b/scrapy/_classutilities.py @@ -33,7 +33,7 @@ class ClassPropertyContainer: :param cls: Type of the class. :return: Value of the class property. """ - if cls is None: + if cls is None: # pragma: no cover cls = type(obj) return self.prop_get.__get__(obj, cls)() @@ -43,7 +43,7 @@ class ClassPropertyContainer: :param obj: Instance of the class. :param value: A value to be set. """ - if not self.prop_set: + if not self.prop_set: # pragma: no cover raise AttributeError("cannot set attribute") _type: type = type(obj) if _type == ClassPropertyMetaClass: diff --git a/tests/test_scraper.py b/tests/test_scraper.py index 523127ff3..194aaf0e4 100644 --- a/tests/test_scraper.py +++ b/tests/test_scraper.py @@ -62,6 +62,31 @@ class TestSlot: == "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated." ) + def test_min_response_size_class_read(self): + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + actual = Slot.MIN_RESPONSE_SIZE + assert actual == 1024 + assert len(warning_messages) == 1 + assert ( + str(warning_messages[0].message) + == "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated." + ) + + def test_min_response_size_class_write(self): + original = Slot._MIN_RESPONSE_SIZE + try: + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + Slot.MIN_RESPONSE_SIZE = 0 + with pytest.warns(ScrapyDeprecationWarning): + assert Slot.MIN_RESPONSE_SIZE == 0 + assert len(warning_messages) == 1 + assert ( + str(warning_messages[0].message) + == "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated." + ) + finally: + Slot._MIN_RESPONSE_SIZE = original + def test_slot_init_max_active_size_default(self): with pytest.warns(ScrapyDeprecationWarning) as warning_messages: slot = Slot(max_active_size=5_000_000) From 38fe2751b3a7d129bf4ad0332b16fc2be7af63bd Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 19 Jun 2026 21:27:52 +0200 Subject: [PATCH 42/42] Fix test failure during all-tests run --- tests/test_scraper.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/test_scraper.py b/tests/test_scraper.py index 194aaf0e4..45e076511 100644 --- a/tests/test_scraper.py +++ b/tests/test_scraper.py @@ -39,6 +39,12 @@ class TestScraper: class TestSlot: + def setup_method(self): + self._saved_min_response_size = Slot._MIN_RESPONSE_SIZE + + def teardown_method(self): + Slot._MIN_RESPONSE_SIZE = self._saved_min_response_size + def test_min_response_time_read(self): slot = Slot() with pytest.warns(ScrapyDeprecationWarning) as warning_messages: @@ -73,19 +79,15 @@ class TestSlot: ) def test_min_response_size_class_write(self): - original = Slot._MIN_RESPONSE_SIZE - try: - with pytest.warns(ScrapyDeprecationWarning) as warning_messages: - Slot.MIN_RESPONSE_SIZE = 0 - with pytest.warns(ScrapyDeprecationWarning): - assert Slot.MIN_RESPONSE_SIZE == 0 - assert len(warning_messages) == 1 - assert ( - str(warning_messages[0].message) - == "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated." - ) - finally: - Slot._MIN_RESPONSE_SIZE = original + with pytest.warns(ScrapyDeprecationWarning) as warning_messages: + Slot.MIN_RESPONSE_SIZE = 0 + with pytest.warns(ScrapyDeprecationWarning): + assert Slot.MIN_RESPONSE_SIZE == 0 + assert len(warning_messages) == 1 + assert ( + str(warning_messages[0].message) + == "scrapy.core.scraper.Slot.MIN_RESPONSE_SIZE is deprecated." + ) def test_slot_init_max_active_size_default(self): with pytest.warns(ScrapyDeprecationWarning) as warning_messages: