Merge pull request #6038 from wRAR/change-init-order

Change extensions/spiders/settings initialisation order, v2

docs/topics/settings.rst

@@ -98,6 +98,29 @@ and settings set there should use the "spider" priority explicitly:
             super().update_settings(settings)
             settings.set("SOME_SETTING", "some value", priority="spider")
 
+.. versionadded:: VERSION
+
+It's also possible to modify the settings in the
+:meth:`~scrapy.Spider.from_crawler` method, e.g. based on :ref:`spider
+arguments <spiderargs>` or other logic:
+
+.. code-block:: python
+
+    import scrapy
+
+
+    class MySpider(scrapy.Spider):
+        name = "myspider"
+
+        @classmethod
+        def from_crawler(cls, crawler, *args, **kwargs):
+            spider = super().from_crawler(crawler, *args, **kwargs)
+            if "some_argument" in kwargs:
+                spider.settings.set(
+                    "SOME_SETTING", kwargs["some_argument"], priority="spider"
+                )
+            return spider
+
 3. Project settings module
 --------------------------
 

docs/topics/spiders.rst

@@ -136,6 +136,15 @@ scrapy.Spider
        attributes in the new instance so they can be accessed later inside the
        spider's code.
 
+       .. versionchanged:: VERSION
+
+           The settings in ``crawler.settings`` can now be modified in this
+           method, which is handy if you want to modify them based on
+           arguments. As a consequence, these settings aren't the final values
+           as they can be modified later by e.g. :ref:`add-ons
+           <topics-addons>`. The final settings are available in the
+           :meth:`start_requests` method and later.
+
        :param crawler: crawler to which the spider will be bound
        :type crawler: :class:`~scrapy.crawler.Crawler` instance
 

scrapy/addons.py

@@ -20,7 +20,7 @@ class AddonManager:
         self.addons: List[Any] = []
 
     def load_settings(self, settings: Settings) -> None:
-        """Load add-ons and configurations from a settings object.
+        """Load add-ons and configurations from a settings object and apply them.
 
         This will load the add-on for every add-on path in the
         ``ADDONS`` setting and execute their ``update_settings`` methods.

scrapy/commands/shell.py

@@ -77,6 +77,7 @@ class Command(ScrapyCommand):
         # The crawler is created this way since the Shell manually handles the
         # crawling engine, so the set up in the crawl method won't work
         crawler = self.crawler_process._create_crawler(spidercls)
+        crawler._apply_settings()
         # The Shell class needs a persistent engine in the crawler
         crawler.engine = crawler._create_engine()
         crawler.engine.start()

scrapy/core/engine.py

@@ -86,6 +86,7 @@ class ExecutionEngine:
         self.crawler: "Crawler" = crawler
         self.settings: Settings = crawler.settings
         self.signals: SignalManager = crawler.signals
+        assert crawler.logformatter
         self.logformatter: LogFormatter = crawler.logformatter
         self.slot: Optional[Slot] = None
         self.spider: Optional[Spider] = None
@@ -368,6 +369,7 @@ class ExecutionEngine:
         if hasattr(scheduler, "open"):
             yield scheduler.open(spider)
         yield self.scraper.open_spider(spider)
+        assert self.crawler.stats
         self.crawler.stats.open_spider(spider)
         yield self.signals.send_catch_log_deferred(signals.spider_opened, spider=spider)
         self.slot.nextcall.schedule()
@@ -439,7 +441,11 @@ class ExecutionEngine:
         )
         dfd.addErrback(log_failure("Error while sending spider_close signal"))
 
