launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/buildmaster-getFile-rename into lp:launchpad.

Commit message:
Write files fetched from builders to a temporary name, and only rename them into place on success.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/buildmaster-getFile-rename/+merge/336709

This is a slightly speculative attempt to fix odd checksum mismatches on files fetched from builders for gcc-7-cross-ports builds: my theory is that a Deferred might be left over from a previous retry iteration and try to write to the same file.  At any rate, it should be pretty safe to follow the rename-into-place protocol here.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/buildmaster-getFile-rename into lp:launchpad.

=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py	2017-04-27 15:54:54 +0000
+++ lib/lp/buildmaster/interactor.py	2018-01-26 16:33:33 +0000
@@ -10,6 +10,8 @@
 
 from collections import namedtuple
 import logging
+import os.path
+import tempfile
 from urlparse import urlparse
 
 import transaction
@@ -67,7 +69,9 @@
 
     def dataReceived(self, data):
         if self.file is None:
-            self.file = open(self.filename, "wb")
+            self.file = tempfile.NamedTemporaryFile(
+                mode="wb", prefix=os.path.basename(self.filename) + "_",
+                dir=os.path.dirname(self.filename), delete=False)
         try:
             self.file.write(data)
         except IOError:
@@ -82,6 +86,8 @@
         try:
             if self.file is not None:
                 self.file.close()
+            if self.filename is not None and reason.check(ResponseDone):
+                os.rename(self.file.name, self.filename)
         except IOError:
             self.finished.errback()
         else: