diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index aba47d998..2218f45e7 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -572,9 +572,12 @@ to ``.json`` or ``.xml``. FEED_STORE_EMPTY ---------------- -Default: ``False`` +Default: ``True`` Whether to export empty feeds (i.e. feeds with no items). +If ``False``, and there are no items to export, no new files are created and +existing files are not modified, even if the :ref:`overwrite feed option +` is enabled. .. setting:: FEED_STORAGES diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 1cdc78f59..6e391d3dc 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -312,8 +312,6 @@ class FTPFeedStorage(BlockingFeedStorage): class FeedSlot: def __init__( self, - file, - exporter, storage, uri, format, @@ -321,9 +319,14 @@ class FeedSlot: batch_id, uri_template, filter, + feed_options, + spider, + exporters, + settings, + crawler, ): - self.file = file - self.exporter = exporter + self.file = None + self.exporter = None self.storage = storage # feed params self.batch_id = batch_id @@ -332,15 +335,44 @@ class FeedSlot: self.uri_template = uri_template self.uri = uri self.filter = filter + # exporter params + self.feed_options = feed_options + self.spider = spider + self.exporters = exporters + self.settings = settings + self.crawler = crawler # flags self.itemcount = 0 self._exporting = False + self._fileloaded = False def start_exporting(self): + if not self._fileloaded: + self.file = self.storage.open(self.spider) + if "postprocessing" in self.feed_options: + self.file = PostProcessingManager( + self.feed_options["postprocessing"], self.file, self.feed_options + ) + self.exporter = self._get_exporter( + file=self.file, + format=self.feed_options["format"], + fields_to_export=self.feed_options["fields"], + encoding=self.feed_options["encoding"], + indent=self.feed_options["indent"], + **self.feed_options["item_export_kwargs"], + ) + self._fileloaded = True + if not self._exporting: self.exporter.start_exporting() self._exporting = True + def _get_instance(self, objcls, *args, **kwargs): + return create_instance(objcls, self.settings, self.crawler, *args, **kwargs) + + def _get_exporter(self, file, format, *args, **kwargs): + return self._get_instance(self.exporters[format], file, *args, **kwargs) + def finish_exporting(self): if self._exporting: self.exporter.finish_exporting() @@ -444,11 +476,16 @@ class FeedExporter: return slot_.file.file return slot_.file - slot.finish_exporting() - if not slot.itemcount and not slot.store_empty: - # We need to call slot.storage.store nonetheless to get the file - # properly closed. - return defer.maybeDeferred(slot.storage.store, get_file(slot)) + if slot.itemcount: + # Normal case + slot.finish_exporting() + elif slot.store_empty and slot.batch_id == 1: + # Need to store the empty file + slot.start_exporting() + slot.finish_exporting() + else: + # In this case, the file is not stored, so no processing is required. + return None logmsg = f"{slot.format} feed ({slot.itemcount} items) in: {slot.uri}" d = defer.maybeDeferred(slot.storage.store, get_file(slot)) @@ -493,23 +530,7 @@ class FeedExporter: :param uri_template: template of uri which contains %(batch_time)s or %(batch_id)d to create new uri """ storage = self._get_storage(uri, feed_options) - file = storage.open(spider) - if "postprocessing" in feed_options: - file = PostProcessingManager( - feed_options["postprocessing"], file, feed_options - ) - - exporter = self._get_exporter( - file=file, - format=feed_options["format"], - fields_to_export=feed_options["fields"], - encoding=feed_options["encoding"], - indent=feed_options["indent"], - **feed_options["item_export_kwargs"], - ) slot = FeedSlot( - file=file, - exporter=exporter, storage=storage, uri=uri, format=feed_options["format"], @@ -517,9 +538,12 @@ class FeedExporter: batch_id=batch_id, uri_template=uri_template, filter=self.filters[uri_template], + feed_options=feed_options, + spider=spider, + exporters=self.exporters, + settings=self.settings, + crawler=getattr(self, "crawler", None), ) - if slot.store_empty: - slot.start_exporting() return slot def item_scraped(self, item, spider): @@ -603,14 +627,6 @@ class FeedExporter: else: logger.error("Unknown feed storage scheme: %(scheme)s", {"scheme": scheme}) - def _get_instance(self, objcls, *args, **kwargs): - return create_instance( - objcls, self.settings, getattr(self, "crawler", None), *args, **kwargs - ) - - def _get_exporter(self, file, format, *args, **kwargs): - return self._get_instance(self.exporters[format], file, *args, **kwargs) - def _get_storage(self, uri, feed_options): """Fork of create_instance specific to feed storage classes diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index a4cb555bd..1bada1c70 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -141,7 +141,7 @@ EXTENSIONS_BASE = { FEED_TEMPDIR = None FEEDS = {} FEED_URI_PARAMS = None # a function to extend uri arguments -FEED_STORE_EMPTY = False +FEED_STORE_EMPTY = True FEED_EXPORT_ENCODING = None FEED_EXPORT_FIELDS = None FEED_STORAGES = {} diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index eace59d37..96bb688ab 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -747,10 +747,9 @@ class FeedExportTest(FeedExportTestBase): yield crawler.crawl() for file_path, feed_options in FEEDS.items(): - if not Path(file_path).exists(): - continue - - content[feed_options["format"]] = Path(file_path).read_bytes() + content[feed_options["format"]] = ( + Path(file_path).read_bytes() if Path(file_path).exists() else None + ) finally: for file_path in FEEDS.keys(): @@ -956,9 +955,10 @@ class FeedExportTest(FeedExportTestBase): "FEEDS": { self._random_temp_filename(): {"format": fmt}, }, + "FEED_STORE_EMPTY": False, } data = yield self.exported_no_data(settings) - self.assertEqual(b"", data[fmt]) + self.assertEqual(None, data[fmt]) @defer.inlineCallbacks def test_start_finish_exporting_items(self): @@ -1074,8 +1074,7 @@ class FeedExportTest(FeedExportTestBase): with LogCapture() as log: yield self.exported_no_data(settings) - print(log) - self.assertEqual(str(log).count("Storage.store is called"), 3) + self.assertEqual(str(log).count("Storage.store is called"), 0) @defer.inlineCallbacks def test_export_multiple_item_classes(self): @@ -1742,10 +1741,9 @@ class FeedPostProcessedExportsTest(FeedExportTestBase): yield crawler.crawl() for file_path, feed_options in FEEDS.items(): - if not Path(file_path).exists(): - continue - - content[str(file_path)] = Path(file_path).read_bytes() + content[str(file_path)] = ( + Path(file_path).read_bytes() if Path(file_path).exists() else None + ) finally: for file_path in FEEDS.keys(): @@ -2246,6 +2244,9 @@ class BatchDeliveriesTest(FeedExportTestBase): for path, feed in FEEDS.items(): dir_name = Path(path).parent + if not dir_name.exists(): + content[feed["format"]] = [] + continue for file in sorted(dir_name.iterdir()): content[feed["format"]].append(file.read_bytes()) finally: @@ -2429,10 +2430,11 @@ class BatchDeliveriesTest(FeedExportTestBase): / self._file_mark: {"format": fmt}, }, "FEED_EXPORT_BATCH_ITEM_COUNT": 1, + "FEED_STORE_EMPTY": False, } data = yield self.exported_no_data(settings) data = dict(data) - self.assertEqual(b"", data[fmt][0]) + self.assertEqual(0, len(data[fmt])) @defer.inlineCallbacks def test_export_no_items_store_empty(self): @@ -2546,9 +2548,6 @@ class BatchDeliveriesTest(FeedExportTestBase): for expected_batch, got_batch in zip(expected, data[fmt]): self.assertEqual(expected_batch, got_batch) - @pytest.mark.skipif( - sys.platform == "win32", reason="Odd behaviour on file creation/output" - ) @defer.inlineCallbacks def test_batch_path_differ(self): """ @@ -2570,7 +2569,7 @@ class BatchDeliveriesTest(FeedExportTestBase): "FEED_EXPORT_BATCH_ITEM_COUNT": 1, } data = yield self.exported_data(items, settings) - self.assertEqual(len(items), len([_ for _ in data["json"] if _])) + self.assertEqual(len(items), len(data["json"])) @defer.inlineCallbacks def test_stats_batch_file_success(self): @@ -2656,7 +2655,7 @@ class BatchDeliveriesTest(FeedExportTestBase): crawler = get_crawler(TestSpider, settings) yield crawler.crawl() - self.assertEqual(len(CustomS3FeedStorage.stubs), len(items) + 1) + self.assertEqual(len(CustomS3FeedStorage.stubs), len(items)) for stub in CustomS3FeedStorage.stubs[:-1]: stub.assert_no_pending_responses()