-        dfd.addBoth(lambda _: self.crawler.stats.close_spider(spider, reason=reason))
+        def close_stats(_: Any) -> None:
+            assert self.crawler.stats
+            self.crawler.stats.close_spider(spider, reason=reason)
+
+        dfd.addBoth(close_stats)
         dfd.addErrback(log_failure("Stats close failure"))
 
         dfd.addBoth(

scrapy/core/scraper.py

@@ -110,6 +110,7 @@ class Scraper:
         self.concurrent_items: int = crawler.settings.getint("CONCURRENT_ITEMS")
         self.crawler: Crawler = crawler
         self.signals: SignalManager = crawler.signals
+        assert crawler.logformatter
         self.logformatter: LogFormatter = crawler.logformatter
 
     @inlineCallbacks
@@ -244,6 +245,7 @@ class Scraper:
             response=response,
             spider=spider,
         )
+        assert self.crawler.stats
         self.crawler.stats.inc_value(
             f"spider_exceptions/{_failure.value.__class__.__name__}", spider=spider
         )

scrapy/crawler.py

@@ -72,34 +72,45 @@ class Crawler:
         self.spidercls: Type[Spider] = spidercls
         self.settings: Settings = settings.copy()
         self.spidercls.update_settings(self.settings)
+        self._update_root_log_handler()
 
         self.addons: AddonManager = AddonManager(self)
-        self.addons.load_settings(self.settings)
-
         self.signals: SignalManager = SignalManager(self)
 
-        self.stats: StatsCollector = load_object(self.settings["STATS_CLASS"])(self)
+        self._init_reactor: bool = init_reactor
+        self.crawling: bool = False
+        self._started: bool = False
 
-        handler = LogCounterHandler(self, level=self.settings.get("LOG_LEVEL"))
-        logging.root.addHandler(handler)
-
-        d = dict(overridden_settings(self.settings))
-        logger.info(
-            "Overridden settings:\n%(settings)s", {"settings": pprint.pformat(d)}
-        )
+        self.extensions: Optional[ExtensionManager] = None
+        self.stats: Optional[StatsCollector] = None
+        self.logformatter: Optional[LogFormatter] = None
+        self.request_fingerprinter: Optional[RequestFingerprinter] = None
+        self.spider: Optional[Spider] = None
+        self.engine: Optional[ExecutionEngine] = None
 
+    def _update_root_log_handler(self) -> None:
         if get_scrapy_root_handler() is not None:
             # scrapy root handler already installed: update it with new settings
             install_scrapy_root_handler(self.settings)
+
+    def _apply_settings(self) -> None:
+        if self.settings.frozen:
+            return
+
+        self.addons.load_settings(self.settings)
+        self.stats = load_object(self.settings["STATS_CLASS"])(self)
+
+        handler = LogCounterHandler(self, level=self.settings.get("LOG_LEVEL"))
+        logging.root.addHandler(handler)
         # lambda is assigned to Crawler attribute because this way it is not
-        # garbage collected after leaving __init__ scope
+        # garbage collected after leaving the scope
         self.__remove_handler = lambda: logging.root.removeHandler(handler)
         self.signals.connect(self.__remove_handler, signals.engine_stopped)
 
         lf_cls: Type[LogFormatter] = load_object(self.settings["LOG_FORMATTER"])
-        self.logformatter: LogFormatter = lf_cls.from_crawler(self)
+        self.logformatter = lf_cls.from_crawler(self)
 
