Launchpad itself

Merge lp:~jelmer/launchpad/bug499115 into lp:launchpad/db-devel

  1. bug499115
  2. Merge into db-devel
Proposed by Jelmer Vernooij
Status: Rejected
Rejected by: Jelmer Vernooij
Proposed branch: lp:~jelmer/launchpad/bug499115
Merge into: lp:launchpad/db-devel
Diff against target: 140 lines (+59/-18) (has conflicts)
2 files modified
lib/lp/buildmaster/model/buildbase.py (+30/-18)
lib/lp/buildmaster/tests/test_buildbase.py (+29/-0)
Text conflict in lib/lp/buildmaster/model/buildbase.py
To merge this branch: bzr merge lp:~jelmer/launchpad/bug499115
Reviewer Review Type Date Requested Status
Henning Eggers (community) code Needs Fixing
Review via email: mp+19328@code.launchpad.net

This proposal supersedes a proposal from 2010-02-11.

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote : Posted in a previous version of this proposal

This trivial change prevents the buildd master from falling over if a container directory was created but no log file uploaded. (bug 499115)

Revision history for this message
Henning Eggers (henninge) wrote :

Thank you for adding this simple fix. I still think this corner case should be tested in a unit test. To make testing possible, I'd suggest that you split out the lines under "# Retrieve log file content" into its own method "get_logfile_content" and add tests for it to test/test_bulidbase.py.

