Repository: scrapy/scrapy (python)

Issue:
_store_in_thread must not receive a closed file when using postprocessing plugins
When using post-processing plugins, the FeedExporter closes the temporary file. This behavior has been introduced in commit 500aaa258fc72e943d7bbd258900aeb5af6c8b0d.

Some storage backends, such as GCSFeedStorage, expect an open file in the _store_in_thread method.

To solve the problem we should only close the e.g. GzipFile instance, not the temporary file. Removing the line self.file.close() in GzipPlugin, Bz2Plugin and LZMAPlugin should resolve the error. I think this change would be a clean solution because usually the component that created a resource should be responsible to close that resource - so closing the BinaryIO object in the plugins is a bit unexpected.

Furthermore, GCSFeedExporter should be modified to close the file in its _store_in_thread method after the upload. FTPFeedStorage and S3FeedStorage do this as well.

Pull request:
Do not close the underlying file from compression plugins
Fixes #5932, closes #6178.

Patch:
diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst
index f64bbac06..922b765db 100644
--- a/docs/topics/feed-exports.rst
+++ b/docs/topics/feed-exports.rst
@@ -390,7 +390,13 @@ Each plugin is a class that must implement the following methods:
 
 .. method:: close(self)
 
-    Close the target file object.
+    Clean up the plugin.
+
+    For example, you might want to close a file wrapper that you might have
+    used to compress data written into the file received in the ``__init__``
+    method.
+
+    .. warning:: Do not close the file from the ``__init__`` method.
 
 To pass a parameter to your plugin, use :ref:`feed options <feed-options>`. You
 can then access those parameters from the ``__init__`` method of your plugin.
diff --git a/scrapy/extensions/postprocessing.py b/scrapy/extensions/postprocessing.py
index 79e3b1656..17969c5b0 100644
--- a/scrapy/extensions/postprocessing.py
+++ b/scrapy/extensions/postprocessing.py
@@ -42,7 +42,6 @@ class GzipPlugin:
 
     def close(self) -> None:
         self.gzipfile.close()
-        self.file.close()
 
 
 class Bz2Plugin:
@@ -69,7 +68,6 @@ class Bz2Plugin:
 
     def close(self) -> None:
         self.bz2file.close()
-        self.file.close()
 
 
 class LZMAPlugin:
@@ -111,7 +109,6 @@ class LZMAPlugin:
 
     def close(self) -> None:
         self.lzmafile.close()
-        self.file.close()
 
 
 # io.IOBase is subclassed here, so that exporters can use the PostProcessingManager


Test patch:
diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py
index 277555608..d7560b5ff 100644
--- a/tests/test_feedexport.py
+++ b/tests/test_feedexport.py
@@ -1731,6 +1731,7 @@ class FeedExportTest(FeedExportTestBase):
 
             def store(self, file):
                 Storage.store_file = file
+                Storage.file_was_closed = file.closed
                 file.close()
 
         settings = {
@@ -1746,6 +1747,7 @@ class FeedExportTest(FeedExportTestBase):
         }
         yield self.exported_no_data(settings)
         self.assertIs(Storage.open_file, Storage.store_file)
+        self.assertFalse(Storage.file_was_closed)
 
 
 class FeedPostProcessedExportsTest(FeedExportTestBase):