-        self.request_fingerprinter: RequestFingerprinter = create_instance(
+        self.request_fingerprinter = create_instance(
             load_object(self.settings["REQUEST_FINGERPRINTER_CLASS"]),
             settings=self.settings,
             crawler=self,
@@ -107,7 +118,7 @@ class Crawler:
 
         reactor_class: str = self.settings["TWISTED_REACTOR"]
         event_loop: str = self.settings["ASYNCIO_EVENT_LOOP"]
-        if init_reactor:
+        if self._init_reactor:
             # this needs to be done after the spider settings are merged,
             # but before something imports twisted.internet.reactor
             if reactor_class:
@@ -120,13 +131,13 @@ class Crawler:
             if is_asyncio_reactor_installed() and event_loop:
                 verify_installed_asyncio_event_loop(event_loop)
 
-        self.extensions: ExtensionManager = ExtensionManager.from_crawler(self)
-
+        self.extensions = ExtensionManager.from_crawler(self)
         self.settings.freeze()
-        self.crawling: bool = False
-        self._started: bool = False
-        self.spider: Optional[Spider] = None
-        self.engine: Optional[ExecutionEngine] = None
+
+        d = dict(overridden_settings(self.settings))
+        logger.info(
+            "Overridden settings:\n%(settings)s", {"settings": pprint.pformat(d)}
+        )
 
     @inlineCallbacks
     def crawl(self, *args: Any, **kwargs: Any) -> Generator[Deferred, Any, None]:
@@ -142,6 +153,8 @@ class Crawler:
 
         try:
             self.spider = self._create_spider(*args, **kwargs)
+            self._apply_settings()
+            self._update_root_log_handler()
             self.engine = self._create_engine()
             start_requests = iter(self.spider.start_requests())
             yield self.engine.open_spider(self.spider, start_requests)

scrapy/downloadermiddlewares/httpcache.py

@@ -52,6 +52,7 @@ class HttpCacheMiddleware:
     def from_crawler(
         cls: Type[HttpCacheMiddlewareTV], crawler: Crawler
     ) -> HttpCacheMiddlewareTV:
+        assert crawler.stats
         o = cls(crawler.settings, crawler.stats)
         crawler.signals.connect(o.spider_opened, signal=signals.spider_opened)
         crawler.signals.connect(o.spider_closed, signal=signals.spider_closed)

scrapy/downloadermiddlewares/retry.py

@@ -11,7 +11,7 @@ once the spider has finished crawling all regular (non failed) pages.
 """
 import warnings
 from logging import Logger, getLogger
-from typing import Optional, Union
+from typing import Optional, Type, Union
 
 from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning
 from scrapy.http.request import Request
@@ -43,7 +43,7 @@ def get_retry_request(
     request: Request,
     *,
     spider: Spider,
-    reason: Union[str, Exception] = "unspecified",
+    reason: Union[str, Exception, Type[Exception]] = "unspecified",
     max_retry_times: Optional[int] = None,
     priority_adjust: Optional[int] = None,
     logger: Logger = retry_logger,
@@ -90,6 +90,7 @@ def get_retry_request(
     retry-related job stats
     """
     settings = spider.crawler.settings
+    assert spider.crawler.stats
     stats = spider.crawler.stats
     retry_times = request.meta.get("retry_times", 0) + 1
     if max_retry_times is None:

scrapy/dupefilters.py

@@ -90,6 +90,7 @@ class RFPDupeFilter(BaseDupeFilter):
 
     @classmethod
     def from_crawler(cls, crawler: Crawler) -> Self:
+        assert crawler.request_fingerprinter
         try:
             return cls.from_settings(
                 crawler.settings,
@@ -137,4 +138,5 @@ class RFPDupeFilter(BaseDupeFilter):
             self.logger.debug(msg, {"request": request}, extra={"spider": spider})
             self.logdupes = False
 
+        assert spider.crawler.stats
         spider.crawler.stats.inc_value("dupefilter/filtered", spider=spider)

scrapy/extensions/httpcache.py

@@ -291,6 +291,7 @@ class FilesystemCacheStorage:
             extra={"spider": spider},
         )
 
+        assert spider.crawler.request_fingerprinter
         self._fingerprinter = spider.crawler.request_fingerprinter
 
     def close_spider(self, spider):

scrapy/settings/__init__.py

@@ -563,7 +563,7 @@ def iter_default_settings() -> Iterable[Tuple[str, Any]]:
 def overridden_settings(
     settings: Mapping[_SettingsKeyT, Any]
 ) -> Iterable[Tuple[str, Any]]:
-    """Return a dict of the settings that have been overridden"""
+    """Return an iterable of the settings that have been overridden"""
     for name, defvalue in iter_default_settings():
         value = settings[name]
         if not isinstance(defvalue, dict) and value != defvalue:

scrapy/utils/log.py

@@ -222,6 +222,7 @@ class LogCounterHandler(logging.Handler):
 
     def emit(self, record: logging.LogRecord) -> None:
         sname = f"log_count/{record.levelname}"
+        assert self.crawler.stats
         self.crawler.stats.inc_value(sname)
 
 

scrapy/utils/test.py

@@ -69,6 +69,10 @@ def get_ftp_content_and_delete(
     return b"".join(ftp_data)
 
 
+class TestSpider(Spider):
+    name = "test"
+
+
 def get_crawler(
     spidercls: Optional[Type[Spider]] = None,
     settings_dict: Optional[Dict[str, Any]] = None,
@@ -79,15 +83,16 @@ def get_crawler(
     priority.
     """
     from scrapy.crawler import CrawlerRunner
-    from scrapy.spiders import Spider
 
     # Set by default settings that prevent deprecation warnings.
-    settings = {}
+    settings: Dict[str, Any] = {}
     if prevent_warnings:
         settings["REQUEST_FINGERPRINTER_IMPLEMENTATION"] = "2.7"
     settings.update(settings_dict or {})
     runner = CrawlerRunner(settings)
-    return runner.create_crawler(spidercls or Spider)
+    crawler = runner.create_crawler(spidercls or TestSpider)
+    crawler._apply_settings()
+    return crawler
 
 
 def get_pythonpath() -> str:

tests/CrawlerProcess/args_settings.py

@@ -0,0 +1,24 @@
+from typing import Any
+
+import scrapy
+from scrapy.crawler import Crawler, CrawlerProcess
+
+
+class NoRequestsSpider(scrapy.Spider):
+    name = "no_request"
+
+    @classmethod
+    def from_crawler(cls, crawler: Crawler, *args: Any, **kwargs: Any):
+        spider = super().from_crawler(crawler, *args, **kwargs)
+        spider.settings.set("FOO", kwargs.get("foo"))
+        return spider
+
+    def start_requests(self):
+        self.logger.info(f"The value of FOO is {self.settings.getint('FOO')}")
+        return []
+
+
+process = CrawlerProcess(settings={})
+
+process.crawl(NoRequestsSpider, foo=42)
+process.start()

tests/CrawlerProcess/asyncio_enabled_reactor_different_loop.py

@@ -2,6 +2,7 @@ import asyncio
 import sys
 
 from twisted.internet import asyncioreactor
+from twisted.python import log
 
 if sys.version_info >= (3, 8) and sys.platform == "win32":
     asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())
@@ -24,5 +25,6 @@ process = CrawlerProcess(
         "ASYNCIO_EVENT_LOOP": "uvloop.Loop",
     }
 )
-process.crawl(NoRequestsSpider)
+d = process.crawl(NoRequestsSpider)
+d.addErrback(log.err)
 process.start()

tests/CrawlerProcess/reactor_default_twisted_reactor_select.py

@@ -1,4 +1,5 @@
 from twisted.internet import reactor  # noqa: F401
+from twisted.python import log
 
 import scrapy
 from scrapy.crawler import CrawlerProcess
@@ -17,5 +18,6 @@ process = CrawlerProcess(
     }
 )
 
-process.crawl(NoRequestsSpider)
+d = process.crawl(NoRequestsSpider)
+d.addErrback(log.err)
 process.start()

tests/CrawlerProcess/reactor_select_subclass_twisted_reactor_select.py

@@ -1,5 +1,6 @@
 from twisted.internet.main import installReactor
 from twisted.internet.selectreactor import SelectReactor
+from twisted.python import log
 
 import scrapy
 from scrapy.crawler import CrawlerProcess
@@ -26,5 +27,6 @@ process = CrawlerProcess(
     }
 )
 
-process.crawl(NoRequestsSpider)
+d = process.crawl(NoRequestsSpider)
+d.addErrback(log.err)
 process.start()

tests/CrawlerProcess/twisted_reactor_custom_settings_conflict.py

@@ -1,3 +1,5 @@
+from twisted.python import log
+
 import scrapy
 from scrapy.crawler import CrawlerProcess
 
@@ -17,6 +19,8 @@ class AsyncioReactorSpider(scrapy.Spider):
 
 
 process = CrawlerProcess()
-process.crawl(SelectReactorSpider)
-process.crawl(AsyncioReactorSpider)
+d1 = process.crawl(SelectReactorSpider)
+d1.addErrback(log.err)
+d2 = process.crawl(AsyncioReactorSpider)
+d2.addErrback(log.err)
 process.start()

tests/test_addons.py

@@ -1,8 +1,10 @@
 import itertools
-import unittest
 from typing import Any, Dict
 from unittest.mock import patch
 
+from twisted.internet.defer import inlineCallbacks
+from twisted.trial import unittest
+
 from scrapy import Spider
 from scrapy.crawler import Crawler, CrawlerRunner
 from scrapy.exceptions import NotConfigured
@@ -111,6 +113,7 @@ class AddonManagerTest(unittest.TestCase):
         settings.set("KEY", 0, priority="default")
         runner = CrawlerRunner(settings)
         crawler = runner.create_crawler(Spider)
+        crawler._apply_settings()
         self.assertEqual(crawler.settings.getint("KEY"), 15)
 
         settings_dict = {
@@ -176,3 +179,24 @@ class AddonManagerTest(unittest.TestCase):
                     {"addons": [addon]},
                     extra={"crawler": crawler},
                 )
+
+    @inlineCallbacks
+    def test_enable_addon_in_spider(self):
+        class MySpider(Spider):
+            name = "myspider"
+
+            @classmethod
+            def from_crawler(cls, crawler, *args, **kwargs):
+                spider = super().from_crawler(crawler, *args, **kwargs)
+                addon_config = {"KEY": "addon"}
+                addon_cls = get_addon_cls(addon_config)
+                spider.settings.set("ADDONS", {addon_cls: 1}, priority="spider")
+                return spider
+
+        settings = Settings()
+        settings.set("KEY", "default", priority="default")
+        runner = CrawlerRunner(settings)
+        crawler = runner.create_crawler(MySpider)
+        self.assertEqual(crawler.settings.get("KEY"), "default")
+        yield crawler.crawl()
+        self.assertEqual(crawler.settings.get("KEY"), "addon")

tests/test_commands.py

@@ -965,6 +965,28 @@ class MySpider(scrapy.Spider):
             log,
         )
 
+    def test_args_change_settings(self):
+        spider_code = """
+import scrapy
+
+class MySpider(scrapy.Spider):
+    name = 'myspider'
+
+    @classmethod
+    def from_crawler(cls, crawler, *args, **kwargs):
+        spider = super().from_crawler(crawler, *args, **kwargs)
+        spider.settings.set("FOO", kwargs.get("foo"))
+        return spider
+
+    def start_requests(self):
+        self.logger.info(f"The value of FOO is {self.settings.getint('FOO')}")
+        return []
+"""
+        args = ["-a", "foo=42"]
+        log = self.get_log(spider_code, args=args)
+        self.assertIn("Spider closed (finished)", log)
+        self.assertIn("The value of FOO is 42", log)
+
 
 @skipIf(platform.system() != "Windows", "Windows required for .pyw files")
 class WindowsRunSpiderCommandTest(RunSpiderCommandTest):

tests/test_crawler.py

@@ -42,9 +42,8 @@ class CrawlerTestCase(BaseCrawlerTest):
 
         settings = Settings()
         settings.setdict(project_settings, priority="project")
-        with warnings.catch_warnings():
-            warnings.simplefilter("ignore", ScrapyDeprecationWarning)
-            crawler = Crawler(CustomSettingsSpider, settings)
+        crawler = Crawler(CustomSettingsSpider, settings)
+        crawler._apply_settings()
 
         self.assertEqual(crawler.settings.get("TEST1"), "spider")
         self.assertEqual(crawler.settings.get("TEST2"), "spider")
@@ -513,6 +512,11 @@ class CrawlerProcessSubprocess(ScriptRunnerMixin, unittest.TestCase):
         self.assertNotIn("Using asyncio event loop: uvloop.Loop", log)
         self.assertIn("async pipeline opened!", log)
 
+    def test_args_change_settings(self):
+        log = self.run_script("args_settings.py")
+        self.assertIn("Spider closed (finished)", log)
+        self.assertIn("The value of FOO is 42", log)
+
 
 class CrawlerRunnerSubprocess(ScriptRunnerMixin, unittest.TestCase):
     script_dir = Path(__file__).parent.resolve() / "CrawlerRunner"

tests/test_extension_periodic_log.py

@@ -60,12 +60,9 @@ class TestExtPeriodicLog(PeriodicLog):
 
 
 def extension(settings=None):
-    return TestExtPeriodicLog.from_crawler(
-        Crawler(
-            MetaSpider,
-            settings=settings,
-        )
-    )
+    crawler = Crawler(MetaSpider, settings=settings)
+    crawler._apply_settings()
+    return TestExtPeriodicLog.from_crawler(crawler)
 
 
 class TestPeriodicLog(unittest.TestCase):

tests/test_scheduler.py

@@ -12,6 +12,7 @@ from scrapy.crawler import Crawler
 from scrapy.http import Request
 from scrapy.spiders import Spider
 from scrapy.utils.httpobj import urlparse_cached
+from scrapy.utils.misc import load_object
 from scrapy.utils.test import get_crawler
 from tests.mockserver import MockServer
 
@@ -54,6 +55,7 @@ class MockCrawler(Crawler):
         )
         super().__init__(Spider, settings)
         self.engine = MockEngine(downloader=MockDownloader())
+        self.stats = load_object(self.settings["STATS_CLASS"])(self)
 
 
 class SchedulerHandler:

tests/test_spider.py

@@ -2,13 +2,16 @@ import gzip
 import inspect
 import warnings
 from io import BytesIO
+from typing import Any
 from unittest import mock
 
 from testfixtures import LogCapture
+from twisted.internet.defer import inlineCallbacks
 from twisted.trial import unittest
 from w3lib.url import safe_url_string
 
 from scrapy import signals
+from scrapy.crawler import Crawler
 from scrapy.http import HtmlResponse, Request, Response, TextResponse, XmlResponse
 from scrapy.linkextractors import LinkExtractor
 from scrapy.settings import Settings
@@ -96,6 +99,28 @@ class SpiderTest(unittest.TestCase):
         self.assertEqual(settings.get("TEST2"), "spider")
         self.assertEqual(settings.get("TEST3"), "project")
 
+    @inlineCallbacks
+    def test_settings_in_from_crawler(self):
+        spider_settings = {"TEST1": "spider", "TEST2": "spider"}
+        project_settings = {"TEST1": "project", "TEST3": "project"}
+
+        class TestSpider(self.spider_class):
+            name = "test"
+            custom_settings = spider_settings
+
+            @classmethod
+            def from_crawler(cls, crawler: Crawler, *args: Any, **kwargs: Any):
+                spider = super().from_crawler(crawler, *args, **kwargs)
+                spider.settings.set("TEST1", "spider_instance", priority="spider")
+                return spider
+
+        crawler = Crawler(TestSpider, project_settings)
+        self.assertEqual(crawler.settings.get("TEST1"), "spider")
+        self.assertEqual(crawler.settings.get("TEST2"), "spider")
+        self.assertEqual(crawler.settings.get("TEST3"), "project")
+        yield crawler.crawl()
+        self.assertEqual(crawler.settings.get("TEST1"), "spider_instance")
+
     def test_logger(self):
         spider = self.spider_class("example.com")
         with LogCapture() as lc: