From a34b929a40c12933f75db4665b71348444a5d603 Mon Sep 17 00:00:00 2001 From: srki24 Date: Fri, 4 Nov 2022 18:00:17 +0100 Subject: [PATCH 1/3] issues/5043 Detaching the stream --- scrapy/exporters.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scrapy/exporters.py b/scrapy/exporters.py index 76cbe4d4b..243ec4fe1 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -247,6 +247,12 @@ class CsvItemExporter(BaseItemExporter): values = list(self._build_row(x for _, x in fields)) self.csv_writer.writerow(values) + def finish_exporting(self): + # Detaching stream in order to avoid file closing. + # The file will be closed with slot.storage.store + # https://github.com/scrapy/scrapy/issues/5043 + self.stream.detach() + def _build_row(self, values): for s in values: try: From 2f2bcb006d349eeeed10018362c780496be96550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 2 Feb 2023 05:55:59 +0100 Subject: [PATCH 2/3] Test stream detaching in CsvItemExporter --- scrapy/exporters.py | 5 +---- tests/test_exporters.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/scrapy/exporters.py b/scrapy/exporters.py index 243ec4fe1..42105690c 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -248,10 +248,7 @@ class CsvItemExporter(BaseItemExporter): self.csv_writer.writerow(values) def finish_exporting(self): - # Detaching stream in order to avoid file closing. - # The file will be closed with slot.storage.store - # https://github.com/scrapy/scrapy/issues/5043 - self.stream.detach() + self.stream.detach() # Avoid closing the wrapped file. def _build_row(self, values): for s in values: diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 69ac928c3..bec8d2267 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -85,6 +85,10 @@ class BaseItemExporterTest(unittest.TestCase): if self.ie.__class__ is not BaseItemExporter: raise self.ie.finish_exporting() + # Delete the item exporter object, so that if it causes the output + # file handle be closed, which should not be the case, follow-up + # interactions with the output file handle will surface the issue. + del self.ie self._check_output() def test_export_item(self): @@ -230,6 +234,7 @@ class PickleItemExporterTest(BaseItemExporterTest): ie.export_item(i1) ie.export_item(i2) ie.finish_exporting() + del ie # See the first “del self.ie” in this file for context. f.seek(0) self.assertEqual(self.item_class(**pickle.load(f)), i1) self.assertEqual(self.item_class(**pickle.load(f)), i2) @@ -241,6 +246,7 @@ class PickleItemExporterTest(BaseItemExporterTest): ie.start_exporting() ie.export_item(item) ie.finish_exporting() + del ie # See the first “del self.ie” in this file for context. self.assertEqual(pickle.loads(fp.getvalue()), item) @@ -267,6 +273,7 @@ class MarshalItemExporterTest(BaseItemExporterTest): ie.start_exporting() ie.export_item(item) ie.finish_exporting() + del ie # See the first “del self.ie” in this file for context. fp.seek(0) self.assertEqual(marshal.load(fp), item) @@ -299,6 +306,7 @@ class CsvItemExporterTest(BaseItemExporterTest): ie.start_exporting() ie.export_item(item) ie.finish_exporting() + del ie # See the first “del self.ie” in this file for context. self.assertCsvEqual(fp.getvalue(), expected) def test_header_export_all(self): @@ -330,6 +338,7 @@ class CsvItemExporterTest(BaseItemExporterTest): ie.export_item(item) ie.export_item(item) ie.finish_exporting() + del ie # See the first “del self.ie” in this file for context. self.assertCsvEqual(output.getvalue(), b'age,name\r\n22,John\xc2\xa3\r\n22,John\xc2\xa3\r\n') @@ -414,6 +423,7 @@ class XmlItemExporterTest(BaseItemExporterTest): ie.start_exporting() ie.export_item(item) ie.finish_exporting() + del ie # See the first “del self.ie” in this file for context. self.assertXmlEquivalent(fp.getvalue(), expected_value) def _check_output(self): @@ -520,6 +530,7 @@ class JsonLinesItemExporterTest(BaseItemExporterTest): self.ie.start_exporting() self.ie.export_item(i3) self.ie.finish_exporting() + del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) self.assertEqual(exported, self._expected_nested) @@ -534,6 +545,7 @@ class JsonLinesItemExporterTest(BaseItemExporterTest): self.ie.start_exporting() self.ie.export_item(item) self.ie.finish_exporting() + del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) item['time'] = str(item['time']) self.assertEqual(exported, item) @@ -561,6 +573,7 @@ class JsonItemExporterTest(JsonLinesItemExporterTest): self.ie.export_item(item) self.ie.export_item(item) self.ie.finish_exporting() + del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) self.assertEqual(exported, [ItemAdapter(item).asdict(), ItemAdapter(item).asdict()]) @@ -577,6 +590,7 @@ class JsonItemExporterTest(JsonLinesItemExporterTest): self.ie.start_exporting() self.ie.export_item(i3) self.ie.finish_exporting() + del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) expected = {'name': 'Jesus', 'age': {'name': 'Maria', 'age': ItemAdapter(i1).asdict()}} self.assertEqual(exported, [expected]) @@ -588,6 +602,7 @@ class JsonItemExporterTest(JsonLinesItemExporterTest): self.ie.start_exporting() self.ie.export_item(i3) self.ie.finish_exporting() + del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) expected = {'name': 'Jesus', 'age': {'name': 'Maria', 'age': i1}} self.assertEqual(exported, [expected]) @@ -597,6 +612,7 @@ class JsonItemExporterTest(JsonLinesItemExporterTest): self.ie.start_exporting() self.ie.export_item(item) self.ie.finish_exporting() + del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) item['time'] = str(item['time']) self.assertEqual(exported, [item]) From 426f3ebb7b368084f6e77ccf8a121c85c7913049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 2 Feb 2023 05:58:32 +0100 Subject: [PATCH 3/3] =?UTF-8?q?Fix=20typo:=20causes=20it=20be=20closed=20?= =?UTF-8?q?=E2=86=92=20causes=20it=20to=20be=20closed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/test_exporters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 34475b05d..95ff5a93c 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -92,7 +92,7 @@ class BaseItemExporterTest(unittest.TestCase): raise self.ie.finish_exporting() # Delete the item exporter object, so that if it causes the output - # file handle be closed, which should not be the case, follow-up + # file handle to be closed, which should not be the case, follow-up # interactions with the output file handle will surface the issue. del self.ie self._check_output()