diff --git a/docs/faq.rst b/docs/faq.rst index 1a574e5da..80658a5bf 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -220,21 +220,15 @@ the :ref:`topics-signals-ref` to know which ones. What does the response status code 999 mean? -------------------------------------------- -999 is a custom response status code used by Yahoo sites to throttle requests. +999 is a custom response status code used by some sites to throttle requests. Try slowing down the crawling speed by using a download delay of ``2`` (or -higher) in your spider: +higher) for the affected domains, with the :setting:`DOWNLOAD_SLOTS` setting: .. code-block:: python - from scrapy.spiders import CrawlSpider - - - class MySpider(CrawlSpider): - name = "myspider" - - download_delay = 2 - - # [ ... rest of the spider code ... ] + DOWNLOAD_SLOTS = { + "example.com": {"delay": 2}, + } Or by setting a global download delay in your project with the :setting:`DOWNLOAD_DELAY` setting. diff --git a/docs/news.rst b/docs/news.rst index 8f8477eaa..670843e0e 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -1417,7 +1417,8 @@ Deprecations - ``download_warnsize`` (use :setting:`DOWNLOAD_WARNSIZE`) - - ``max_concurrent_requests`` (use :setting:`CONCURRENT_REQUESTS`) + - ``max_concurrent_requests`` (use + :setting:`CONCURRENT_REQUESTS_PER_DOMAIN`) - ``user_agent`` (use :setting:`USER_AGENT`) diff --git a/docs/topics/autothrottle.rst b/docs/topics/autothrottle.rst index 4f28019da..33289545c 100644 --- a/docs/topics/autothrottle.rst +++ b/docs/topics/autothrottle.rst @@ -106,10 +106,9 @@ delay of its download slot: Request("https://example.com", meta={"autothrottle_dont_adjust_delay": True}) Note, however, that AutoThrottle still determines the starting delay of every -download slot by setting the ``download_delay`` attribute on the running -spider. If you want AutoThrottle not to impact a download slot at all, in -addition to setting this meta key in all requests that use that download slot, -you might want to set a custom value for the ``delay`` attribute of that +download slot. If you want AutoThrottle not to impact a download slot at all, +in addition to setting this meta key in all requests that use that download +slot, you might want to set a custom value for the ``delay`` attribute of that download slot, e.g. using :setting:`DOWNLOAD_SLOTS`. Settings diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index e287c3bd5..65ee77258 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -953,10 +953,6 @@ desired. .. _spider-download_delay-attribute: -.. note:: - - This delay can be set per spider using :attr:`download_delay` spider attribute. - It is possible to change this setting per domain by using :setting:`DOWNLOAD_SLOTS`. diff --git a/extras/qpsclient.py b/extras/qpsclient.py index 8e5001c1d..efb582254 100644 --- a/extras/qpsclient.py +++ b/extras/qpsclient.py @@ -16,23 +16,20 @@ class QPSSpider(Spider): name = "qps" benchurl = "http://localhost:8880/" - # Max concurrency is limited by global CONCURRENT_REQUESTS setting - max_concurrent_requests = 8 # Requests per second goal - qps = None # same as: 1 / download_delay - download_delay = None + qps = None # same as: 1 / DOWNLOAD_DELAY # time in seconds to delay server responses latency = None # number of slots to create slots = 1 - def __init__(self, *a, **kw): - super().__init__(*a, **kw) - if self.qps is not None: - self.qps = float(self.qps) - self.download_delay = 1 / self.qps - elif self.download_delay is not None: - self.download_delay = float(self.download_delay) + @classmethod + def from_crawler(cls, crawler, *args, **kwargs): + spider = super().from_crawler(crawler, *args, **kwargs) + if spider.qps is not None: + spider.qps = float(spider.qps) + crawler.settings.set("DOWNLOAD_DELAY", 1 / spider.qps, priority="spider") + return spider async def start(self): url = self.benchurl diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index eb2079d0c..f9ee62838 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -27,7 +27,6 @@ from scrapy.utils.defer import ( deferred_from_coro, maybe_deferred_to_future, ) -from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute from scrapy.utils.httpobj import urlparse_cached if TYPE_CHECKING: @@ -80,22 +79,6 @@ class Slot: ) -def _get_concurrency_delay( - concurrency: int, spider: Spider, settings: BaseSettings -) -> tuple[int, float]: - delay: float = settings.getfloat("DOWNLOAD_DELAY") - if hasattr(spider, "download_delay"): - delay = spider.download_delay - - if hasattr(spider, "max_concurrent_requests"): # pragma: no cover - warn_on_deprecated_spider_attribute( - "max_concurrent_requests", "CONCURRENT_REQUESTS" - ) - concurrency = spider.max_concurrent_requests - - return concurrency, delay - - class Downloader: DOWNLOAD_SLOT = "download_slot" _SLOT_GC_INTERVAL: float = 60.0 # seconds @@ -112,6 +95,9 @@ class Downloader: "CONCURRENT_REQUESTS_PER_DOMAIN" ) self.ip_concurrency: int = self.settings.getint("CONCURRENT_REQUESTS_PER_IP") + # Default delay of new slots. AutoThrottle overrides it to apply + # AUTOTHROTTLE_START_DELAY. + self._delay: float = self.settings.getfloat("DOWNLOAD_DELAY") self.randomize_delay: bool = self.settings.getbool("RANDOMIZE_DOWNLOAD_DELAY") self.middleware: DownloaderMiddlewareManager = ( DownloaderMiddlewareManager.from_crawler(crawler) @@ -147,16 +133,11 @@ class Downloader: ) -> tuple[str, Slot]: key = self.get_slot_key(request) if key not in self.slots: - assert self.crawler.spider slot_settings = self.per_slot_settings.get(key, {}) - conc = self.ip_concurrency or self.domain_concurrency - conc, delay = _get_concurrency_delay( - conc, self.crawler.spider, self.settings - ) - conc, delay = ( - slot_settings.get("concurrency", conc), - slot_settings.get("delay", delay), + conc = slot_settings.get( + "concurrency", self.ip_concurrency or self.domain_concurrency ) + delay = slot_settings.get("delay", self._delay) randomize_delay = slot_settings.get("randomize_delay", self.randomize_delay) new_slot = Slot(conc, delay, randomize_delay) self.slots[key] = new_slot diff --git a/scrapy/crawler.py b/scrapy/crawler.py index c8d74fba7..e2f726519 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -100,6 +100,10 @@ class Crawler: return self.addons.load_settings(self.settings) + self._apply_deprecated_spider_attr("download_delay", "DOWNLOAD_DELAY") + self._apply_deprecated_spider_attr( + "max_concurrent_requests", "CONCURRENT_REQUESTS_PER_DOMAIN" + ) self.stats = load_object(self.settings["STATS_CLASS"])(self) lf_cls: type[LogFormatter] = load_object(self.settings["LOG_FORMATTER"]) @@ -155,6 +159,30 @@ class Crawler: "Overridden settings:\n%(settings)s", {"settings": pprint.pformat(d)} ) + def _apply_deprecated_spider_attr(self, attr: str, setting: str) -> None: + """Bridge a deprecated spider attribute onto *setting*, warning about + the deprecation (and about being ignored when *setting* is already set + at spider or higher priority).""" + spider = self.spider if self.spider is not None else self.spidercls + if not hasattr(spider, attr): + return + if (self.settings.getpriority(setting) or 0) >= SETTINGS_PRIORITIES["spider"]: + warnings.warn( + f"The {attr!r} spider attribute is deprecated. It is also being " + f"ignored because {setting} is already set at spider or higher " + f"priority. Remove the {attr!r} attribute from your spider.", + category=ScrapyDeprecationWarning, + stacklevel=3, + ) + return + warnings.warn( + f"The {attr!r} spider attribute is deprecated. Use the {setting} " + f"setting instead.", + category=ScrapyDeprecationWarning, + stacklevel=3, + ) + self.settings.set(setting, getattr(spider, attr), priority="spider") + def _apply_reactorless_default_settings(self) -> None: """Change some setting defaults when not using a Twisted reactor. diff --git a/scrapy/extensions/throttle.py b/scrapy/extensions/throttle.py index 542ff1cdc..cde73f12e 100644 --- a/scrapy/extensions/throttle.py +++ b/scrapy/extensions/throttle.py @@ -43,18 +43,18 @@ class AutoThrottle: return cls(crawler) def _spider_opened(self, spider: Spider) -> None: - self.mindelay = self._min_delay(spider) - self.maxdelay = self._max_delay(spider) - spider.download_delay = self._start_delay(spider) # type: ignore[attr-defined] + self.mindelay = self._min_delay() + self.maxdelay = self._max_delay() + assert self.crawler.engine + self.crawler.engine.downloader._delay = self._start_delay() - def _min_delay(self, spider: Spider) -> float: - s = self.crawler.settings - return getattr(spider, "download_delay", s.getfloat("DOWNLOAD_DELAY")) + def _min_delay(self) -> float: + return self.crawler.settings.getfloat("DOWNLOAD_DELAY") - def _max_delay(self, spider: Spider) -> float: + def _max_delay(self) -> float: return self.crawler.settings.getfloat("AUTOTHROTTLE_MAX_DELAY") - def _start_delay(self, spider: Spider) -> float: + def _start_delay(self) -> float: return max( self.mindelay, self.crawler.settings.getfloat("AUTOTHROTTLE_START_DELAY") ) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index b4f906e25..358f20ed7 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -74,6 +74,39 @@ class TestCrawler: assert not settings.frozen assert crawler.settings.frozen + @pytest.mark.parametrize( + ("attr", "setting"), + [ + ("download_delay", "DOWNLOAD_DELAY"), + ("max_concurrent_requests", "CONCURRENT_REQUESTS_PER_DOMAIN"), + ], + ) + def test_deprecated_spider_attr(self, attr: str, setting: str) -> None: + crawler = get_raw_crawler(type("_Spider", (DefaultSpider,), {attr: 2})) + with pytest.warns( + ScrapyDeprecationWarning, + match=f"The {attr!r} spider attribute is deprecated. Use the {setting} ", + ): + crawler._apply_settings() + assert crawler.settings.getint(setting) == 2 + + @pytest.mark.parametrize( + ("attr", "setting"), + [ + ("download_delay", "DOWNLOAD_DELAY"), + ("max_concurrent_requests", "CONCURRENT_REQUESTS_PER_DOMAIN"), + ], + ) + def test_deprecated_spider_attr_ignored(self, attr: str, setting: str) -> None: + crawler = get_raw_crawler(type("_Spider", (DefaultSpider,), {attr: 2})) + crawler.settings.set(setting, 3, priority="spider") + with pytest.warns( + ScrapyDeprecationWarning, + match=f"The {attr!r} spider attribute is deprecated. It is also being ", + ): + crawler._apply_settings() + assert crawler.settings.getint(setting) == 3 + def test_crawler_accepts_dict(self) -> None: crawler = get_crawler(DefaultSpider, {"foo": "bar"}) assert crawler.settings["foo"] == "bar" diff --git a/tests/test_extension_throttle.py b/tests/test_extension_throttle.py index 4874f284a..2c718d95f 100644 --- a/tests/test_extension_throttle.py +++ b/tests/test_extension_throttle.py @@ -3,7 +3,7 @@ from unittest.mock import Mock import pytest -from scrapy import Request, Spider +from scrapy import Request from scrapy.exceptions import NotConfigured from scrapy.extensions.throttle import AutoThrottle from scrapy.http.response import Response @@ -25,6 +25,13 @@ def get_crawler(settings=None, spidercls=None): return _get_crawler(settings_dict=settings, spidercls=spidercls) +def _mock_downloader(crawler): + """Give *crawler* a mock engine, whose downloader AutoThrottle reads.""" + crawler.engine = Mock() + crawler.engine.downloader.slots = {} + return crawler.engine.downloader + + @pytest.mark.parametrize( ("value", "expected"), [ @@ -60,29 +67,21 @@ def test_target_concurrency_invalid(value): @pytest.mark.parametrize( - ("spider", "setting", "expected"), + ("setting", "expected"), [ - (UNSET, UNSET, DOWNLOAD_DELAY), - (1.0, UNSET, 1.0), - (UNSET, 1.0, 1.0), - (1.0, 2.0, 1.0), - (3.0, 2.0, 3.0), + (UNSET, DOWNLOAD_DELAY), + (1.0, 1.0), ], ) -def test_mindelay_definition(spider, setting, expected): +def test_mindelay_definition(setting, expected): settings = {} if setting is not UNSET: settings["DOWNLOAD_DELAY"] = setting - class _TestSpider(Spider): - name = "test" - - if spider is not UNSET: - _TestSpider.download_delay = spider - - crawler = get_crawler(settings, _TestSpider) + crawler = get_crawler(settings) at = build_from_crawler(AutoThrottle, crawler) - at._spider_opened(_TestSpider()) + _mock_downloader(crawler) + at._spider_opened(DefaultSpider()) assert at.mindelay == expected @@ -99,58 +98,43 @@ def test_maxdelay_definition(value, expected): settings["AUTOTHROTTLE_MAX_DELAY"] = value crawler = get_crawler(settings) at = build_from_crawler(AutoThrottle, crawler) + _mock_downloader(crawler) at._spider_opened(DefaultSpider()) assert at.maxdelay == expected @pytest.mark.parametrize( - ("min_spider", "min_setting", "start_setting", "expected"), + ("min_setting", "start_setting", "expected"), [ - (UNSET, UNSET, UNSET, AUTOTHROTTLE_START_DELAY), - (AUTOTHROTTLE_START_DELAY - 1.0, UNSET, UNSET, AUTOTHROTTLE_START_DELAY), - (AUTOTHROTTLE_START_DELAY + 1.0, UNSET, UNSET, AUTOTHROTTLE_START_DELAY + 1.0), - (UNSET, AUTOTHROTTLE_START_DELAY - 1.0, UNSET, AUTOTHROTTLE_START_DELAY), - (UNSET, AUTOTHROTTLE_START_DELAY + 1.0, UNSET, AUTOTHROTTLE_START_DELAY + 1.0), - (UNSET, UNSET, AUTOTHROTTLE_START_DELAY - 1.0, AUTOTHROTTLE_START_DELAY - 1.0), - (UNSET, UNSET, AUTOTHROTTLE_START_DELAY + 1.0, AUTOTHROTTLE_START_DELAY + 1.0), - ( - AUTOTHROTTLE_START_DELAY + 1.0, - AUTOTHROTTLE_START_DELAY + 2.0, - UNSET, - AUTOTHROTTLE_START_DELAY + 1.0, - ), + (UNSET, UNSET, AUTOTHROTTLE_START_DELAY), + (AUTOTHROTTLE_START_DELAY - 1.0, UNSET, AUTOTHROTTLE_START_DELAY), + (AUTOTHROTTLE_START_DELAY + 1.0, UNSET, AUTOTHROTTLE_START_DELAY + 1.0), + (UNSET, AUTOTHROTTLE_START_DELAY - 1.0, AUTOTHROTTLE_START_DELAY - 1.0), + (UNSET, AUTOTHROTTLE_START_DELAY + 1.0, AUTOTHROTTLE_START_DELAY + 1.0), ( AUTOTHROTTLE_START_DELAY + 2.0, - UNSET, AUTOTHROTTLE_START_DELAY + 1.0, AUTOTHROTTLE_START_DELAY + 2.0, ), ( AUTOTHROTTLE_START_DELAY + 1.0, - UNSET, AUTOTHROTTLE_START_DELAY + 2.0, AUTOTHROTTLE_START_DELAY + 2.0, ), ], ) -def test_startdelay_definition(min_spider, min_setting, start_setting, expected): +def test_startdelay_definition(min_setting, start_setting, expected): settings = {} if min_setting is not UNSET: settings["DOWNLOAD_DELAY"] = min_setting if start_setting is not UNSET: settings["AUTOTHROTTLE_START_DELAY"] = start_setting - class _TestSpider(Spider): - name = "test" - - if min_spider is not UNSET: - _TestSpider.download_delay = min_spider - - crawler = get_crawler(settings, _TestSpider) + crawler = get_crawler(settings) at = build_from_crawler(AutoThrottle, crawler) - spider = _TestSpider() - at._spider_opened(spider) - assert spider.download_delay == expected + downloader = _mock_downloader(crawler) + at._spider_opened(DefaultSpider()) + assert downloader._delay == expected @pytest.mark.parametrize( @@ -174,15 +158,13 @@ def test_startdelay_definition(min_spider, min_setting, start_setting, expected) def test_skipped(meta, slot): crawler = get_crawler() at = build_from_crawler(AutoThrottle, crawler) + downloader = _mock_downloader(crawler) spider = DefaultSpider() at._spider_opened(spider) request = Request("https://example.com", meta=meta) - crawler.engine = Mock() - crawler.engine.downloader = Mock() - crawler.engine.downloader.slots = {} if slot is not None: - crawler.engine.downloader.slots[slot] = object() + downloader.slots[slot] = object() at._adjust_delay = None # Raise exception if called. at._response_downloaded(None, request, spider) @@ -204,18 +186,16 @@ def test_adjustment(download_latency, target_concurrency, slot_delay, expected): settings = {"AUTOTHROTTLE_TARGET_CONCURRENCY": target_concurrency} crawler = get_crawler(settings) at = build_from_crawler(AutoThrottle, crawler) + downloader = _mock_downloader(crawler) spider = DefaultSpider() at._spider_opened(spider) meta = {"download_latency": download_latency, "download_slot": "foo"} request = Request("https://example.com", meta=meta) response = Response(request.url) - crawler.engine = Mock() - crawler.engine.downloader = Mock() - crawler.engine.downloader.slots = {} slot = Mock() slot.delay = slot_delay - crawler.engine.downloader.slots["foo"] = slot + downloader.slots["foo"] = slot at._response_downloaded(response, request, spider) @@ -240,18 +220,16 @@ def test_adjustment_limits(mindelay, maxdelay, expected): } crawler = get_crawler(settings) at = build_from_crawler(AutoThrottle, crawler) + downloader = _mock_downloader(crawler) spider = DefaultSpider() at._spider_opened(spider) meta = {"download_latency": download_latency, "download_slot": "foo"} request = Request("https://example.com", meta=meta) response = Response(request.url) - crawler.engine = Mock() - crawler.engine.downloader = Mock() - crawler.engine.downloader.slots = {} slot = Mock() slot.delay = slot_delay - crawler.engine.downloader.slots["foo"] = slot + downloader.slots["foo"] = slot at._response_downloaded(response, request, spider) @@ -272,18 +250,16 @@ def test_adjustment_bad_response( settings = {"AUTOTHROTTLE_TARGET_CONCURRENCY": target_concurrency} crawler = get_crawler(settings) at = build_from_crawler(AutoThrottle, crawler) + downloader = _mock_downloader(crawler) spider = DefaultSpider() at._spider_opened(spider) meta = {"download_latency": download_latency, "download_slot": "foo"} request = Request("https://example.com", meta=meta) response = Response(request.url, status=400) - crawler.engine = Mock() - crawler.engine.downloader = Mock() - crawler.engine.downloader.slots = {} slot = Mock() slot.delay = slot_delay - crawler.engine.downloader.slots["foo"] = slot + downloader.slots["foo"] = slot at._response_downloaded(response, request, spider) @@ -294,19 +270,17 @@ def test_debug(caplog): settings = {"AUTOTHROTTLE_DEBUG": True} crawler = get_crawler(settings) at = build_from_crawler(AutoThrottle, crawler) + downloader = _mock_downloader(crawler) spider = DefaultSpider() at._spider_opened(spider) meta = {"download_latency": 1.0, "download_slot": "foo"} request = Request("https://example.com", meta=meta) response = Response(request.url, body=b"foo") - crawler.engine = Mock() - crawler.engine.downloader = Mock() - crawler.engine.downloader.slots = {} slot = Mock() slot.delay = 2.0 slot.transferring = (None, None) - crawler.engine.downloader.slots["foo"] = slot + downloader.slots["foo"] = slot caplog.clear() with caplog.at_level(INFO): @@ -324,19 +298,17 @@ def test_debug(caplog): def test_debug_disabled(caplog): crawler = get_crawler() at = build_from_crawler(AutoThrottle, crawler) + downloader = _mock_downloader(crawler) spider = DefaultSpider() at._spider_opened(spider) meta = {"download_latency": 1.0, "download_slot": "foo"} request = Request("https://example.com", meta=meta) response = Response(request.url, body=b"foo") - crawler.engine = Mock() - crawler.engine.downloader = Mock() - crawler.engine.downloader.slots = {} slot = Mock() slot.delay = 2.0 slot.transferring = (None, None) - crawler.engine.downloader.slots["foo"] = slot + downloader.slots["foo"] = slot caplog.clear() with caplog.at_level(INFO):