diff --git a/swh/storage/db.py b/swh/storage/db.py --- a/swh/storage/db.py +++ b/swh/storage/db.py @@ -197,15 +197,19 @@ write_thread = threading.Thread(target=writer) write_thread.start() - with open(write_file, 'w') as f: - for d in items: - if item_cb is not None: - item_cb(d) - line = [escape(d.get(k)) for k in columns] - f.write(','.join(line)) - f.write('\n') - - write_thread.join() + try: + with open(write_file, 'w') as f: + for d in items: + if item_cb is not None: + item_cb(d) + line = [escape(d.get(k)) for k in columns] + f.write(','.join(line)) + f.write('\n') + finally: + # No problem bubbling up exceptions, but we still need to make sure + # we finish copying, even though we're probably going to cancel the + # transaction. + write_thread.join() def mktemp(self, tblname, cur=None): self._cursor(cur).execute('SELECT swh_mktemp(%s)', (tblname,)) diff --git a/swh/storage/tests/test_storage.py b/swh/storage/tests/test_storage.py --- a/swh/storage/tests/test_storage.py +++ b/swh/storage/tests/test_storage.py @@ -10,7 +10,7 @@ import unittest from uuid import UUID -from unittest.mock import patch +from unittest.mock import Mock, patch from nose.tools import istest from nose.plugins.attrib import attr @@ -2464,6 +2464,21 @@ }, ]) + # This test is only relevant on the local storage, with an actual + # objstorage raising an exception + @istest + def content_add_objstorage_exception(self): + self.storage.objstorage.add = Mock( + side_effect=Exception('mocked broken objstorage') + ) + + with self.assertRaises(Exception) as e: + self.storage.content_add([self.cont]) + + self.assertEqual(e.exception.args, ('mocked broken objstorage',)) + missing = list(self.storage.content_missing([self.cont])) + self.assertEqual(missing, [self.cont['sha1']]) + class AlteringSchemaTest(BaseTestStorage, unittest.TestCase): """This class is dedicated for the rare case where the schema needs to