launchpad-reviewers team mailing list archive

[Matt Goodall <matt.goodall@xxxxxxxxxxxxx>]

Matt Goodall has proposed merging lp:~matt-goodall/python-oops-datedir-repo/close-oops-tmp into lp:python-oops-datedir-repo.

Commit message:
Ensure OOPS*.tmp file is closed before it's moved.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~matt-goodall/python-oops-datedir-repo/close-oops-tmp/+merge/279098

The oops is written to `{filename}.tmp` before it's moved to `{filename}`.

This branch ensures the .tmp file is closed before the move, just in case it's the cause of https://pastebin.canonical.com/145077/.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~matt-goodall/python-oops-datedir-repo/close-oops-tmp into lp:python-oops-datedir-repo.

=== modified file 'oops_datedir_repo/repository.py'
--- oops_datedir_repo/repository.py	2015-10-27 16:24:57 +0000
+++ oops_datedir_repo/repository.py	2015-12-01 12:00:38 +0000
@@ -122,7 +122,8 @@
         if self.inherit_id:
             oopsid = report.get('id') or oopsid
         report['id'] = oopsid
-        self.serializer.write(report, open(filename + '.tmp', 'wb'))
+        with open(filename + '.tmp', 'wb') as f:
+            self.serializer.write(report, f)
         os.rename(filename + '.tmp', filename)
         if self.stash_path:
             original_report['datedir_repo_filepath'] = filename

=== modified file 'oops_datedir_repo/tests/test_repository.py'
--- oops_datedir_repo/tests/test_repository.py	2015-10-27 16:24:57 +0000
+++ oops_datedir_repo/tests/test_repository.py	2015-12-01 12:00:38 +0000
@@ -408,3 +408,21 @@
         repo = DateDirRepo(self.useFixture(TempDir()).path)
         repo.publish({'id': '1'})
         repo.publish({'id': '2'})
+
+    def test_oops_tmp_is_closed(self):
+
+        # Collect opened OOPS-*.tmp files.
+        oops_tmp = []
+        open_real = open
+        def open_intercept(path, *a, **k):
+            f = open_real(path, *a, **k)
+            name = os.path.basename(path)
+            if name.startswith('OOPS-') and name.endswith('.tmp'):
+                oops_tmp.append(f)
+            return f
+        self.useFixture(MonkeyPatch('__builtin__.open', open_intercept))
+
+        repo = DateDirRepo(self.useFixture(TempDir()).path)
+        repo.publish({'id': '1'})
+        self.assertEqual(1, len(oops_tmp))
+        self.assertTrue(oops_tmp[0].closed)