Merge lp:~jtv/launchpad/bug-777941 into lp:launchpad
- bug-777941
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Jeroen T. Vermeulen |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12989 |
Proposed branch: | lp:~jtv/launchpad/bug-777941 |
Merge into: | lp:launchpad |
Diff against target: |
1351 lines (+300/-628) 10 files modified
lib/canonical/config/schema-lazr.conf (+0/-10) lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py (+0/-26) lib/lp/archivepublisher/model/createdistroseriesindexesjob.py (+0/-155) lib/lp/archivepublisher/scripts/publish_ftpmaster.py (+82/-15) lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py (+0/-309) lib/lp/archivepublisher/tests/test_publish_ftpmaster.py (+214/-55) lib/lp/archivepublisher/zcml/configure.zcml (+0/-12) lib/lp/soyuz/interfaces/distributionjob.py (+0/-8) lib/lp/soyuz/scripts/initialise_distroseries.py (+0/-12) lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py (+4/-26) |
To merge this branch: | bzr merge lp:~jtv/launchpad/bug-777941 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
j.c.sackett (community) | Approve | ||
Review via email: mp+60116@code.launchpad.net |
Commit message
[r=jcsackett][bug=55211,777941] Create new distroseries indexes from publish-ftpmaster script.
Description of the change
= Summary =
Whenever we create a distroseries that we intend to publish, the publish-distro script needs to be run on it with the -A option to create its indexes in the filesystem.
== Proposed fix ==
This is currently a manual step in the Ubuntu release procedure. I had a new job type for it coded up, but we realized that there would be several advantages to running it from the publish-ftpmaster script instead:
* It's less work, of course. No need for a job type, config change, cron entry, etc.
* No more need to disable the cron jobs for fear that this might run concurrently with the cron script: it *is* the cron script, and it has a script lock.
* It's much more obvious how to deal with failure: better luck next time.
Most of the diff in this branch is deletion of the old, now unnecessary job work.
== Pre-implementation notes ==
The change was discussed with Julian, but also with Colin on the Ubuntu side. My job type sent notifications to the release manager, who could then (as per the release procedure) verify archive states manually. As it turns out, the release managers have no need for notifications because monitoring publisher progress is a routine part of the release procedure anyway. They will watch and verify as a matter of course.
Eventually we hope the need to disable cron jobs will disappear altogether. Until then, as Julian figured out, the Ubuntu case (which is the only distribution that we really want the verification step for) does not need the cron jobs disabled for verification. We can keep the publisher running regularly while waiting for the release managers to do the verification work.
== Implementation details ==
To ensure that we can absolutely, definitely, certainly, unequivocally, and reliably(*) see whether index creation has been completed, I made the script write a marker file in the archive root to indicate the fact. However the script only looks for this marker file for series that are in Frozen state (which happens when a series is created and again later for feature freeze). This will reduce the chance of indexes being re-created unnecessarily for existing series, or for series that are not in an inappropriate state. Julian agreed that this seemed sane.
(*) Unless the system went down at an awkward moment and left the filesystem in a really bad state. But then we'll probably have worse things to worry about.
Once this change is deployed, the current step 13 (have the LP team run publish-distro.py -A) can be dropped from the Ubuntu release procedure. Steps 12 & 14 (verification of the results) must be moved behind step 15 (re-enabling the cron jobs), and a wait of up to an hour and a half will be needed to ensure that the work has been done. We hope that disabling the cron jobs will become unnecessary altogether, but that also involves other changes.
== Tests ==
{{{
./bin/test -vvc lp.archivepubli
}}}
== Demo and Q/A ==
Create a new distroseries, in the Frozen state, and go through applicable steps of the Ubuntu release process.
Run cronscripts/
Run the script again. This time it will not log that it's creating archive indexes, but instead go through its regular publishing work.
= Launchpad lint =
I cleaned up some pre-existing lint. The stuff in schema-lazr.conf isn't something I can do much about.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/canonical
lib/lp/
lib/lp/
lib/lp/
lib/lp/
./lib/canonical
536: Line exceeds 78 characters.
619: Line exceeds 78 characters.
995: Line exceeds 78 characters.
1088: Line exceeds 78 characters.
Jeroen T. Vermeulen (jtv) wrote : | # |
Thanks, I implemented both changes.
Preview Diff
1 | === modified file 'lib/canonical/config/schema-lazr.conf' |
2 | --- lib/canonical/config/schema-lazr.conf 2011-05-03 04:39:43 +0000 |
3 | +++ lib/canonical/config/schema-lazr.conf 2011-05-06 03:11:24 +0000 |
4 | @@ -596,16 +596,6 @@ |
5 | licensing_policy_url: https://help.launchpad.net/Legal/ProjectLicensing |
6 | |
7 | |
8 | -[create_distroseries_indexes] |
9 | -# The database role that these jobs will run in. |
10 | -# datatype: string |
11 | -dbuser: archivepublisher |
12 | - |
13 | -# Utility interface to iterate jobs for job_runner to run. |
14 | -# datatype: string |
15 | -source_interface: lp.archivepublisher.interfaces.createdistroseriesindexesjob.ICreateDistroSeriesIndexesJobSource |
16 | - |
17 | - |
18 | [create_merge_proposals] |
19 | # The database user which will be used by this process. |
20 | # datatype: string |
21 | |
22 | === removed file 'lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py' |
23 | --- lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py 2011-04-27 08:20:37 +0000 |
24 | +++ lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py 1970-01-01 00:00:00 +0000 |
25 | @@ -1,26 +0,0 @@ |
26 | -# Copyright 2011 Canonical Ltd. This software is licensed under the |
27 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
28 | - |
29 | -"""`IJobSource` for `CreateDistroSeriesIndexesJob`.""" |
30 | - |
31 | -__metaclass__ = type |
32 | -__all__ = [ |
33 | - 'ICreateDistroSeriesIndexesJobSource', |
34 | - ] |
35 | - |
36 | -from lp.services.job.interfaces.job import IJobSource |
37 | - |
38 | - |
39 | -class ICreateDistroSeriesIndexesJobSource(IJobSource): |
40 | - """Create and manage `ICreateDistroSeriesIndexesJob`s.""" |
41 | - |
42 | - def makeFor(distroseries): |
43 | - """Create `ICreateDistroSeriesIndexesJob` if appropriate. |
44 | - |
45 | - If the `Distribution` that `distroseries` belongs to has no |
46 | - publisher configuration, no job will be returned. |
47 | - |
48 | - :param distroseries: A `DistroSeries` that needs its archive |
49 | - indexes created. |
50 | - :return: An `ICreateDistroSeriesIndexesJob`, or None. |
51 | - """ |
52 | |
53 | === removed file 'lib/lp/archivepublisher/model/createdistroseriesindexesjob.py' |
54 | --- lib/lp/archivepublisher/model/createdistroseriesindexesjob.py 2011-04-28 15:33:57 +0000 |
55 | +++ lib/lp/archivepublisher/model/createdistroseriesindexesjob.py 1970-01-01 00:00:00 +0000 |
56 | @@ -1,155 +0,0 @@ |
57 | -# Copyright 2011 Canonical Ltd. This software is licensed under the |
58 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
59 | - |
60 | -"""Job to create a distroseries' archive indexes.""" |
61 | - |
62 | -__metaclass__ = type |
63 | -__all__ = [ |
64 | - 'CreateDistroSeriesIndexesJob', |
65 | - ] |
66 | - |
67 | -from optparse import OptionParser |
68 | -from storm.locals import Store |
69 | -from textwrap import dedent |
70 | -import transaction |
71 | -from zope.component import getUtility |
72 | -from zope.interface import ( |
73 | - classProvides, |
74 | - implements, |
75 | - ) |
76 | - |
77 | -from canonical.config import config |
78 | -from canonical.launchpad.interfaces.lpstorm import IMasterStore |
79 | -from lp.archivepublisher.interfaces.createdistroseriesindexesjob import ( |
80 | - ICreateDistroSeriesIndexesJobSource, |
81 | - ) |
82 | -from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet |
83 | -from lp.registry.interfaces.pocket import pocketsuffix |
84 | -from lp.registry.model.person import get_recipients |
85 | -from lp.services.features import getFeatureFlag |
86 | -from lp.services.job.interfaces.job import IRunnableJob |
87 | -from lp.services.mail.sendmail import ( |
88 | - format_address_for_person, |
89 | - MailController, |
90 | - ) |
91 | -from lp.soyuz.interfaces.distributionjob import ( |
92 | - DistributionJobType, |
93 | - IDistributionJob, |
94 | - ) |
95 | -from lp.soyuz.model.distributionjob import ( |
96 | - DistributionJob, |
97 | - DistributionJobDerived, |
98 | - ) |
99 | -from lp.soyuz.scripts import publishdistro |
100 | - |
101 | - |
102 | -FEATURE_FLAG_ENABLE_MODULE = u"archivepublisher.auto_create_indexes.enabled" |
103 | - |
104 | - |
105 | -class CreateDistroSeriesIndexesJob(DistributionJobDerived): |
106 | - """Job to create a distroseries's archive indexes. |
107 | - |
108 | - To do this it runs publish-distro on the distribution, in careful mode. |
109 | - """ |
110 | - implements(IDistributionJob, IRunnableJob) |
111 | - classProvides(ICreateDistroSeriesIndexesJobSource) |
112 | - |
113 | - class_job_type = DistributionJobType.CREATE_DISTROSERIES_INDEXES |
114 | - |
115 | - @classmethod |
116 | - def create(cls, distroseries): |
117 | - job = DistributionJob( |
118 | - distroseries.distribution, distroseries, cls.class_job_type, |
119 | - metadata={}) |
120 | - IMasterStore(DistributionJob).add(job) |
121 | - return cls(job) |
122 | - |
123 | - @classmethod |
124 | - def makeFor(cls, distroseries): |
125 | - """See `ICreateDistroSeriesIndexesJob`.""" |
126 | - if not getFeatureFlag(FEATURE_FLAG_ENABLE_MODULE): |
127 | - return None |
128 | - |
129 | - distro = distroseries.distribution |
130 | - config_set = getUtility(IPublisherConfigSet) |
131 | - publisher_config = config_set.getByDistribution(distro) |
132 | - if publisher_config is None: |
133 | - return None |
134 | - else: |
135 | - return cls.create(distroseries) |
136 | - |
137 | - def run(self): |
138 | - """See `IRunnableJob`.""" |
139 | - self.runPublishDistro() |
140 | - if self.distribution.getArchiveByComponent('partner') is not None: |
141 | - self.runPublishDistro('--partner') |
142 | - |
143 | - self.notifySuccess() |
144 | - |
145 | - def getOperationDescription(self): |
146 | - """See `IRunnableJob`.""" |
147 | - return "initializing archive indexes for %s" % self.distroseries |
148 | - |
149 | - def getSuites(self): |
150 | - """List the suites for this `DistroSeries`.""" |
151 | - series_name = self.distroseries.name |
152 | - return [series_name + suffix for suffix in pocketsuffix.itervalues()] |
153 | - |
154 | - def runPublishDistro(self, extra_args=None): |
155 | - """Invoke the publish-distro script to create indexes. |
156 | - |
157 | - Publishes only the distroseries in question, in careful indices |
158 | - mode. |
159 | - """ |
160 | - arguments = [ |
161 | - "-A", |
162 | - "-d", self.distribution.name, |
163 | - ] |
164 | - for suite in self.getSuites(): |
165 | - arguments += ["-s", suite] |
166 | - if extra_args is not None: |
167 | - arguments.append(extra_args) |
168 | - |
169 | - parser = OptionParser() |
170 | - publishdistro.add_options(parser) |
171 | - options, args = parser.parse_args(arguments) |
172 | - publishdistro.run_publisher(options, transaction) |
173 | - |
174 | - def getMailRecipients(self): |
175 | - """List email addresses to notify of success or failure.""" |
176 | - recipient = self.distroseries.driver or self.distribution.owner |
177 | - return [ |
178 | - format_address_for_person(recipient) |
179 | - for person in get_recipients(recipient)] |
180 | - |
181 | - def notifySuccess(self): |
182 | - """Notify the distribution's owners of success.""" |
183 | - subject = "Launchpad has created archive indexes for %s %s" % ( |
184 | - self.distribution.displayname, self.distroseries.displayname) |
185 | - message = dedent("""\ |
186 | - You are receiving this email because you are registered in |
187 | - Launchpad as a release manager for %s. |
188 | - |
189 | - The archive indexes for %s have been successfully created. |
190 | - |
191 | - This automated process is one of many steps in setting up a |
192 | - new distribution release series in Launchpad. The fact that |
193 | - this part of the work is now done may mean that you can now |
194 | - proceed with subsequent steps. |
195 | - |
196 | - This is an automated email; please do not reply. Contact |
197 | - the Launchpad development team if you have any problems. |
198 | - """ % (self.distribution.displayname, self.distroseries.title)) |
199 | - from_addr = config.canonical.noreply_from_address |
200 | - MailController( |
201 | - from_addr, self.getMailRecipients(), subject, message).send() |
202 | - |
203 | - def getErrorRecipients(self): |
204 | - """See `BaseRunnableJob`.""" |
205 | - return self.getMailRecipients() |
206 | - |
207 | - def destroySelf(self): |
208 | - """See `IDistributionJob`.""" |
209 | - job = self.context.job |
210 | - Store.of(self.context).remove(self.context) |
211 | - Store.of(job).remove(job) |
212 | |
213 | === modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py' |
214 | --- lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2011-04-13 04:16:19 +0000 |
215 | +++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2011-05-06 03:11:24 +0000 |
216 | @@ -8,17 +8,22 @@ |
217 | 'PublishFTPMaster', |
218 | ] |
219 | |
220 | +from datetime import datetime |
221 | from optparse import OptionParser |
222 | import os |
223 | +from pytz import utc |
224 | from zope.component import getUtility |
225 | |
226 | from canonical.config import config |
227 | from lp.archivepublisher.config import getPubConfig |
228 | +from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet |
229 | from lp.registry.interfaces.distribution import IDistributionSet |
230 | +from lp.registry.interfaces.pocket import pocketsuffix |
231 | from lp.services.scripts.base import ( |
232 | LaunchpadCronScript, |
233 | LaunchpadScriptFailure, |
234 | ) |
235 | +from lp.registry.interfaces.series import SeriesStatus |
236 | from lp.services.utils import file_exists |
237 | from lp.soyuz.enums import ArchivePurpose |
238 | from lp.soyuz.scripts import publishdistro |
239 | @@ -94,6 +99,7 @@ |
240 | """ |
241 | return get_dists(archive_config) + ".in-progress" |
242 | |
243 | + |
244 | def extend_PATH(): |
245 | """Produce env dict for extending $PATH. |
246 | |
247 | @@ -222,6 +228,58 @@ |
248 | (archive.purpose, getPubConfig(archive)) |
249 | for archive in self.archives) |
250 | |
251 | + def locateIndexesMarker(self, distroseries): |
252 | + """Give path for marker file whose presence marks index creation. |
253 | + |
254 | + The file will be created once the archive indexes for |
255 | + `distroseries` have been created. This is how future runs will |
256 | + know that this work is done. |
257 | + """ |
258 | + archive_root = self.configs[ArchivePurpose.PRIMARY].archiveroot |
259 | + return os.path.join( |
260 | + archive_root, ".created-indexes-for-%s" % distroseries.name) |
261 | + |
262 | + def needsIndexesCreated(self, distroseries): |
263 | + """Does `distroseries` still need its archive indexes created? |
264 | + |
265 | + Checks for the marker left by `markIndexCreationComplete`. |
266 | + """ |
267 | + if distroseries.status != SeriesStatus.FROZEN: |
268 | + # DistroSeries are created in Frozen state. If the series |
269 | + # is in any other state yet has not been marked as having |
270 | + # its indexes created, that's because it predates automatic |
271 | + # index creation. |
272 | + return False |
273 | + distro = distroseries.distribution |
274 | + publisher_config_set = getUtility(IPublisherConfigSet) |
275 | + if publisher_config_set.getByDistribution(distro) is None: |
276 | + # We won't be able to do a thing without a publisher config, |
277 | + # but that's alright: we have those for all distributions |
278 | + # that we want to publish. |
279 | + return False |
280 | + return not file_exists(self.locateIndexesMarker(distroseries)) |
281 | + |
282 | + def markIndexCreationComplete(self, distroseries): |
283 | + """Note that archive indexes for `distroseries` have been created. |
284 | + |
285 | + This tells `needsIndexesCreated` that no, this series no longer needs |
286 | + archive indexes to be set up. |
287 | + """ |
288 | + with file(self.locateIndexesMarker(distroseries), "w") as marker: |
289 | + marker.write( |
290 | + "Indexes for %s were created on %s.\n" |
291 | + % (distroseries, datetime.now(utc))) |
292 | + |
293 | + def createIndexes(self, distroseries): |
294 | + """Create archive indexes for `distroseries`.""" |
295 | + self.logger.info( |
296 | + "Creating archive indexes for series %s.", distroseries) |
297 | + suites = [ |
298 | + distroseries.getSuite(pocket) |
299 | + for pocket in pocketsuffix.iterkeys()] |
300 | + self.runPublishDistro(args=['-A'], suites=suites) |
301 | + self.markIndexCreationComplete(distroseries) |
302 | + |
303 | def processAccepted(self): |
304 | """Run the process-accepted script.""" |
305 | self.logger.debug( |
306 | @@ -294,6 +352,20 @@ |
307 | "Creating backup dists directory %s", distscopy) |
308 | os.makedirs(distscopy) |
309 | |
310 | + def runPublishDistro(self, args=[], suites=None): |
311 | + """Execute `publish-distro`.""" |
312 | + if suites is None: |
313 | + suites = [] |
314 | + arguments = ( |
315 | + ['-d', self.distribution.name] + |
316 | + args + |
317 | + sum([['-s', suite] for suite in suites], [])) |
318 | + |
319 | + parser = OptionParser() |
320 | + publishdistro.add_options(parser) |
321 | + options, args = parser.parse_args(arguments) |
322 | + publishdistro.run_publisher(options, self.txn, log=self.logger) |
323 | + |
324 | def publishDistroArchive(self, archive, security_suites=None): |
325 | """Publish the results for an archive. |
326 | |
327 | @@ -311,26 +383,13 @@ |
328 | # for the duration. |
329 | temporary_dists = get_working_dists(archive_config) |
330 | |
331 | - arguments = [ |
332 | - '-v', '-v', |
333 | - '-d', self.distribution.name, |
334 | - '-R', temporary_dists, |
335 | - ] |
336 | - |
337 | + arguments = ['-R', temporary_dists] |
338 | if archive.purpose == ArchivePurpose.PARTNER: |
339 | arguments.append('--partner') |
340 | |
341 | - if security_suites is not None: |
342 | - arguments += sum([['-s', suite] for suite in security_suites], []) |
343 | - |
344 | - parser = OptionParser() |
345 | - publishdistro.add_options(parser) |
346 | - |
347 | os.rename(get_backup_dists(archive_config), temporary_dists) |
348 | try: |
349 | - options, args = parser.parse_args(arguments) |
350 | - publishdistro.run_publisher( |
351 | - options, txn=self.txn, log=self.logger) |
352 | + self.runPublishDistro(args=arguments, suites=security_suites) |
353 | finally: |
354 | os.rename(temporary_dists, get_backup_dists(archive_config)) |
355 | |
356 | @@ -495,6 +554,14 @@ |
357 | """See `LaunchpadScript`.""" |
358 | self.setUp() |
359 | self.recoverWorkingDists() |
360 | + |
361 | + for series in self.distribution.series: |
362 | + if self.needsIndexesCreated(series): |
363 | + self.createIndexes(series) |
364 | + # Don't try to do too much in one run. Leave the rest |
365 | + # of the work for next time. |
366 | + return |
367 | + |
368 | self.processAccepted() |
369 | self.setUpDirs() |
370 | |
371 | |
372 | === removed file 'lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py' |
373 | --- lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py 2011-04-28 15:33:57 +0000 |
374 | +++ lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py 1970-01-01 00:00:00 +0000 |
375 | @@ -1,309 +0,0 @@ |
376 | -# Copyright 2011 Canonical Ltd. This software is licensed under the |
377 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
378 | - |
379 | -"""Tests for `CreateDistroSeriesIndexesJob`.""" |
380 | - |
381 | -__metaclass__ = type |
382 | - |
383 | -from logging import ( |
384 | - FATAL, |
385 | - getLogger, |
386 | - ) |
387 | -import os.path |
388 | -from zope.component import getUtility |
389 | -from zope.security.proxy import removeSecurityProxy |
390 | - |
391 | -from canonical.config import config |
392 | -from canonical.launchpad.interfaces.lpstorm import IMasterStore |
393 | -from canonical.launchpad.webapp.testing import verifyObject |
394 | -from canonical.testing.layers import LaunchpadZopelessLayer |
395 | -from lp.archivepublisher.config import getPubConfig |
396 | -from lp.archivepublisher.interfaces.createdistroseriesindexesjob import ( |
397 | - ICreateDistroSeriesIndexesJobSource, |
398 | - ) |
399 | -from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet |
400 | -from lp.archivepublisher.model.createdistroseriesindexesjob import ( |
401 | - FEATURE_FLAG_ENABLE_MODULE, |
402 | - CreateDistroSeriesIndexesJob, |
403 | - ) |
404 | -from lp.registry.interfaces.pocket import pocketsuffix |
405 | -from lp.services.job.model.job import Job |
406 | -from lp.services.features.testing import FeatureFixture |
407 | -from lp.services.job.interfaces.job import ( |
408 | - IRunnableJob, |
409 | - JobStatus, |
410 | - ) |
411 | -from lp.services.job.runner import JobCronScript |
412 | -from lp.services.log.logger import DevNullLogger |
413 | -from lp.services.mail import stub |
414 | -from lp.services.mail.sendmail import format_address_for_person |
415 | -from lp.services.utils import file_exists |
416 | -from lp.soyuz.enums import ArchivePurpose |
417 | -from lp.soyuz.interfaces.distributionjob import IDistributionJob |
418 | -from lp.soyuz.model.distributionjob import DistributionJob |
419 | -from lp.testing import TestCaseWithFactory |
420 | -from lp.testing.fakemethod import FakeMethod |
421 | -from lp.testing.mail_helpers import run_mail_jobs |
422 | - |
423 | - |
424 | -def silence_publisher_logger(): |
425 | - """Silence the logger that `run_publisher` creates.""" |
426 | - getLogger("publish-distro").setLevel(FATAL) |
427 | - |
428 | - |
429 | -class TestCreateDistroSeriesIndexesJobSource(TestCaseWithFactory): |
430 | - """Test utility.""" |
431 | - |
432 | - layer = LaunchpadZopelessLayer |
433 | - |
434 | - def setUp(self): |
435 | - super(TestCreateDistroSeriesIndexesJobSource, self).setUp() |
436 | - self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u'on'})) |
437 | - |
438 | - def removePublisherConfig(self, distribution): |
439 | - """Strip `distribution` of its publisher configuration.""" |
440 | - publisher_config = getUtility(IPublisherConfigSet).getByDistribution( |
441 | - distribution) |
442 | - IMasterStore(publisher_config).remove(publisher_config) |
443 | - |
444 | - def test_baseline(self): |
445 | - # The utility conforms to the interfaces it claims to implement. |
446 | - jobsource = getUtility(ICreateDistroSeriesIndexesJobSource) |
447 | - self.assertTrue( |
448 | - verifyObject(ICreateDistroSeriesIndexesJobSource, jobsource)) |
449 | - |
450 | - def test_creates_job_for_distro_with_publisher_config(self): |
451 | - # The utility can create a job if the distribution has a |
452 | - # publisher configuration. |
453 | - distroseries = self.factory.makeDistroSeries() |
454 | - jobset = getUtility(ICreateDistroSeriesIndexesJobSource) |
455 | - job = jobset.makeFor(distroseries) |
456 | - self.assertIsInstance(job, CreateDistroSeriesIndexesJob) |
457 | - |
458 | - def test_does_not_create_job_for_distro_without_publisher_config(self): |
459 | - # If the distribution has no publisher configuration, the |
460 | - # utility creates no job for it. |
461 | - distroseries = self.factory.makeDistroSeries() |
462 | - self.removePublisherConfig(distroseries.distribution) |
463 | - jobset = getUtility(ICreateDistroSeriesIndexesJobSource) |
464 | - job = jobset.makeFor(distroseries) |
465 | - self.assertIs(None, job) |
466 | - |
467 | - def test_feature_flag_disables_feature(self): |
468 | - # The creation of jobs is controlled by a feature flag. |
469 | - self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u''})) |
470 | - jobset = getUtility(ICreateDistroSeriesIndexesJobSource) |
471 | - self.assertIs(None, jobset.makeFor(self.factory.makeDistroSeries())) |
472 | - |
473 | - |
474 | -class HorribleFailure(Exception): |
475 | - """A sample error for testing purposes.""" |
476 | - |
477 | - |
478 | -class TestCreateDistroSeriesIndexesJob(TestCaseWithFactory): |
479 | - """Test job class.""" |
480 | - |
481 | - layer = LaunchpadZopelessLayer |
482 | - |
483 | - def setUp(self): |
484 | - super(TestCreateDistroSeriesIndexesJob, self).setUp() |
485 | - self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u'on'})) |
486 | - |
487 | - def getJobSource(self): |
488 | - """Shorthand for getting at the job-source utility.""" |
489 | - return getUtility(ICreateDistroSeriesIndexesJobSource) |
490 | - |
491 | - def makeJob(self, distroseries=None): |
492 | - """Create an `CreateDistroSeriesIndexesJob`.""" |
493 | - if distroseries is None: |
494 | - distroseries = self.factory.makeDistroSeries() |
495 | - return self.getJobSource().makeFor(distroseries) |
496 | - |
497 | - def getDistsRoot(self, distribution): |
498 | - """Get distsroot directory for `distribution`.""" |
499 | - archive = removeSecurityProxy(distribution.main_archive) |
500 | - pub_config = getPubConfig(archive) |
501 | - return pub_config.distsroot |
502 | - |
503 | - def makeDistsDirs(self, distroseries): |
504 | - """Create dists directories in `distsroot` for `distroseries`.""" |
505 | - distsroot = self.getDistsRoot(distroseries.distribution) |
506 | - base = os.path.join(distsroot, distroseries.name) |
507 | - for suffix in pocketsuffix.itervalues(): |
508 | - os.makedirs(base + suffix) |
509 | - |
510 | - def makeCredibleJob(self): |
511 | - """Create a job with fixtures required for running it.""" |
512 | - silence_publisher_logger() |
513 | - distro = self.factory.makeDistribution( |
514 | - publish_root_dir=unicode(self.makeTemporaryDirectory())) |
515 | - distroseries = self.factory.makeDistroSeries(distribution=distro) |
516 | - self.makeDistsDirs(distroseries) |
517 | - return self.makeJob(distroseries) |
518 | - |
519 | - def becomeArchivePublisher(self): |
520 | - """Become the archive publisher database user (and clean up later).""" |
521 | - self.becomeDbUser(config.archivepublisher.dbuser) |
522 | - self.addCleanup(self.becomeDbUser, 'launchpad') |
523 | - |
524 | - def getSuites(self, distroseries): |
525 | - """Get the list of suites for `distroseries`.""" |
526 | - return [ |
527 | - distroseries.name + suffix |
528 | - for suffix in pocketsuffix.itervalues()] |
529 | - |
530 | - def test_baseline(self): |
531 | - # The job class conforms to the interfaces it claims to implement. |
532 | - job = self.makeJob() |
533 | - self.assertTrue(verifyObject(IRunnableJob, job)) |
534 | - self.assertTrue(verifyObject(IDistributionJob, job)) |
535 | - |
536 | - def test_getSuites_identifies_distroseries_suites(self): |
537 | - # getSuites lists all suites in the distroseries. |
538 | - job = self.makeJob() |
539 | - self.assertContentEqual( |
540 | - self.getSuites(job.distroseries), |
541 | - removeSecurityProxy(job).getSuites()) |
542 | - |
543 | - def test_getSuites_ignores_suites_for_other_distroseries(self): |
544 | - # getSuites does not list suites in the distribution that do not |
545 | - # belong to the right distroseries. |
546 | - job = self.makeJob() |
547 | - self.assertContentEqual( |
548 | - self.getSuites(job.distroseries), |
549 | - removeSecurityProxy(job).getSuites()) |
550 | - |
551 | - def test_job_runs_publish_distro_for_main(self): |
552 | - # The job always runs publish_distro for the distribution's main |
553 | - # archive. |
554 | - job = self.makeJob() |
555 | - naked_job = removeSecurityProxy(job) |
556 | - naked_job.runPublishDistro = FakeMethod() |
557 | - job.run() |
558 | - args, kwargs = naked_job.runPublishDistro.calls[-1] |
559 | - self.assertEqual((), args) |
560 | - |
561 | - def test_job_runs_publish_distro_for_partner_if_present(self): |
562 | - # If the distribution has a partner archive, the job will run |
563 | - # publish_distro for it. This differs from the run for the main |
564 | - # archive in that publish_distro receives the --partner option. |
565 | - distroseries = self.factory.makeDistroSeries() |
566 | - self.factory.makeArchive( |
567 | - distribution=distroseries.distribution, |
568 | - purpose=ArchivePurpose.PARTNER) |
569 | - job = self.makeJob(distroseries) |
570 | - naked_job = removeSecurityProxy(job) |
571 | - naked_job.runPublishDistro = FakeMethod() |
572 | - job.run() |
573 | - self.assertIn( |
574 | - ('--partner', ), |
575 | - [args for args, kwargs in naked_job.runPublishDistro.calls]) |
576 | - |
577 | - def test_job_does_not_run_publish_distro_for_partner_if_not_present(self): |
578 | - # If the distribution does not have a partner archive, |
579 | - # publish_distro is not run for the partner archive. |
580 | - job = self.makeJob() |
581 | - naked_job = removeSecurityProxy(job) |
582 | - naked_job.runPublishDistro = FakeMethod() |
583 | - job.run() |
584 | - self.assertEqual(1, naked_job.runPublishDistro.call_count) |
585 | - |
586 | - def test_job_notifies_if_successful(self): |
587 | - # Once the indexes have been created, the job calls its |
588 | - # notifySuccess method to let stakeholders know that they may |
589 | - # proceed with their release process. |
590 | - job = self.makeJob() |
591 | - naked_job = removeSecurityProxy(job) |
592 | - naked_job.runPublishDistro = FakeMethod() |
593 | - naked_job.notifySuccess = FakeMethod() |
594 | - job.run() |
595 | - self.assertEqual(1, naked_job.notifySuccess.call_count) |
596 | - |
597 | - def test_failure_notifies_recipients(self): |
598 | - # Failure notices are sent to the addresses returned by |
599 | - # getMailRecipients. |
600 | - job = self.makeJob() |
601 | - removeSecurityProxy(job).getMailRecipients = FakeMethod( |
602 | - result=["foo@example.com"]) |
603 | - job.notifyUserError(HorribleFailure("Boom!")) |
604 | - run_mail_jobs() |
605 | - sender, recipients, body = stub.test_emails.pop() |
606 | - self.assertIn("foo@example.com", recipients) |
607 | - |
608 | - def test_success_notifies_recipients(self): |
609 | - # Success notices are sent to the addresses returned by |
610 | - # getMailRecipients. |
611 | - job = self.makeJob() |
612 | - naked_job = removeSecurityProxy(job) |
613 | - naked_job.getMailRecipients = FakeMethod(result=["bar@example.com"]) |
614 | - naked_job.notifySuccess() |
615 | - run_mail_jobs() |
616 | - sender, recipients, body = stub.test_emails.pop() |
617 | - self.assertIn("bar@example.com", recipients) |
618 | - |
619 | - def test_notifySuccess_sends_email(self): |
620 | - # notifySuccess sends out a success notice by email. |
621 | - job = self.makeJob() |
622 | - removeSecurityProxy(job).notifySuccess() |
623 | - run_mail_jobs() |
624 | - sender, recipients, body = stub.test_emails.pop() |
625 | - self.assertIn("success", body) |
626 | - |
627 | - def test_release_manager_gets_notified(self): |
628 | - # The release manager gets notified. This role is represented |
629 | - # by the driver for the distroseries. |
630 | - distroseries = self.factory.makeDistroSeries() |
631 | - distroseries.driver = self.factory.makePerson() |
632 | - job = self.makeJob(distroseries) |
633 | - self.assertIn( |
634 | - format_address_for_person(distroseries.driver), |
635 | - removeSecurityProxy(job).getMailRecipients()) |
636 | - |
637 | - def test_distribution_owner_gets_notified_if_no_release_manager(self): |
638 | - # If no release manager is available, the distribution owners |
639 | - # are notified. |
640 | - distroseries = self.factory.makeDistroSeries() |
641 | - distroseries.driver = None |
642 | - job = self.makeJob(distroseries) |
643 | - self.assertIn( |
644 | - format_address_for_person(distroseries.distribution.owner), |
645 | - removeSecurityProxy(job).getMailRecipients()) |
646 | - |
647 | - def test_destroySelf_destroys_job(self): |
648 | - job = self.makeJob() |
649 | - job_id = removeSecurityProxy(job).job.id |
650 | - self.becomeArchivePublisher() |
651 | - job.destroySelf() |
652 | - store = IMasterStore(Job) |
653 | - self.assertIs(None, store.find(Job, Job.id == job_id).one()) |
654 | - |
655 | - def test_destroySelf_destroys_DistributionJob(self): |
656 | - job = self.makeJob() |
657 | - job_id = job.id |
658 | - self.becomeArchivePublisher() |
659 | - job.destroySelf() |
660 | - store = IMasterStore(DistributionJob) |
661 | - self.assertIs( |
662 | - None, |
663 | - store.find(DistributionJob, DistributionJob.id == job_id).one()) |
664 | - |
665 | - def test_run_does_the_job(self): |
666 | - # The job runs publish_distro and generates the expected output |
667 | - # files. |
668 | - job = self.makeCredibleJob() |
669 | - self.becomeArchivePublisher() |
670 | - job.run() |
671 | - distsroot = self.getDistsRoot(job.distribution) |
672 | - output = os.path.join(distsroot, job.distroseries.name, "Release") |
673 | - self.assertTrue(file_exists(output)) |
674 | - |
675 | - def test_job_runner_runs_jobs(self): |
676 | - # The generic job runner can set itself up to run these jobs. |
677 | - job = self.makeCredibleJob() |
678 | - script = JobCronScript( |
679 | - test_args=["create_distroseries_indexes"], |
680 | - commandline_config=True) |
681 | - script.logger = DevNullLogger() |
682 | - script.main() |
683 | - self.assertEqual( |
684 | - JobStatus.COMPLETED, removeSecurityProxy(job).context.job.status) |
685 | |
686 | === modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py' |
687 | --- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py 2011-04-13 04:16:19 +0000 |
688 | +++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py 2011-05-06 03:11:24 +0000 |
689 | @@ -8,11 +8,13 @@ |
690 | from apt_pkg import TagFile |
691 | import logging |
692 | import os |
693 | +from testtools.matchers import StartsWith |
694 | from textwrap import dedent |
695 | from zope.component import getUtility |
696 | |
697 | from canonical.config import config |
698 | from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities |
699 | +from canonical.launchpad.interfaces.lpstorm import IMasterStore |
700 | from canonical.testing.layers import ( |
701 | LaunchpadZopelessLayer, |
702 | ZopelessDatabaseLayer, |
703 | @@ -23,6 +25,7 @@ |
704 | PackagePublishingPocket, |
705 | pocketsuffix, |
706 | ) |
707 | +from lp.registry.interfaces.series import SeriesStatus |
708 | from lp.services.log.logger import ( |
709 | BufferLogger, |
710 | DevNullLogger, |
711 | @@ -125,6 +128,30 @@ |
712 | |
713 | self.addCleanup(config.pop, "run-parts") |
714 | |
715 | + def makeDistroWithPublishDirectory(self): |
716 | + """Create a `Distribution` for testing. |
717 | + |
718 | + The distribution will have a publishing directory set up, which |
719 | + will be cleaned up after the test. |
720 | + """ |
721 | + return self.factory.makeDistribution( |
722 | + publish_root_dir=unicode(self.makeTemporaryDirectory())) |
723 | + |
724 | + def makeScript(self, distro=None, extra_args=[]): |
725 | + """Produce instance of the `PublishFTPMaster` script.""" |
726 | + if distro is None: |
727 | + distro = self.makeDistroWithPublishDirectory() |
728 | + script = PublishFTPMaster(test_args=["-d", distro.name] + extra_args) |
729 | + script.txn = self.layer.txn |
730 | + script.logger = DevNullLogger() |
731 | + return script |
732 | + |
733 | + def setUpForScriptRun(self, distro): |
734 | + """Mock up config to run the script on `distro`.""" |
735 | + pub_config = getUtility(IPublisherConfigSet).getByDistribution(distro) |
736 | + pub_config.root_dir = unicode( |
737 | + self.makeTemporaryDirectory()) |
738 | + |
739 | |
740 | class TestPublishFTPMasterHelpers(TestCase): |
741 | |
742 | @@ -205,21 +232,6 @@ |
743 | # Location of shell script. |
744 | SCRIPT_PATH = "cronscripts/publish-ftpmaster.py" |
745 | |
746 | - def setUpForScriptRun(self, distro): |
747 | - """Mock up config to run the script on `distro`.""" |
748 | - pub_config = getUtility(IPublisherConfigSet).getByDistribution(distro) |
749 | - pub_config.root_dir = unicode( |
750 | - self.makeTemporaryDirectory()) |
751 | - |
752 | - def makeDistro(self): |
753 | - """Create a `Distribution` for testing. |
754 | - |
755 | - The distribution will have a publishing directory set up, which |
756 | - will be cleaned up after the test. |
757 | - """ |
758 | - return self.factory.makeDistribution( |
759 | - publish_root_dir=unicode(self.makeTemporaryDirectory())) |
760 | - |
761 | def prepareUbuntu(self): |
762 | """Obtain a reference to Ubuntu, set up for testing. |
763 | |
764 | @@ -230,15 +242,6 @@ |
765 | self.setUpForScriptRun(ubuntu) |
766 | return ubuntu |
767 | |
768 | - def makeScript(self, distro=None, extra_args=[]): |
769 | - """Produce instance of the `PublishFTPMaster` script.""" |
770 | - if distro is None: |
771 | - distro = self.makeDistro() |
772 | - script = PublishFTPMaster(test_args=["-d", distro.name] + extra_args) |
773 | - script.txn = self.layer.txn |
774 | - script.logger = DevNullLogger() |
775 | - return script |
776 | - |
777 | def readReleaseFile(self, filename): |
778 | """Read a Release file, return as a keyword/value dict.""" |
779 | sections = list(TagFile(file(filename))) |
780 | @@ -274,18 +277,18 @@ |
781 | os.chmod(script_path, 0755) |
782 | |
783 | def test_script_runs_successfully(self): |
784 | - ubuntu = self.prepareUbuntu() |
785 | + self.prepareUbuntu() |
786 | self.layer.txn.commit() |
787 | stdout, stderr, retval = run_script( |
788 | self.SCRIPT_PATH + " -d ubuntu") |
789 | self.assertEqual(0, retval, "Script failure:\n" + stderr) |
790 | |
791 | def test_script_is_happy_with_no_publications(self): |
792 | - distro = self.makeDistro() |
793 | + distro = self.makeDistroWithPublishDirectory() |
794 | self.makeScript(distro).main() |
795 | |
796 | def test_produces_listings(self): |
797 | - distro = self.makeDistro() |
798 | + distro = self.makeDistroWithPublishDirectory() |
799 | self.makeScript(distro).main() |
800 | self.assertTrue( |
801 | path_exists(get_archive_root(get_pub_config(distro)), 'ls-lR.gz')) |
802 | @@ -294,7 +297,6 @@ |
803 | test_publisher = SoyuzTestPublisher() |
804 | distroseries = test_publisher.setUpDefaultDistroSeries() |
805 | distro = distroseries.distribution |
806 | - pub_config = get_pub_config(distro) |
807 | self.factory.makeComponentSelection( |
808 | distroseries=distroseries, component="main") |
809 | self.factory.makeArchive( |
810 | @@ -363,7 +365,7 @@ |
811 | self.assertEqual([name_spph_suite(spph)], script.getDirtySuites()) |
812 | |
813 | def test_getDirtySuites_returns_suites_with_pending_publications(self): |
814 | - distro = self.makeDistro() |
815 | + distro = self.makeDistroWithPublishDirectory() |
816 | spphs = [ |
817 | self.factory.makeSourcePackagePublishingHistory( |
818 | distroseries=self.factory.makeDistroSeries( |
819 | @@ -384,7 +386,7 @@ |
820 | self.assertEqual([], script.getDirtySuites()) |
821 | |
822 | def test_getDirtySecuritySuites_returns_security_suites(self): |
823 | - distro = self.makeDistro() |
824 | + distro = self.makeDistroWithPublishDirectory() |
825 | spphs = [ |
826 | self.factory.makeSourcePackagePublishingHistory( |
827 | distroseries=self.factory.makeDistroSeries( |
828 | @@ -400,21 +402,21 @@ |
829 | |
830 | def test_getDirtySecuritySuites_ignores_non_security_suites(self): |
831 | distroseries = self.factory.makeDistroSeries() |
832 | - spphs = [ |
833 | + pockets = [ |
834 | + PackagePublishingPocket.RELEASE, |
835 | + PackagePublishingPocket.UPDATES, |
836 | + PackagePublishingPocket.PROPOSED, |
837 | + PackagePublishingPocket.BACKPORTS, |
838 | + ] |
839 | + for pocket in pockets: |
840 | self.factory.makeSourcePackagePublishingHistory( |
841 | distroseries=distroseries, pocket=pocket) |
842 | - for pocket in [ |
843 | - PackagePublishingPocket.RELEASE, |
844 | - PackagePublishingPocket.UPDATES, |
845 | - PackagePublishingPocket.PROPOSED, |
846 | - PackagePublishingPocket.BACKPORTS, |
847 | - ]] |
848 | script = self.makeScript(distroseries.distribution) |
849 | script.setUp() |
850 | self.assertEqual([], script.getDirtySecuritySuites()) |
851 | |
852 | def test_rsync_copies_files(self): |
853 | - distro = self.makeDistro() |
854 | + distro = self.makeDistroWithPublishDirectory() |
855 | script = self.makeScript(distro) |
856 | script.setUp() |
857 | dists_root = get_dists_root(get_pub_config(distro)) |
858 | @@ -428,7 +430,7 @@ |
859 | "New file", read_marker_file([dists_backup, "new-file"])) |
860 | |
861 | def test_rsync_cleans_up_obsolete_files(self): |
862 | - distro = self.makeDistro() |
863 | + distro = self.makeDistroWithPublishDirectory() |
864 | script = self.makeScript(distro) |
865 | script.setUp() |
866 | dists_backup = os.path.join( |
867 | @@ -441,7 +443,7 @@ |
868 | self.assertFalse(path_exists(*old_file)) |
869 | |
870 | def test_setUpDirs_creates_directory_structure(self): |
871 | - distro = self.makeDistro() |
872 | + distro = self.makeDistroWithPublishDirectory() |
873 | pub_config = get_pub_config(distro) |
874 | archive_root = get_archive_root(pub_config) |
875 | dists_root = get_dists_root(pub_config) |
876 | @@ -457,7 +459,7 @@ |
877 | self.assertTrue(file_exists(get_distscopy_root(pub_config))) |
878 | |
879 | def test_setUpDirs_does_not_mind_if_dist_directories_already_exist(self): |
880 | - distro = self.makeDistro() |
881 | + distro = self.makeDistroWithPublishDirectory() |
882 | script = self.makeScript(distro) |
883 | script.setUp() |
884 | script.setUpDirs() |
885 | @@ -465,7 +467,7 @@ |
886 | self.assertTrue(file_exists(get_archive_root(get_pub_config(distro)))) |
887 | |
888 | def test_publishDistroArchive_runs_parts(self): |
889 | - distro = self.makeDistro() |
890 | + distro = self.makeDistroWithPublishDirectory() |
891 | script = self.makeScript(distro) |
892 | script.setUp() |
893 | script.setUpDirs() |
894 | @@ -477,7 +479,7 @@ |
895 | self.assertEqual("publish-distro.d", parts_dir) |
896 | |
897 | def test_runPublishDistroParts_passes_parameters(self): |
898 | - distro = self.makeDistro() |
899 | + distro = self.makeDistroWithPublishDirectory() |
900 | script = self.makeScript(distro) |
901 | script.setUp() |
902 | script.setUpDirs() |
903 | @@ -507,7 +509,7 @@ |
904 | # XXX JeroenVermeulen 2011-03-29 bug=741683: Retire |
905 | # runCommercialCompat as soon as Dapper support ends. |
906 | self.enableCommercialCompat() |
907 | - script = self.makeScript(self.makeDistro()) |
908 | + script = self.makeScript(self.makeDistroWithPublishDirectory()) |
909 | script.setUp() |
910 | script.executeShell = FakeMethod() |
911 | script.runCommercialCompat() |
912 | @@ -523,7 +525,7 @@ |
913 | self.assertEqual(0, script.executeShell.call_count) |
914 | |
915 | def test_generateListings_writes_ls_lR_gz(self): |
916 | - distro = self.makeDistro() |
917 | + distro = self.makeDistroWithPublishDirectory() |
918 | script = self.makeScript(distro) |
919 | script.setUp() |
920 | script.setUpDirs() |
921 | @@ -531,7 +533,7 @@ |
922 | pass |
923 | |
924 | def test_clearEmptyDirs_cleans_up_empty_directories(self): |
925 | - distro = self.makeDistro() |
926 | + distro = self.makeDistroWithPublishDirectory() |
927 | script = self.makeScript(distro) |
928 | script.setUp() |
929 | script.setUpDirs() |
930 | @@ -542,7 +544,7 @@ |
931 | self.assertFalse(file_exists(empty_dir)) |
932 | |
933 | def test_clearEmptyDirs_does_not_clean_up_nonempty_directories(self): |
934 | - distro = self.makeDistro() |
935 | + distro = self.makeDistroWithPublishDirectory() |
936 | script = self.makeScript(distro) |
937 | script.setUp() |
938 | script.setUpDirs() |
939 | @@ -554,7 +556,7 @@ |
940 | self.assertTrue(file_exists(nonempty_dir)) |
941 | |
942 | def test_processOptions_finds_distribution(self): |
943 | - distro = self.makeDistro() |
944 | + distro = self.makeDistroWithPublishDirectory() |
945 | script = self.makeScript(distro) |
946 | script.processOptions() |
947 | self.assertEqual(distro.name, script.options.distribution) |
948 | @@ -592,7 +594,7 @@ |
949 | self.assertIn("%s=%s" % (key, value), command_line) |
950 | |
951 | def test_executeShell_executes_shell_command(self): |
952 | - distro = self.makeDistro() |
953 | + distro = self.makeDistroWithPublishDirectory() |
954 | script = self.makeScript(distro) |
955 | marker = os.path.join( |
956 | get_pub_config(distro).root_dir, "marker") |
957 | @@ -600,7 +602,7 @@ |
958 | self.assertTrue(file_exists(marker)) |
959 | |
960 | def test_executeShell_reports_failure_if_requested(self): |
961 | - distro = self.makeDistro() |
962 | + distro = self.makeDistroWithPublishDirectory() |
963 | script = self.makeScript(distro) |
964 | |
965 | class ArbitraryFailure(Exception): |
966 | @@ -611,7 +613,7 @@ |
967 | script.executeShell, "/bin/false", failure=ArbitraryFailure()) |
968 | |
969 | def test_executeShell_does_not_report_failure_if_not_requested(self): |
970 | - distro = self.makeDistro() |
971 | + distro = self.makeDistroWithPublishDirectory() |
972 | script = self.makeScript(distro) |
973 | # The test is that this does not fail: |
974 | script.executeShell("/bin/false") |
975 | @@ -636,7 +638,7 @@ |
976 | self.assertEqual(0, script.installDists.call_count) |
977 | |
978 | def test_publishAllUploads_publishes_all_distro_archives(self): |
979 | - distro = self.makeDistro() |
980 | + distro = self.makeDistroWithPublishDirectory() |
981 | distroseries = self.factory.makeDistroSeries(distribution=distro) |
982 | partner_archive = self.factory.makeArchive( |
983 | distribution=distro, purpose=ArchivePurpose.PARTNER) |
984 | @@ -666,7 +668,7 @@ |
985 | self.assertEqual('', script.logger.getLogBuffer()) |
986 | |
987 | def test_recoverWorkingDists_recovers_working_directory(self): |
988 | - distro = self.makeDistro() |
989 | + distro = self.makeDistroWithPublishDirectory() |
990 | script = self.makeScript(distro) |
991 | script.setUp() |
992 | script.logger = BufferLogger() |
993 | @@ -702,7 +704,7 @@ |
994 | self.assertEqual({'security_only': True}, kwargs) |
995 | |
996 | def test_publishAllUploads_processes_all_archives(self): |
997 | - distro = self.makeDistro() |
998 | + distro = self.makeDistroWithPublishDirectory() |
999 | partner_archive = self.factory.makeArchive( |
1000 | distribution=distro, purpose=ArchivePurpose.PARTNER) |
1001 | script = self.makeScript(distro) |
1002 | @@ -723,7 +725,7 @@ |
1003 | # list. It'll probably go wrong if the configured archive root |
1004 | # contains spaces and such, but should work with Unix-sensible |
1005 | # paths. |
1006 | - distro = self.makeDistro() |
1007 | + distro = self.makeDistroWithPublishDirectory() |
1008 | self.factory.makeArchive( |
1009 | distribution=distro, purpose=ArchivePurpose.PARTNER) |
1010 | script = self.makeScript(distro) |
1011 | @@ -759,3 +761,160 @@ |
1012 | read_marker_file([archive_root, "marker file"]).rstrip(), |
1013 | "Did not find expected marker for %s." |
1014 | % archive.purpose.title) |
1015 | + |
1016 | + |
1017 | +class TestCreateDistroSeriesIndexes(TestCaseWithFactory, HelpersMixin): |
1018 | + """Test initial creation of archive indexes for a `DistroSeries`.""" |
1019 | + layer = LaunchpadZopelessLayer |
1020 | + |
1021 | + def createIndexesMarkerDir(self, script, distroseries): |
1022 | + """Create the directory for `distroseries`'s indexes marker.""" |
1023 | + marker = script.locateIndexesMarker(distroseries) |
1024 | + os.makedirs(os.path.dirname(marker)) |
1025 | + |
1026 | + def test_new_frozen_series_needs_indexes_created(self): |
1027 | + # If a distroseries is Frozen and has not had its indexes |
1028 | + # created yet, needsIndexesCreated returns True for it. |
1029 | + series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN) |
1030 | + script = self.makeScript(series.distribution) |
1031 | + script.setUp() |
1032 | + self.assertTrue(script.needsIndexesCreated(series)) |
1033 | + |
1034 | + def test_new_nonfrozen_series_does_not_need_indexes_created(self): |
1035 | + # needsIndexesCreated only returns True for Frozen distroseries. |
1036 | + series = self.factory.makeDistroSeries() |
1037 | + script = self.makeScript(series.distribution) |
1038 | + self.assertFalse(script.needsIndexesCreated(series)) |
1039 | + |
1040 | + def test_distro_without_publisher_config_gets_no_indexes(self): |
1041 | + # needsIndexesCreated returns False for distributions that have |
1042 | + # no publisher config, such as Debian. We don't want to publish |
1043 | + # these distributions. |
1044 | + series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN) |
1045 | + pub_config = get_pub_config(series.distribution) |
1046 | + IMasterStore(pub_config).remove(pub_config) |
1047 | + script = self.makeScript(series.distribution) |
1048 | + self.assertFalse(script.needsIndexesCreated(series)) |
1049 | + |
1050 | + def test_markIndexCreationComplete_tells_needsIndexesCreated_no(self): |
1051 | + # The effect of markIndexCreationComplete is to make |
1052 | + # needsIndexesCreated for that distroseries return False. |
1053 | + distro = self.makeDistroWithPublishDirectory() |
1054 | + series = self.factory.makeDistroSeries( |
1055 | + status=SeriesStatus.FROZEN, distribution=distro) |
1056 | + script = self.makeScript(distro) |
1057 | + script.setUp() |
1058 | + self.createIndexesMarkerDir(script, series) |
1059 | + |
1060 | + self.assertTrue(script.needsIndexesCreated(series)) |
1061 | + script.markIndexCreationComplete(series) |
1062 | + self.assertFalse(script.needsIndexesCreated(series)) |
1063 | + |
1064 | + def test_createIndexes_marks_index_creation_complete(self): |
1065 | + # createIndexes calls markIndexCreationComplete for the |
1066 | + # distroseries. |
1067 | + distro = self.makeDistroWithPublishDirectory() |
1068 | + series = self.factory.makeDistroSeries(distribution=distro) |
1069 | + script = self.makeScript(distro) |
1070 | + script.markIndexCreationComplete = FakeMethod() |
1071 | + script.runPublishDistro = FakeMethod() |
1072 | + script.createIndexes(series) |
1073 | + self.assertEqual( |
1074 | + [((series, ), {})], script.markIndexCreationComplete.calls) |
1075 | + |
1076 | + def test_failed_index_creation_is_not_marked_complete(self): |
1077 | + # If index creation fails, it is not marked as having been |
1078 | + # completed. The next run will retry. |
1079 | + class Boom(Exception): |
1080 | + """Simulated failure.""" |
1081 | + |
1082 | + series = self.factory.makeDistroSeries() |
1083 | + script = self.makeScript(series.distribution) |
1084 | + script.markIndexCreationComplete = FakeMethod() |
1085 | + script.runPublishDistro = FakeMethod(failure=Boom("Sorry!")) |
1086 | + try: |
1087 | + script.createIndexes(series) |
1088 | + except: |
1089 | + pass |
1090 | + self.assertEqual([], script.markIndexCreationComplete.calls) |
1091 | + |
1092 | + def test_locateIndexesMarker_places_file_in_archive_root(self): |
1093 | + # The marker file for index creation is in the distribution's |
1094 | + # archive root. |
1095 | + series = self.factory.makeDistroSeries() |
1096 | + script = self.makeScript(series.distribution) |
1097 | + script.setUp() |
1098 | + archive_root = script.configs[ArchivePurpose.PRIMARY].archiveroot |
1099 | + self.assertThat( |
1100 | + script.locateIndexesMarker(series), |
1101 | + StartsWith(os.path.normpath(archive_root))) |
1102 | + |
1103 | + def test_locateIndexesMarker_uses_separate_files_per_series(self): |
1104 | + # Different release series of the same distribution get separate |
1105 | + # marker files for index creation. |
1106 | + distro = self.makeDistroWithPublishDirectory() |
1107 | + series1 = self.factory.makeDistroSeries(distribution=distro) |
1108 | + series2 = self.factory.makeDistroSeries(distribution=distro) |
1109 | + script = self.makeScript(distro) |
1110 | + script.setUp() |
1111 | + self.assertNotEqual( |
1112 | + script.locateIndexesMarker(series1), |
1113 | + script.locateIndexesMarker(series2)) |
1114 | + |
1115 | + def test_locateIndexMarker_uses_hidden_file(self): |
1116 | + # The index-creation marker file is a "dot file," so it's not |
1117 | + # visible in normal directory listings. |
1118 | + series = self.factory.makeDistroSeries() |
1119 | + script = self.makeScript(series.distribution) |
1120 | + script.setUp() |
1121 | + self.assertThat( |
1122 | + os.path.basename(script.locateIndexesMarker(series)), |
1123 | + StartsWith(".")) |
1124 | + |
1125 | + def test_script_calls_createIndexes_for_new_series(self): |
1126 | + # If the script's main() finds a distroseries that needs its |
1127 | + # indexes created, it calls createIndexes on that distroseries. |
1128 | + distro = self.makeDistroWithPublishDirectory() |
1129 | + series = self.factory.makeDistroSeries( |
1130 | + status=SeriesStatus.FROZEN, distribution=distro) |
1131 | + script = self.makeScript(distro) |
1132 | + script.createIndexes = FakeMethod() |
1133 | + script.main() |
1134 | + self.assertEqual([((series, ), {})], script.createIndexes.calls) |
1135 | + |
1136 | + def test_createIndexes_ignores_other_series(self): |
1137 | + # createIndexes does not accidentally also touch other |
1138 | + # distroseries than the one it's meant to. |
1139 | + distro = self.makeDistroWithPublishDirectory() |
1140 | + series = self.factory.makeDistroSeries(distribution=distro) |
1141 | + self.factory.makeDistroSeries(distribution=distro) |
1142 | + script = self.makeScript(distro) |
1143 | + script.setUp() |
1144 | + script.runPublishDistro = FakeMethod() |
1145 | + self.createIndexesMarkerDir(script, series) |
1146 | + |
1147 | + script.createIndexes(series) |
1148 | + |
1149 | + args, kwargs = script.runPublishDistro.calls[0] |
1150 | + suites = kwargs['suites'] |
1151 | + self.assertEqual(len(pocketsuffix), len(suites)) |
1152 | + for suite in suites: |
1153 | + self.assertThat(suite, StartsWith(series.name)) |
1154 | + |
1155 | + def test_script_creates_indexes(self): |
1156 | + # End-to-end test: the script creates indexes for distroseries |
1157 | + # that need them. |
1158 | + test_publisher = SoyuzTestPublisher() |
1159 | + series = test_publisher.setUpDefaultDistroSeries() |
1160 | + series.status = SeriesStatus.FROZEN |
1161 | + self.factory.makeComponentSelection( |
1162 | + distroseries=series, component="main") |
1163 | + self.layer.txn.commit() |
1164 | + self.setUpForScriptRun(series.distribution) |
1165 | + script = self.makeScript(series.distribution) |
1166 | + script.main() |
1167 | + self.assertFalse(script.needsIndexesCreated(series)) |
1168 | + sources = os.path.join( |
1169 | + script.configs[ArchivePurpose.PRIMARY].distsroot, |
1170 | + series.name, "main", "source", "Sources") |
1171 | + self.assertTrue(file_exists(sources)) |
1172 | |
1173 | === modified file 'lib/lp/archivepublisher/zcml/configure.zcml' |
1174 | --- lib/lp/archivepublisher/zcml/configure.zcml 2011-04-26 04:31:38 +0000 |
1175 | +++ lib/lp/archivepublisher/zcml/configure.zcml 2011-05-06 03:11:24 +0000 |
1176 | @@ -28,16 +28,4 @@ |
1177 | set_schema="lp.archivepublisher.interfaces.publisherconfig.IPublisherConfig"/> |
1178 | </class> |
1179 | |
1180 | - <class class="lp.archivepublisher.model.createdistroseriesindexesjob.CreateDistroSeriesIndexesJob"> |
1181 | - <allow |
1182 | - interface="lp.soyuz.interfaces.distributionjob.IDistributionJob"/> |
1183 | - <allow interface="lp.services.job.interfaces.job.IRunnableJob"/> |
1184 | - </class> |
1185 | - <securedutility |
1186 | - component="lp.archivepublisher.model.createdistroseriesindexesjob.CreateDistroSeriesIndexesJob" |
1187 | - provides="lp.archivepublisher.interfaces.createdistroseriesindexesjob.ICreateDistroSeriesIndexesJobSource"> |
1188 | - <allow |
1189 | - interface="lp.archivepublisher.interfaces.createdistroseriesindexesjob.ICreateDistroSeriesIndexesJobSource"/> |
1190 | - </securedutility> |
1191 | - |
1192 | </configure> |
1193 | |
1194 | === modified file 'lib/lp/soyuz/interfaces/distributionjob.py' |
1195 | --- lib/lp/soyuz/interfaces/distributionjob.py 2011-04-29 07:37:38 +0000 |
1196 | +++ lib/lp/soyuz/interfaces/distributionjob.py 2011-05-06 03:11:24 +0000 |
1197 | @@ -28,7 +28,6 @@ |
1198 | Int, |
1199 | List, |
1200 | Object, |
1201 | - TextLine, |
1202 | Tuple, |
1203 | ) |
1204 | |
1205 | @@ -89,13 +88,6 @@ |
1206 | distribution release series and its parent series. |
1207 | """) |
1208 | |
1209 | - CREATE_DISTROSERIES_INDEXES = DBItem(4, """ |
1210 | - Set up a series' archive indexes. |
1211 | - |
1212 | - Performans an initial run of the publish-distro script to |
1213 | - create indexes for the distroseries. |
1214 | - """) |
1215 | - |
1216 | |
1217 | class IInitialiseDistroSeriesJobSource(IJobSource): |
1218 | """An interface for acquiring IInitialiseDistroSeriesJobs.""" |
1219 | |
1220 | === modified file 'lib/lp/soyuz/scripts/initialise_distroseries.py' |
1221 | --- lib/lp/soyuz/scripts/initialise_distroseries.py 2011-04-26 05:04:45 +0000 |
1222 | +++ lib/lp/soyuz/scripts/initialise_distroseries.py 2011-05-06 03:11:24 +0000 |
1223 | @@ -17,9 +17,6 @@ |
1224 | from canonical.database.sqlbase import sqlvalues |
1225 | from canonical.launchpad.helpers import ensure_unicode |
1226 | from canonical.launchpad.interfaces.lpstorm import IMasterStore |
1227 | -from lp.archivepublisher.interfaces.createdistroseriesindexesjob import ( |
1228 | - ICreateDistroSeriesIndexesJobSource, |
1229 | - ) |
1230 | from lp.buildmaster.enums import BuildStatus |
1231 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
1232 | from lp.soyuz.adapters.packagelocation import PackageLocation |
1233 | @@ -56,9 +53,6 @@ |
1234 | selections will be duplicated, as will any permission-related |
1235 | structures. |
1236 | |
1237 | - A `CreateDistroSeriesIndexesJob` is created if appropriate, so that |
1238 | - the series' archive indexes will be initialized. |
1239 | - |
1240 | Note: |
1241 | This method will raise a InitialisationError when the pre-conditions |
1242 | are not met. After this is run, you still need to construct chroots |
1243 | @@ -143,7 +137,6 @@ |
1244 | self._copy_architectures() |
1245 | self._copy_packages() |
1246 | self._copy_packagesets() |
1247 | - self._request_index_creation() |
1248 | transaction.commit() |
1249 | |
1250 | def _set_parent(self): |
1251 | @@ -336,8 +329,3 @@ |
1252 | new_series_ps.add(parent_to_child[old_series_child]) |
1253 | new_series_ps.add(old_series_ps.sourcesIncluded( |
1254 | direct_inclusion=True)) |
1255 | - |
1256 | - def _request_index_creation(self): |
1257 | - """Schedule a `CreateDistroSeriesIndexesJob` if appropriate.""" |
1258 | - getUtility(ICreateDistroSeriesIndexesJobSource).makeFor( |
1259 | - self.distroseries) |
1260 | |
1261 | === modified file 'lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py' |
1262 | --- lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py 2011-04-28 13:21:34 +0000 |
1263 | +++ lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py 2011-05-06 03:11:24 +0000 |
1264 | @@ -9,7 +9,6 @@ |
1265 | import subprocess |
1266 | import sys |
1267 | |
1268 | -from storm.locals import Store |
1269 | from testtools.content import Content |
1270 | from testtools.content_type import UTF8_TEXT |
1271 | import transaction |
1272 | @@ -18,10 +17,8 @@ |
1273 | from canonical.config import config |
1274 | from canonical.launchpad.interfaces.lpstorm import IStore |
1275 | from canonical.testing.layers import LaunchpadZopelessLayer |
1276 | -from lp.archivepublisher.model import createdistroseriesindexesjob |
1277 | from lp.buildmaster.enums import BuildStatus |
1278 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
1279 | -from lp.services.features.testing import FeatureFixture |
1280 | from lp.soyuz.enums import SourcePackageFormat |
1281 | from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet |
1282 | from lp.soyuz.interfaces.packageset import ( |
1283 | @@ -32,10 +29,6 @@ |
1284 | from lp.soyuz.interfaces.sourcepackageformat import ( |
1285 | ISourcePackageFormatSelectionSet, |
1286 | ) |
1287 | -from lp.soyuz.model.distributionjob import ( |
1288 | - DistributionJob, |
1289 | - DistributionJobType, |
1290 | - ) |
1291 | from lp.soyuz.model.distroarchseries import DistroArchSeries |
1292 | from lp.soyuz.scripts.initialise_distroseries import ( |
1293 | InitialisationError, |
1294 | @@ -258,7 +251,7 @@ |
1295 | # When initialising a new series within a distro, the copied |
1296 | # packagesets have ownership preserved. |
1297 | ps_owner = self.factory.makePerson() |
1298 | - ps = getUtility(IPackagesetSet).new( |
1299 | + getUtility(IPackagesetSet).new( |
1300 | u'ps', u'packageset', ps_owner, distroseries=self.parent) |
1301 | child = self._full_initialise(distribution=self.parent.distribution) |
1302 | child_ps = getUtility(IPackagesetSet).getByName( |
1303 | @@ -268,7 +261,7 @@ |
1304 | def test_packageset_owner_not_preserved_cross_distro(self): |
1305 | # In the case of a cross-distro initialisation, the new |
1306 | # packagesets are owned by the new distro owner. |
1307 | - ps = getUtility(IPackagesetSet).new( |
1308 | + getUtility(IPackagesetSet).new( |
1309 | u'ps', u'packageset', self.factory.makePerson(), |
1310 | distroseries=self.parent) |
1311 | child = self._full_initialise() |
1312 | @@ -282,7 +275,7 @@ |
1313 | test1 = getUtility(IPackagesetSet).new( |
1314 | u'test1', u'test 1 packageset', self.parent.owner, |
1315 | distroseries=self.parent) |
1316 | - test2 = getUtility(IPackagesetSet).new( |
1317 | + getUtility(IPackagesetSet).new( |
1318 | u'test2', u'test 2 packageset', self.parent.owner, |
1319 | distroseries=self.parent) |
1320 | packages = ('udev', 'chromium', 'libc6') |
1321 | @@ -321,7 +314,7 @@ |
1322 | test1 = getUtility(IPackagesetSet).new( |
1323 | u'test1', u'test 1 packageset', self.parent.owner, |
1324 | distroseries=self.parent) |
1325 | - test2 = getUtility(IPackagesetSet).new( |
1326 | + getUtility(IPackagesetSet).new( |
1327 | u'test2', u'test 2 packageset', self.parent.owner, |
1328 | distroseries=self.parent) |
1329 | packages = ('udev', 'chromium') |
1330 | @@ -356,21 +349,6 @@ |
1331 | self.assertEqual( |
1332 | das[0].architecturetag, self.parent_das.architecturetag) |
1333 | |
1334 | - def test_schedule_index_creation(self): |
1335 | - # One follow-up step is creation of the new series' archive |
1336 | - # indexes. The initialise() method schedules a job for this. |
1337 | - feature_flag = createdistroseriesindexesjob.FEATURE_FLAG_ENABLE_MODULE |
1338 | - self.useFixture(FeatureFixture({feature_flag: u'on'})) |
1339 | - child = self.factory.makeDistroSeries() |
1340 | - ids = InitialiseDistroSeries(self.parent, child) |
1341 | - ids.initialise() |
1342 | - job = Store.of(child).find( |
1343 | - DistributionJob, |
1344 | - DistributionJob.job_type == |
1345 | - DistributionJobType.CREATE_DISTROSERIES_INDEXES, |
1346 | - DistributionJob.distroseries == child).one() |
1347 | - self.assertNotEqual(None, job) |
1348 | - |
1349 | def test_script(self): |
1350 | # Do an end-to-end test using the command-line tool. |
1351 | uploader = self.factory.makePerson() |
Jeroen--
This largely looks good. I have a couple of suggestions.
markIndexCreati onComplete is a great candidate for the with statement to control use of the marker file: onComplete( self, distroseries): eated` that no, this series no longer needs locateIndexesMa rker(distroseri es), "w") as marker:
+ def markIndexCreati
+ """Note that archive indexes for `distroseries` have been created.
+
+ This tells `needsIndexesCr
+ archive indexes to be set up.
+ """
+ with file(self.
+ marker.write(
+ "Indexes for %s were created on %s.\n"
+ % (distroseries, datetime.now(utc)))
To me, it reads a little bit cleaner. And context management is always fun.
In test_publish_ ftpmaster, I might rename the method makeDistro to makeDistroWithP ublishDirectory ; it'll be clearer to someone scanning the tests to update to realize the difference between that and the factory.make* methods.
Both of those are suggestions--this is fine to land as is.