diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index 9f6c752d7..e1e027c95 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -15,8 +15,7 @@ from scrapy.exceptions import UsageError from scrapy.http import Request, Response from scrapy.utils import display from scrapy.utils.asyncgen import collect_asyncgen -from scrapy.utils.defer import aiter_errback, deferred_from_coro -from scrapy.utils.deprecate import argument_is_required +from scrapy.utils.defer import _schedule_coro, aiter_errback, deferred_from_coro from scrapy.utils.log import failure_to_exc_info from scrapy.utils.misc import arg_to_iter from scrapy.utils.spider import spidercls_for_request @@ -285,12 +284,12 @@ class Command(BaseRunSpiderCommand): if opts.pipelines: assert self.pcrawler.engine itemproc = self.pcrawler.engine.scraper.itemproc - needs_spider = argument_is_required(itemproc.process_item, "spider") - for item in items: - if needs_spider: + if hasattr(itemproc, "process_item_async"): + for item in items: + _schedule_coro(itemproc.process_item_async(item)) + else: + for item in items: itemproc.process_item(item, spider) - else: - itemproc.process_item(item) self.add_items(depth, items) self.add_requests(depth, requests) diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 9184731cd..84cc6a0ee 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -35,7 +35,7 @@ from scrapy.utils.defer import ( parallel, parallel_async, ) -from scrapy.utils.deprecate import argument_is_required, method_is_overridden +from scrapy.utils.deprecate import method_is_overridden from scrapy.utils.log import failure_to_exc_info, logformatter_adapter from scrapy.utils.misc import load_object, warn_on_generator_with_return_value from scrapy.utils.python import global_object_name @@ -110,48 +110,13 @@ class Scraper: crawler.settings["ITEM_PROCESSOR"] ) self.itemproc: ItemPipelineManager = itemproc_cls.from_crawler(crawler) - itemproc_methods = [ + self._itemproc_has_async: dict[str, bool] = {} + for method in [ "open_spider", "close_spider", - ] - if not hasattr(self.itemproc, "process_item_async"): - warnings.warn( - f"{global_object_name(itemproc_cls)} doesn't define a process_item_async() method," - f" this is deprecated and the method will be required in future Scrapy versions.", - ScrapyDeprecationWarning, - stacklevel=2, - ) - itemproc_methods.append("process_item") - self._itemproc_has_process_async = False - elif ( - issubclass(itemproc_cls, ItemPipelineManager) - and method_is_overridden(itemproc_cls, ItemPipelineManager, "process_item") - and not method_is_overridden( - itemproc_cls, ItemPipelineManager, "process_item_async" - ) - ): - warnings.warn( - f"{global_object_name(itemproc_cls)} overrides process_item() but doesn't override process_item_async()." - f" This is deprecated. process_item() will be used, but in future Scrapy versions process_item_async() will be used instead.", - ScrapyDeprecationWarning, - stacklevel=2, - ) - itemproc_methods.append("process_item") - self._itemproc_has_process_async = False - else: - self._itemproc_has_process_async = True - self._itemproc_needs_spider: dict[str, bool] = {} - for method in itemproc_methods: - self._itemproc_needs_spider[method] = argument_is_required( - getattr(self.itemproc, method), "spider" - ) - if self._itemproc_needs_spider[method]: - warnings.warn( - f"The {method}() method of {global_object_name(itemproc_cls)} requires a spider argument," - f" this is deprecated and the argument will not be passed in future Scrapy versions.", - ScrapyDeprecationWarning, - stacklevel=2, - ) + "process_item", + ]: + self._check_deprecated_itemproc_method(method) self.concurrent_items: int = crawler.settings.getint("CONCURRENT_ITEMS") self.crawler: Crawler = crawler @@ -159,6 +124,33 @@ class Scraper: assert crawler.logformatter self.logformatter: LogFormatter = crawler.logformatter + def _check_deprecated_itemproc_method(self, method: str) -> None: + itemproc_cls = type(self.itemproc) + if not hasattr(self.itemproc, "process_item_async"): + warnings.warn( + f"{global_object_name(itemproc_cls)} doesn't define a {method}_async() method," + f" this is deprecated and the method will be required in future Scrapy versions.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + self._itemproc_has_async[method] = False + elif ( + issubclass(itemproc_cls, ItemPipelineManager) + and method_is_overridden(itemproc_cls, ItemPipelineManager, method) + and not method_is_overridden( + itemproc_cls, ItemPipelineManager, f"{method}_async" + ) + ): + warnings.warn( + f"{global_object_name(itemproc_cls)} overrides {method}() but doesn't override {method}_async()." + f" This is deprecated. {method}() will be used, but in future Scrapy versions {method}_async() will be used instead.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + self._itemproc_has_async[method] = False + else: + self._itemproc_has_async[method] = True + def open_spider(self, spider: Spider | None = None) -> Deferred[None]: warnings.warn( "Scraper.open_spider() is deprecated, use open_spider_async() instead", @@ -177,12 +169,12 @@ class Scraper: raise RuntimeError( "Scraper.open_spider() called before Crawler.spider is set." ) - if self._itemproc_needs_spider["open_spider"]: + if self._itemproc_has_async["open_spider"]: + await self.itemproc.open_spider_async() + else: await maybe_deferred_to_future( self.itemproc.open_spider(self.crawler.spider) ) - else: - await maybe_deferred_to_future(self.itemproc.open_spider()) def close_spider(self, spider: Spider | None = None) -> Deferred[None]: warnings.warn( @@ -202,12 +194,13 @@ class Scraper: self.slot.closing = Deferred() self._check_if_closing() await maybe_deferred_to_future(self.slot.closing) - if self._itemproc_needs_spider["close_spider"]: + if self._itemproc_has_async["close_spider"]: + await self.itemproc.close_spider_async() + else: + assert self.crawler.spider await maybe_deferred_to_future( self.itemproc.close_spider(self.crawler.spider) ) - else: - await maybe_deferred_to_future(self.itemproc.close_spider()) def is_idle(self) -> bool: """Return True if there isn't any more spiders to process""" @@ -487,14 +480,12 @@ class Scraper: assert self.crawler.spider is not None # typing self.slot.itemproc_size += 1 try: - if self._itemproc_has_process_async: + if self._itemproc_has_async["process_item"]: output = await self.itemproc.process_item_async(item) else: - if self._itemproc_needs_spider["process_item"]: - d = self.itemproc.process_item(item, self.crawler.spider) - else: - d = self.itemproc.process_item(item) - output = await maybe_deferred_to_future(d) + output = await maybe_deferred_to_future( + self.itemproc.process_item(item, self.crawler.spider) + ) except DropItem as ex: logkws = self.logformatter.dropped(item, ex, response, self.crawler.spider) if logkws is not None: diff --git a/scrapy/middleware.py b/scrapy/middleware.py index be41b52e4..d441be3ba 100644 --- a/scrapy/middleware.py +++ b/scrapy/middleware.py @@ -194,17 +194,13 @@ class MiddlewareManager(ABC): obj = await ensure_awaitable(method(obj, *args)) return obj - def open_spider( - self, spider: Spider | None = None - ) -> Deferred[list[None]]: # pragma: no cover + def open_spider(self, spider: Spider) -> Deferred[list[None]]: # pragma: no cover raise NotImplementedError( "MiddlewareManager.open_spider() is no longer implemented" " and will be removed in a future Scrapy version." ) - def close_spider( - self, spider: Spider | None = None - ) -> Deferred[list[None]]: # pragma: no cover + def close_spider(self, spider: Spider) -> Deferred[list[None]]: # pragma: no cover raise NotImplementedError( "MiddlewareManager.close_spider() is no longer implemented" " and will be removed in a future Scrapy version." diff --git a/scrapy/pipelines/__init__.py b/scrapy/pipelines/__init__.py index b7ec928e8..6d2494719 100644 --- a/scrapy/pipelines/__init__.py +++ b/scrapy/pipelines/__init__.py @@ -14,7 +14,11 @@ from twisted.internet.defer import Deferred, DeferredList from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.middleware import MiddlewareManager from scrapy.utils.conf import build_component_list -from scrapy.utils.defer import deferred_from_coro, maybeDeferred_coro +from scrapy.utils.defer import ( + deferred_from_coro, + maybe_deferred_to_future, + maybeDeferred_coro, +) from scrapy.utils.python import global_object_name if TYPE_CHECKING: @@ -44,14 +48,13 @@ class ItemPipelineManager(MiddlewareManager): self.methods["process_item"].append(pipe.process_item) self._check_mw_method_spider_arg(pipe.process_item) - def process_item(self, item: Any, spider: Spider | None = None) -> Deferred[Any]: - if spider: - self._set_compat_spider(spider) + def process_item(self, item: Any, spider: Spider) -> Deferred[Any]: warnings.warn( f"{global_object_name(type(self))}.process_item() is deprecated, use process_item_async() instead.", category=ScrapyDeprecationWarning, stacklevel=2, ) + self._set_compat_spider(spider) return deferred_from_coro(self.process_item_async(item)) async def process_item_async(self, item: Any) -> Any: @@ -77,14 +80,26 @@ class ItemPipelineManager(MiddlewareManager): d2.addErrback(eb) return d2 - def open_spider(self, spider: Spider | None = None) -> Deferred[list[None]]: - if spider: - self._warn_spider_arg("open_spider") - self._set_compat_spider(spider) + def open_spider(self, spider: Spider) -> Deferred[list[None]]: + warnings.warn( + f"{global_object_name(type(self))}.open_spider() is deprecated, use open_spider_async() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + self._set_compat_spider(spider) return self._process_parallel("open_spider") - def close_spider(self, spider: Spider | None = None) -> Deferred[list[None]]: - if spider: - self._warn_spider_arg("close_spider") - self._set_compat_spider(spider) + async def open_spider_async(self) -> None: + await maybe_deferred_to_future(self._process_parallel("open_spider")) + + def close_spider(self, spider: Spider) -> Deferred[list[None]]: + warnings.warn( + f"{global_object_name(type(self))}.close_spider() is deprecated, use close_spider_async() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + self._set_compat_spider(spider) return self._process_parallel("close_spider") + + async def close_spider_async(self) -> None: + await maybe_deferred_to_future(self._process_parallel("close_spider")) diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index f50e7bec7..a4233d447 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -309,7 +309,8 @@ class TestDownloadDeprecated(TestManagerBase): async with self.get_mwman() as mwman: with pytest.warns( ScrapyDeprecationWarning, - match=r"Passing a spider argument to DownloaderMiddlewareManager.download\(\) is deprecated", + match=r"Passing a spider argument to DownloaderMiddlewareManager.download\(\)" + r" is deprecated and the passed value is ignored.", ): ret = await maybe_deferred_to_future( mwman.download(download_func, req, mwman.crawler.spider) diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index 4b8007ead..131a0e36b 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -32,6 +32,10 @@ class DeprecatedSpiderArgPipeline: def close_spider(self, spider): pass + def process_item(self, item, spider): + item["pipeline_passed"] = True + return item + class DeferredPipeline: def cb(self, item): @@ -145,11 +149,32 @@ class TestPipeline: yield crawler.crawl(mockserver=self.mockserver) assert len(self.items) == 1 + @deferred_f_from_coro_f + async def test_deprecated_spider_arg(self, mockserver: MockServer) -> None: + crawler = self._create_crawler(DeprecatedSpiderArgPipeline) + with ( + pytest.warns( + ScrapyDeprecationWarning, + match=r"DeprecatedSpiderArgPipeline.open_spider\(\) requires a spider argument", + ), + pytest.warns( + ScrapyDeprecationWarning, + match=r"DeprecatedSpiderArgPipeline.close_spider\(\) requires a spider argument", + ), + pytest.warns( + ScrapyDeprecationWarning, + match=r"DeprecatedSpiderArgPipeline.process_item\(\) requires a spider argument", + ), + ): + await maybe_deferred_to_future(crawler.crawl(mockserver=mockserver)) + + assert len(self.items) == 1 + class TestCustomPipelineManager: def test_deprecated_process_item_spider_arg(self) -> None: class CustomPipelineManager(ItemPipelineManager): - def process_item(self, item, spider): # pylint: disable=signature-differs + def process_item(self, item, spider): # pylint: disable=useless-parent-delegation return super().process_item(item, spider) crawler = get_crawler(DefaultSpider) @@ -190,13 +215,21 @@ class TestCustomPipelineManager: @deferred_f_from_coro_f async def test_integration_no_async_subclass(self, mockserver: MockServer) -> None: class CustomPipelineManager(ItemPipelineManager): - def open_spider(self, spider): # pylint: disable=signature-differs - return super().open_spider(spider) + def open_spider(self, spider): + with pytest.warns( + ScrapyDeprecationWarning, + match=r"CustomPipelineManager.open_spider\(\) is deprecated, use open_spider_async\(\)", + ): + return super().open_spider(spider) - def close_spider(self, spider): # pylint: disable=signature-differs - return super().close_spider(spider) + def close_spider(self, spider): + with pytest.warns( + ScrapyDeprecationWarning, + match=r"CustomPipelineManager.close_spider\(\) is deprecated, use close_spider_async\(\)", + ): + return super().close_spider(spider) - def process_item(self, item, spider): # pylint: disable=signature-differs + def process_item(self, item, spider): with pytest.warns( ScrapyDeprecationWarning, match=r"CustomPipelineManager.process_item\(\) is deprecated, use process_item_async\(\)", @@ -222,23 +255,11 @@ class TestCustomPipelineManager: with ( pytest.warns( ScrapyDeprecationWarning, - match=r"The open_spider\(\) method of .+\.CustomPipelineManager requires a spider argument", + match=r"CustomPipelineManager overrides open_spider\(\) but doesn't override open_spider_async\(\)", ), pytest.warns( ScrapyDeprecationWarning, - match=r"The close_spider\(\) method of .+\.CustomPipelineManager requires a spider argument", - ), - pytest.warns( - ScrapyDeprecationWarning, - match=r"The process_item\(\) method of .+\.CustomPipelineManager requires a spider argument", - ), - pytest.warns( - ScrapyDeprecationWarning, - match=r"Passing a spider argument to CustomPipelineManager.open_spider\(\) is deprecated", - ), - pytest.warns( - ScrapyDeprecationWarning, - match=r"Passing a spider argument to CustomPipelineManager.close_spider\(\) is deprecated", + match=r"CustomPipelineManager overrides close_spider\(\) but doesn't override close_spider_async\(\)", ), pytest.warns( ScrapyDeprecationWarning, @@ -294,22 +315,18 @@ class TestCustomPipelineManager: crawler.spider = crawler._create_spider() crawler.signals.connect(_on_item_scraped, signals.item_scraped) with ( + pytest.warns( + ScrapyDeprecationWarning, + match=r"CustomPipelineManager doesn't define a open_spider_async\(\) method", + ), + pytest.warns( + ScrapyDeprecationWarning, + match=r"CustomPipelineManager doesn't define a close_spider_async\(\) method", + ), pytest.warns( ScrapyDeprecationWarning, match=r"CustomPipelineManager doesn't define a process_item_async\(\) method", ), - pytest.warns( - ScrapyDeprecationWarning, - match=r"The open_spider\(\) method of .+\.CustomPipelineManager requires a spider argument", - ), - pytest.warns( - ScrapyDeprecationWarning, - match=r"The close_spider\(\) method of .+\.CustomPipelineManager requires a spider argument", - ), - pytest.warns( - ScrapyDeprecationWarning, - match=r"The process_item\(\) method of .+\.CustomPipelineManager requires a spider argument", - ), ): await maybe_deferred_to_future(crawler.crawl(mockserver=mockserver)) @@ -325,9 +342,12 @@ class TestMiddlewareManagerSpider: def crawler(self) -> Crawler: return get_crawler(Spider) - def test_deprecated_spider_arg_no_crawler_spider(self, crawler: Crawler) -> None: - """Crawler is provided, but doesn't have a spider. The instance passed to the method is - ignored and raises a warning.""" + @deferred_f_from_coro_f + async def test_deprecated_spider_arg_no_crawler_spider( + self, crawler: Crawler + ) -> None: + """Crawler is provided, but doesn't have a spider, the methods raise an exception. + The instance passed to a deprecated method is ignored.""" mwman = ItemPipelineManager(crawler=crawler) with ( pytest.warns( @@ -338,12 +358,16 @@ class TestMiddlewareManagerSpider: ScrapyDeprecationWarning, match=r"DeprecatedSpiderArgPipeline.close_spider\(\) requires a spider argument", ), + pytest.warns( + ScrapyDeprecationWarning, + match=r"DeprecatedSpiderArgPipeline.process_item\(\) requires a spider argument", + ), ): mwman._add_middleware(DeprecatedSpiderArgPipeline()) with ( pytest.warns( ScrapyDeprecationWarning, - match=r"Passing a spider argument to ItemPipelineManager.open_spider\(\) is deprecated", + match=r"ItemPipelineManager.open_spider\(\) is deprecated, use open_spider_async\(\) instead", ), pytest.raises( ValueError, @@ -351,10 +375,15 @@ class TestMiddlewareManagerSpider: ), ): mwman.open_spider(DefaultSpider()) + with pytest.raises( + ValueError, + match="ItemPipelineManager needs to access self.crawler.spider but it is None", + ): + await mwman.open_spider_async() with ( pytest.warns( ScrapyDeprecationWarning, - match=r"Passing a spider argument to ItemPipelineManager.close_spider\(\) is deprecated", + match=r"ItemPipelineManager.close_spider\(\) is deprecated, use close_spider_async\(\) instead", ), pytest.raises( ValueError, @@ -362,59 +391,59 @@ class TestMiddlewareManagerSpider: ), ): mwman.close_spider(DefaultSpider()) + with pytest.raises( + ValueError, + match="ItemPipelineManager needs to access self.crawler.spider but it is None", + ): + await mwman.close_spider_async() def test_deprecated_spider_arg_with_crawler(self, crawler: Crawler) -> None: - """Crawler is provided and has a spider, works. The instance passed to the method is ignored, - even if mismatched, but raises a warning.""" + """Crawler is provided and has a spider, works. The instance passed to a deprecated method + is ignored, even if mismatched.""" mwman = ItemPipelineManager(crawler=crawler) crawler.spider = crawler._create_spider("foo") with pytest.warns( ScrapyDeprecationWarning, - match=r"Passing a spider argument to ItemPipelineManager.open_spider\(\) is deprecated", + match=r"ItemPipelineManager.open_spider\(\) is deprecated, use open_spider_async\(\) instead", ): mwman.open_spider(DefaultSpider()) with pytest.warns( ScrapyDeprecationWarning, - match=r"Passing a spider argument to ItemPipelineManager.close_spider\(\) is deprecated", + match=r"ItemPipelineManager.close_spider\(\) is deprecated, use close_spider_async\(\) instead", ): mwman.close_spider(DefaultSpider()) def test_deprecated_spider_arg_without_crawler(self) -> None: - """The first instance passed to the method is used, with a warning. Mismatched ones raise an error.""" + """The first instance passed to a deprecated method is used. Mismatched ones raise an error.""" with pytest.warns( ScrapyDeprecationWarning, match="was called without the crawler argument", ): mwman = ItemPipelineManager() - with ( - pytest.warns( - ScrapyDeprecationWarning, - match=r"DeprecatedSpiderArgPipeline.open_spider\(\) requires a spider argument", - ), - pytest.warns( - ScrapyDeprecationWarning, - match=r"DeprecatedSpiderArgPipeline.close_spider\(\) requires a spider argument", - ), - ): - mwman._add_middleware(DeprecatedSpiderArgPipeline()) + spider = DefaultSpider() with pytest.warns( ScrapyDeprecationWarning, - match=r"Passing a spider argument to ItemPipelineManager.open_spider\(\) is deprecated", + match=r"ItemPipelineManager.open_spider\(\) is deprecated, use open_spider_async\(\) instead", ): - mwman.open_spider(DefaultSpider()) + mwman.open_spider(spider) with ( pytest.warns( ScrapyDeprecationWarning, - match=r"Passing a spider argument to ItemPipelineManager.close_spider\(\) is deprecated", + match=r"ItemPipelineManager.close_spider\(\) is deprecated, use close_spider_async\(\) instead", ), pytest.raises( RuntimeError, match="Different instances of Spider were passed" ), ): mwman.close_spider(DefaultSpider()) - mwman.close_spider() + with pytest.warns( + ScrapyDeprecationWarning, + match=r"ItemPipelineManager.close_spider\(\) is deprecated, use close_spider_async\(\) instead", + ): + mwman.close_spider(spider) - def test_no_spider_arg_without_crawler(self) -> None: + @deferred_f_from_coro_f + async def test_no_spider_arg_without_crawler(self) -> None: """If no crawler and no spider arg, raise an error.""" with pytest.warns( ScrapyDeprecationWarning, @@ -430,6 +459,10 @@ class TestMiddlewareManagerSpider: ScrapyDeprecationWarning, match=r"DeprecatedSpiderArgPipeline.close_spider\(\) requires a spider argument", ), + pytest.warns( + ScrapyDeprecationWarning, + match=r"DeprecatedSpiderArgPipeline.process_item\(\) requires a spider argument", + ), ): mwman._add_middleware(DeprecatedSpiderArgPipeline()) with ( @@ -438,4 +471,4 @@ class TestMiddlewareManagerSpider: match="has no known Spider instance", ), ): - mwman.open_spider() + await mwman.open_spider_async() diff --git a/tests/test_spidermiddleware.py b/tests/test_spidermiddleware.py index 8fd33eb13..977273673 100644 --- a/tests/test_spidermiddleware.py +++ b/tests/test_spidermiddleware.py @@ -16,6 +16,7 @@ from scrapy.spiders import Spider from scrapy.utils.asyncgen import collect_asyncgen from scrapy.utils.asyncio import call_later from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future +from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler if TYPE_CHECKING: @@ -604,7 +605,7 @@ class TestProcessSpiderException(TestBaseAsyncSpiderMiddleware): class TestDeprecatedSpiderArg(TestSpiderMiddleware): @deferred_f_from_coro_f - async def test_deprecated_spider_arg(self): + async def test_deprecated_mw_spider_arg(self): class DeprecatedSpiderArgMiddleware: def process_spider_input(self, response, spider): return None @@ -631,3 +632,26 @@ class TestDeprecatedSpiderArg(TestSpiderMiddleware): ): self.mwman._add_middleware(DeprecatedSpiderArgMiddleware()) await self._scrape_response() + + @deferred_f_from_coro_f + async def test_deprecated_mwman_spider_arg(self): + with pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to SpiderMiddlewareManager.process_start\(\)" + r" is deprecated and the passed value is ignored", + ): + await self.mwman.process_start(DefaultSpider()) + + @deferred_f_from_coro_f + async def test_deprecated_mwman_spider_arg_no_crawler(self): + with pytest.warns( + ScrapyDeprecationWarning, + match=r"MiddlewareManager.__init__\(\) was called without the crawler argument", + ): + mwman = SpiderMiddlewareManager() + with pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to SpiderMiddlewareManager.process_start\(\)" + r" is deprecated, SpiderMiddlewareManager should be instantiated with a Crawler", + ): + await mwman.process_start(DefaultSpider())