From a4bfd5ab6fd75c4badac1c5d9b40706181c41bd9 Mon Sep 17 00:00:00 2001 From: Stanislau Hluboki Date: Sat, 13 Jun 2020 18:04:38 +0300 Subject: [PATCH] Fix duplicated feed logs --- scrapy/extensions/feedexport.py | 19 +++++++--- tests/test_feedexport.py | 63 +++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 30e6349d6..61dad8726 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -279,11 +279,20 @@ class FeedExporter: 'itemcount': slot.itemcount, 'uri': slot.uri} d = defer.maybeDeferred(slot.storage.store, slot.file) - d.addCallback(lambda _: logger.info(logfmt % "Stored", log_args, - extra={'spider': spider})) - d.addErrback(lambda f: logger.error(logfmt % "Error storing", log_args, - exc_info=failure_to_exc_info(f), - extra={'spider': spider})) + + # Use `largs=log_args` to copy log_args into function's scope + # instead of using `log_args` from the outer scope + d.addCallback( + lambda _, largs=log_args: logger.info( + logfmt % "Stored", largs, extra={'spider': spider} + ) + ) + d.addErrback( + lambda f, largs=log_args: logger.error( + logfmt % "Error storing", largs, + exc_info=failure_to_exc_info(f), extra={'spider': spider} + ) + ) deferred_list.append(d) return defer.DeferredList(deferred_list) if deferred_list else None diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index f7013bc44..e38644214 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -393,6 +393,27 @@ class FromCrawlerFileFeedStorage(FileFeedStorage, FromCrawlerMixin): pass +class DummyBlockingFeedStorage(BlockingFeedStorage): + + def __init__(self, uri): + self.path = file_uri_to_path(uri) + + def _store_in_thread(self, file): + dirname = os.path.dirname(self.path) + if dirname and not os.path.exists(dirname): + os.makedirs(dirname) + with open(self.path, 'ab') as output_file: + output_file.write(file.read()) + + file.close() + + +class FailingBlockingFeedStorage(DummyBlockingFeedStorage): + + def _store_in_thread(self, file): + raise OSError('Cannot store') + + @implementer(IFeedStorage) class LogOnStoreFileStorage: """ @@ -1025,3 +1046,45 @@ class FeedExportTest(unittest.TestCase): } data = yield self.exported_no_data(settings) self.assertEqual(data['csv'], b'') + + @defer.inlineCallbacks + def test_multiple_feeds_success_logs_blocking_feed_storage(self): + settings = { + 'FEEDS': { + self._random_temp_filename(): {'format': 'json'}, + self._random_temp_filename(): {'format': 'xml'}, + self._random_temp_filename(): {'format': 'csv'}, + }, + 'FEED_STORAGES': {'file': 'tests.test_feedexport.DummyBlockingFeedStorage'}, + } + items = [ + {'foo': 'bar1', 'baz': ''}, + {'foo': 'bar2', 'baz': 'quux'}, + ] + with LogCapture() as log: + yield self.exported_data(items, settings) + + print(log) + for fmt in ['json', 'xml', 'csv']: + self.assertIn('Stored %s feed (2 items)' % fmt, str(log)) + + @defer.inlineCallbacks + def test_multiple_feeds_failing_logs_blocking_feed_storage(self): + settings = { + 'FEEDS': { + self._random_temp_filename(): {'format': 'json'}, + self._random_temp_filename(): {'format': 'xml'}, + self._random_temp_filename(): {'format': 'csv'}, + }, + 'FEED_STORAGES': {'file': 'tests.test_feedexport.FailingBlockingFeedStorage'}, + } + items = [ + {'foo': 'bar1', 'baz': ''}, + {'foo': 'bar2', 'baz': 'quux'}, + ] + with LogCapture() as log: + yield self.exported_data(items, settings) + + print(log) + for fmt in ['json', 'xml', 'csv']: + self.assertIn('Error storing %s feed (2 items)' % fmt, str(log))