diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index f83037e6c..3b730c432 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -453,7 +453,17 @@ class FilesPipeline(MediaPipeline): if not store_uri: raise NotConfigured - if isinstance(settings, dict) or settings is None: + if crawler is not None: + if settings is not None: + warnings.warn( + f"FilesPipeline.__init__() was called with a crawler instance and a settings instance" + f" when creating {self.__class__.__qualname__}. The settings instance will be ignored" + f" and crawler.settings will be used. The settings argument will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + settings = crawler.settings + elif isinstance(settings, dict) or settings is None: settings = Settings(settings) cls_name = "FilesPipeline" self.store: FilesStoreProtocol = self._get_store(store_uri) @@ -473,7 +483,9 @@ class FilesPipeline(MediaPipeline): ) super().__init__( - download_func=download_func, settings=settings, crawler=crawler + download_func=download_func, + settings=settings if not crawler else None, + crawler=crawler, ) @classmethod @@ -526,7 +538,7 @@ class FilesPipeline(MediaPipeline): store_uri = settings["FILES_STORE"] if "crawler" in get_func_args(cls.__init__): - o = cls(store_uri, settings=settings, crawler=crawler) + o = cls(store_uri, crawler=crawler) else: o = cls(store_uri, settings=settings) if crawler: diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 71da6a196..fa26133bb 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -79,10 +79,23 @@ class ImagesPipeline(FilesPipeline): ) super().__init__( - store_uri, settings=settings, download_func=download_func, crawler=crawler + store_uri, + settings=settings if not crawler else None, + download_func=download_func, + crawler=crawler, ) - if isinstance(settings, dict) or settings is None: + if crawler is not None: + if settings is not None: + warnings.warn( + f"ImagesPipeline.__init__() was called with a crawler instance and a settings instance" + f" when creating {self.__class__.__qualname__}. The settings instance will be ignored" + f" and crawler.settings will be used. The settings argument will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + settings = crawler.settings + elif isinstance(settings, dict) or settings is None: settings = Settings(settings) resolve = functools.partial( @@ -140,7 +153,7 @@ class ImagesPipeline(FilesPipeline): store_uri = settings["IMAGES_STORE"] if "crawler" in get_func_args(cls.__init__): - o = cls(store_uri, settings=settings, crawler=crawler) + o = cls(store_uri, crawler=crawler) else: o = cls(store_uri, settings=settings) if crawler: diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 99abed09e..70c52d090 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -81,7 +81,17 @@ class MediaPipeline(ABC): ): self.download_func = download_func - if isinstance(settings, dict) or settings is None: + if crawler is not None: + if settings is not None: + warnings.warn( + f"MediaPipeline.__init__() was called with a crawler instance and a settings instance" + f" when creating {self.__class__.__qualname__}. The settings instance will be ignored" + f" and crawler.settings will be used. The settings argument will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + settings = crawler.settings + elif isinstance(settings, dict) or settings is None: settings = Settings(settings) resolve = functools.partial( self._key_for_pipe, base_class_name="MediaPipeline", settings=settings @@ -92,7 +102,6 @@ class MediaPipeline(ABC): self._handle_statuses(self.allow_redirects) if crawler: - # TODO use crawler.settings self._finish_init(crawler) self._modern_init = True else: diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 5e94f9271..9dcb3e4d1 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -755,7 +755,7 @@ class BuildFromCrawlerTestCase(unittest.TestCase): def from_crawler(cls, crawler): settings = crawler.settings store_uri = settings["FILES_STORE"] - o = cls(store_uri, settings=settings, crawler=crawler) + o = cls(store_uri, crawler=crawler) o._from_crawler_called = True return o diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 3f18c83f7..3ffef4102 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -13,7 +13,6 @@ from twisted.trial import unittest from scrapy.http import Request, Response from scrapy.item import Field, Item from scrapy.pipelines.images import ImageException, ImagesPipeline -from scrapy.settings import Settings from scrapy.utils.test import get_crawler skip_pillow: str | None @@ -392,10 +391,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): have different settings. """ custom_settings = self._generate_fake_settings() - default_settings = Settings() - default_sts_pipe = ImagesPipeline( - self.tempdir, settings=default_settings, crawler=get_crawler(None) # TODO - ) + default_sts_pipe = ImagesPipeline(self.tempdir, crawler=get_crawler(None)) user_sts_pipe = ImagesPipeline.from_crawler(get_crawler(None, custom_settings)) for pipe_attr, settings_attr in self.img_cls_attribute_names: expected_default_value = self.default_pipeline_settings.get(pipe_attr) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index a825de92a..58a2d3678 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -378,8 +378,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): class MediaPipelineAllowRedirectSettingsTestCase(unittest.TestCase): def _assert_request_no3xx(self, pipeline_class, settings): - crawler = get_crawler(None, settings) - pipe = pipeline_class(settings=settings, crawler=crawler) # TODO + pipe = pipeline_class(crawler=get_crawler(None, settings)) request = Request("http://url") pipe._modify_media_request(request)