review: Needs Fixing (code)
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Superseded by https://code.edge.launchpad.net/~jelmer/launchpad/bug499115/+merge/20465

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/buildmaster/model/buildbase.py'
--- lib/lp/buildmaster/model/buildbase.py 2010-02-16 02:27:24 +0000
+++ lib/lp/buildmaster/model/buildbase.py 2010-03-03 02:43:34 +0000
@@ -3,6 +3,8 @@
33
4# pylint: disable-msg=E0211,E02134# pylint: disable-msg=E0211,E0213
55
6from __future__ import with_statement
7
6"""Common build base classes."""8"""Common build base classes."""
79
8__metaclass__ = type10__metaclass__ = type
@@ -32,6 +34,9 @@
32from lp.soyuz.model.buildqueue import BuildQueue34from lp.soyuz.model.buildqueue import BuildQueue
3335
3436
37UPLOAD_LOG_FILENAME = 'uploader.log'
38
39
35class BuildBase:40class BuildBase:
36 """A mixin class providing functionality for farm jobs that build a41 """A mixin class providing functionality for farm jobs that build a
37 package.42 package.
@@ -96,6 +101,7 @@
96 return None101 return None
97 return self._getProxiedFileURL(self.buildlog)102 return self._getProxiedFileURL(self.buildlog)
98103
104<<<<<<< TREE
99 @property105 @property
100 def upload_log_url(self):106 def upload_log_url(self):
101 """See `IBuildBase`."""107 """See `IBuildBase`."""
@@ -103,6 +109,27 @@
103 return None109 return None
104 return self._getProxiedFileURL(self.upload_log)110 return self._getProxiedFileURL(self.upload_log)
105111
112=======
113 def getUploadLogContent(self, root, leaf):
114 """Retrieve the upload log contents.
115
116 :param root: Root directory for the uploads
117 :param leaf: Leaf for this particular upload
118 :return: Contents of log file or message saying no log file was found.
119 """
120 # Retrieve log file content.
121 possible_locations = (
122 'failed', 'failed-to-move', 'rejected', 'accepted')
123 for location_dir in possible_locations:
124 log_filepath = os.path.join(root, location_dir, leaf,
125 UPLOAD_LOG_FILENAME)
126 if os.path.exists(log_filepath):
127 with open(log_filepath, 'r') as uploader_log_file:
128 return uploader_log_file.read()
129 else:
130 return 'Could not find upload log file'
131
132>>>>>>> MERGE-SOURCE
106 def handleStatus(self, status, librarian, slave_status):133 def handleStatus(self, status, librarian, slave_status):
107 """See `IBuildBase`."""134 """See `IBuildBase`."""
108 logger = logging.getLogger()135 logger = logging.getLogger()
@@ -161,7 +188,7 @@
161 out_file = open(out_file_name, "wb")188 out_file = open(out_file_name, "wb")
162 copy_and_close(slave_file, out_file)189 copy_and_close(slave_file, out_file)
163190
164 uploader_logfilename = os.path.join(upload_dir, 'uploader.log')191 uploader_logfilename = os.path.join(upload_dir, UPLOAD_LOG_FILENAME)
165 uploader_command = self.getUploaderCommand(192 uploader_command = self.getUploaderCommand(
166 upload_leaf, uploader_logfilename)193 upload_leaf, uploader_logfilename)
167 logger.debug("Saving uploader log at '%s'" % uploader_logfilename)194 logger.debug("Saving uploader log at '%s'" % uploader_logfilename)
@@ -239,23 +266,8 @@
239 not self.verifySuccessfulUpload()):266 not self.verifySuccessfulUpload()):
240 logger.debug("Build %s upload failed." % self.id)267 logger.debug("Build %s upload failed." % self.id)
241 self.buildstate = BuildStatus.FAILEDTOUPLOAD268 self.buildstate = BuildStatus.FAILEDTOUPLOAD
242 # Retrieve log file content.269 uploader_log_content = self.getUploadLogContent(root,
243 possible_locations = (270 upload_leaf)
244 'failed', 'failed-to-move', 'rejected', 'accepted')
245 for location_dir in possible_locations:
246 upload_final_location = os.path.join(
247 root, location_dir, upload_leaf)
248 if os.path.exists(upload_final_location):
249 log_filepath = os.path.join(
250 upload_final_location, 'uploader.log')
251 uploader_log_file = open(log_filepath)
252 try:
253 uploader_log_content = uploader_log_file.read()
254 finally:
255 uploader_log_file.close()
256 break
257 else:
258 uploader_log_content = 'Could not find upload log file'
259 # Store the upload_log_contents in librarian so it can be271 # Store the upload_log_contents in librarian so it can be
260 # accessed by anyone with permission to see the build.272 # accessed by anyone with permission to see the build.
261 self.storeUploadLog(uploader_log_content)273 self.storeUploadLog(uploader_log_content)
262274
=== modified file 'lib/lp/buildmaster/tests/test_buildbase.py'
--- lib/lp/buildmaster/tests/test_buildbase.py 2010-02-03 16:21:57 +0000
+++ lib/lp/buildmaster/tests/test_buildbase.py 2010-03-03 02:43:34 +0000
@@ -1,6 +1,8 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4from __future__ import with_statement
5
4"""Tests for `IBuildBase`."""6"""Tests for `IBuildBase`."""
57
6__metaclass__ = type8__metaclass__ = type
@@ -45,6 +47,33 @@
4547
46 layer = DatabaseFunctionalLayer48 layer = DatabaseFunctionalLayer
4749
50 def test_getUploadLogContent_nolog(self):
51 """If there is no log file there, a string explaining that is returned.
52 """
53 self.useTempDir()
54 build_base = BuildBase()
55 self.assertEquals('Could not find upload log file',
56 build_base.getUploadLogContent(os.getcwd(), "myleaf"))
57
58 def test_getUploadLogContent_only_dir(self):
59 """If there is a directory but no log file, expect the error string,
60 not an exception."""
61 self.useTempDir()
62 os.makedirs("accepted/myleaf")
63 build_base = BuildBase()
64 self.assertEquals('Could not find upload log file',
65 build_base.getUploadLogContent(os.getcwd(), "myleaf"))
66
67 def test_getUploadLogContent_readsfile(self):
68 """If there is a log file, return its contents."""
69 self.useTempDir()
70 os.makedirs("accepted/myleaf")
71 with open('accepted/myleaf/uploader.log', 'w') as f:
72 f.write('foo')
73 build_base = BuildBase()
74 self.assertEquals('foo',
75 build_base.getUploadLogContent(os.getcwd(), "myleaf"))
76
48 def test_getUploaderCommand(self):77 def test_getUploaderCommand(self):
49 build_base = BuildBase()78 build_base = BuildBase()
50 upload_leaf = self.factory.getUniqueString('upload-leaf')79 upload_leaf = self.factory.getUniqueString('upload-leaf')

Subscribers

People subscribed via source and target branches

to status/vote changes: