Merge lp:~cjwatson/launchpad/fix-buildmaster-twisted-agent into lp:launchpad

Proposed by Colin Watson on 2016-05-26
Status: Merged
Merged at revision: 18071
Proposed branch: lp:~cjwatson/launchpad/fix-buildmaster-twisted-agent
Merge into: lp:launchpad
Diff against target: 141 lines (+41/-9)
5 files modified
lib/lp/buildmaster/interactor.py (+14/-5)
lib/lp/buildmaster/manager.py (+2/-1)
lib/lp/buildmaster/model/buildfarmjobbehaviour.py (+4/-3)
lib/lp/buildmaster/tests/mock_slaves.py (+5/-0)
lib/lp/buildmaster/tests/test_interactor.py (+16/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/fix-buildmaster-twisted-agent
Reviewer Review Type Date Requested Status
Colin Watson Approve on 2016-05-26
Review via email: mp+295833@code.launchpad.net

Commit message

Fix BuilderSlave.getFiles to accept file-like objects again. Transferring build logs from slaves requires this.

Description of the change

Fix BuilderSlave.getFiles to accept file-like objects again. Transferring build logs from slaves requires this.

I also improved the logging a bit, so that we can see builder-side URLs again. The new downloader makes this a bit harder, since a factory is no longer associated with a single URL, but the results of this are reasonably tolerable on dogfood.

To post a comment you must log in.
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/interactor.py'
2--- lib/lp/buildmaster/interactor.py 2016-05-25 15:31:48 +0000
3+++ lib/lp/buildmaster/interactor.py 2016-05-26 14:04:28 +0000
4@@ -57,10 +57,14 @@
5 class FileWritingProtocol(Protocol):
6 """A protocol that saves data to a file."""
7
8- def __init__(self, finished, filename):
9+ def __init__(self, finished, file_to_write):
10 self.finished = finished
11- self.filename = filename
12- self.file = None
13+ if isinstance(file_to_write, (bytes, unicode)):
14+ self.filename = file_to_write
15+ self.file = None
16+ else:
17+ self.filename = None
18+ self.file = file_to_write
19
20 def dataReceived(self, data):
21 if self.file is None:
22@@ -77,7 +81,8 @@
23
24 def connectionLost(self, reason):
25 try:
26- self.file.close()
27+ if self.file is not None:
28+ self.file.close()
29 except IOError:
30 self.finished.errback()
31 else:
32@@ -224,6 +229,10 @@
33 'ensurepresent', sha1sum, url, username, password),
34 self.timeout * 5)
35
36+ def getURL(self, sha1):
37+ """Get the URL for a file on the builder with a given SHA-1."""
38+ return urlappend(self._file_cache_url, sha1).encode('utf8')
39+
40 def getFile(self, sha_sum, file_to_write):
41 """Fetch a file from the builder.
42
43@@ -234,7 +243,7 @@
44 :return: A Deferred that calls back when the download is done, or
45 errback with the error string.
46 """
47- file_url = urlappend(self._file_cache_url, sha_sum).encode('utf8')
48+ file_url = self.getURL(sha_sum)
49 d = Agent(self.reactor, pool=self.pool).request("GET", file_url)
50
51 def got_response(response):
52
53=== modified file 'lib/lp/buildmaster/manager.py'
54--- lib/lp/buildmaster/manager.py 2015-02-17 07:57:31 +0000
55+++ lib/lp/buildmaster/manager.py 2016-05-26 14:04:28 +0000
56@@ -1,4 +1,4 @@
57-# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
58+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
59 # GNU Affero General Public License version 3 (see the file LICENSE).
60
61 """Soyuz buildd slave manager logic."""
62@@ -8,6 +8,7 @@
63 __all__ = [
64 'BuilddManager',
65 'BUILDD_MANAGER_LOG_NAME',
66+ 'SlaveScanner',
67 ]
68
69 import datetime
70
71=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehaviour.py'
72--- lib/lp/buildmaster/model/buildfarmjobbehaviour.py 2015-09-23 21:44:36 +0000
73+++ lib/lp/buildmaster/model/buildfarmjobbehaviour.py 2016-05-26 14:04:28 +0000
74@@ -291,8 +291,9 @@
75 os.makedirs(upload_path)
76
77 filenames_to_download = []
78- for filename in filemap:
79- logger.info("Grabbing file: %s" % filename)
80+ for filename, sha1 in filemap.items():
81+ logger.info("Grabbing file: %s (%s)" % (
82+ filename, self._slave.getURL(sha1)))
83 out_file_name = os.path.join(upload_path, filename)
84 # If the evaluated output file name is not within our
85 # upload path, then we don't try to copy this or any
86@@ -300,7 +301,7 @@
87 if not os.path.realpath(out_file_name).startswith(upload_path):
88 raise BuildDaemonError(
89 "Build returned a file named %r." % filename)
90- filenames_to_download.append((filemap[filename], out_file_name))
91+ filenames_to_download.append((sha1, out_file_name))
92 yield self._slave.getFiles(filenames_to_download)
93
94 transaction.commit()
95
96=== modified file 'lib/lp/buildmaster/tests/mock_slaves.py'
97--- lib/lp/buildmaster/tests/mock_slaves.py 2016-05-25 15:31:48 +0000
98+++ lib/lp/buildmaster/tests/mock_slaves.py 2016-05-26 14:04:28 +0000
99@@ -37,6 +37,7 @@
100 from lp.buildmaster.interactor import BuilderSlave
101 from lp.buildmaster.interfaces.builder import CannotFetchFile
102 from lp.services.config import config
103+from lp.services.webapp import urlappend
104 from lp.testing.sampledata import I386_ARCHITECTURE_NAME
105
106
107@@ -136,6 +137,10 @@
108
109 return d.addCallback(check_present)
110
111+ def getURL(self, sha1):
112+ return urlappend(
113+ 'http://localhost:8221/filecache/', sha1).encode('utf8')
114+
115 def getFiles(self, files):
116 dl = defer.gatherResults([
117 self.getFile(builder_file, local_file)
118
119=== modified file 'lib/lp/buildmaster/tests/test_interactor.py'
120--- lib/lp/buildmaster/tests/test_interactor.py 2016-05-25 15:31:48 +0000
121+++ lib/lp/buildmaster/tests/test_interactor.py 2016-05-26 14:04:28 +0000
122@@ -862,3 +862,19 @@
123 dl.append(d)
124
125 return defer.DeferredList(dl).addCallback(finished_uploading)
126+
127+ @defer.inlineCallbacks
128+ def test_getFiles_with_file_objects(self):
129+ # getFiles works with file-like objects as well as file names.
130+ self.slave_helper.getServerSlave()
131+ slave = self.slave_helper.getClientSlave()
132+ temp_fd, temp_name = tempfile.mkstemp()
133+ self.addCleanup(os.remove, temp_name)
134+ lf = self.factory.makeLibraryFileAlias(
135+ 'content.txt', content='content')
136+ self.layer.txn.commit()
137+ yield slave.ensurepresent(lf.content.sha1, lf.http_url, "", "")
138+ yield slave.getFiles([(lf.content.sha1, os.fdopen(temp_fd, "w"))])
139+ with open(temp_name) as f:
140+ self.assertEqual('content', f.read())
141+ yield slave.pool.closeCachedConnections()