Merge ~cjwatson/launchpad:publish-single-archive-option into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: fe7e2924511b316b6a177feb664a4ebaa098da49
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:publish-single-archive-option
Merge into: launchpad:master
Diff against target: 129 lines (+84/-2)
2 files modified
lib/lp/archivepublisher/scripts/publishdistro.py (+18/-2)
lib/lp/archivepublisher/tests/test_publishdistro.py (+66/-0)
Reviewer Review Type Date Requested Status
Guruprasad Approve
Review via email: mp+435203@code.launchpad.net

Commit message

publish-distro: Add an --archive option

Description of the change

This fixes an annoyance when performing QA: `publish-distro` already had `--ppas` options and similar to tell it to run over all archives of a particular kind, but previously the only way to get it to run over a single archive was to hack the `is_ppa_public` etc. functions in place. This option should make publisher QA a little more convenient in future.

To post a comment you must log in.
Revision history for this message
Guruprasad (lgp171188) wrote :

LGTM 👍

P.S. I do not understand the usage of `defer.inlineCallbacks` and the `yield` statements in a test method even after reading https://docs.twisted.org/en/twisted-20.3.0/core/howto/defer-intro.html#inline-callbacks-using-yield. But as the assertions made at the end of that test look okay to me, I approved this MP.

review: Approve
Revision history for this message
Colin Watson (cjwatson) wrote :

The `inlineCallbacks`/`yield` stuff is because we need to run some Twisted code to set up an in-process key server and set up signing keys for test archives. That code is asynchronous - i.e. it returns a `Deferred`, which is a promise to eventually deliver either a result or an error, rather than directly returning a result, because it's doing network communication that might block.

However, in the case of this test we essentially just want to convert that asynchronous code into synchronous code by waiting for each deferred result to appear. `yield` within `inlineCallbacks` does that. Technically this is a generator, but it's best not to think of it as generating a sequence of values: just think of each `yield` as a point where we stop and wait for some asynchronous code to complete, and you'll be pretty close.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/lib/lp/archivepublisher/scripts/publishdistro.py b/lib/lp/archivepublisher/scripts/publishdistro.py
index 85511be..6c93a6e 100644
--- a/lib/lp/archivepublisher/scripts/publishdistro.py
+++ b/lib/lp/archivepublisher/scripts/publishdistro.py
@@ -202,6 +202,13 @@ class PublishDistro(PublisherScript):
202 help="Only run over the copy archives.",202 help="Only run over the copy archives.",
203 )203 )
204204
205 self.parser.add_option(
206 "--archive",
207 dest="archive",
208 metavar="REFERENCE",
209 help="Only run over the archive identified by this reference.",
210 )
211
205 def isCareful(self, option):212 def isCareful(self, option):
206 """Is the given "carefulness" option enabled?213 """Is the given "carefulness" option enabled?
207214
@@ -243,6 +250,7 @@ class PublishDistro(PublisherScript):
243 self.options.ppa,250 self.options.ppa,
244 self.options.private_ppa,251 self.options.private_ppa,
245 self.options.copy_archive,252 self.options.copy_archive,
253 self.options.archive,
246 ]254 ]
247 return len(list(filter(None, exclusive_options)))255 return len(list(filter(None, exclusive_options)))
248256
@@ -271,7 +279,7 @@ class PublishDistro(PublisherScript):
271 if self.countExclusiveOptions() > 1:279 if self.countExclusiveOptions() > 1:
272 raise OptionValueError(280 raise OptionValueError(
273 "Can only specify one of partner, ppa, private-ppa, "281 "Can only specify one of partner, ppa, private-ppa, "
274 "copy-archive."282 "copy-archive, archive."
275 )283 )
276284
277 if self.options.all_derived and self.options.distribution is not None:285 if self.options.all_derived and self.options.distribution is not None:
@@ -342,7 +350,15 @@ class PublishDistro(PublisherScript):
342350
343 def getTargetArchives(self, distribution):351 def getTargetArchives(self, distribution):
344 """Find the archive(s) selected by the script's options."""352 """Find the archive(s) selected by the script's options."""
345 if self.options.partner:353 if self.options.archive:
354 archive = getUtility(IArchiveSet).getByReference(
355 self.options.archive
356 )
357 if archive.distribution == distribution:
358 return [archive]
359 else:
360 return []
361 elif self.options.partner:
346 return [distribution.getArchiveByComponent("partner")]362 return [distribution.getArchiveByComponent("partner")]
347 elif self.options.ppa:363 elif self.options.ppa:
348 return filter(is_ppa_public, self.getPPAs(distribution))364 return filter(is_ppa_public, self.getPPAs(distribution))
diff --git a/lib/lp/archivepublisher/tests/test_publishdistro.py b/lib/lp/archivepublisher/tests/test_publishdistro.py
index 96ade8e..f99ee6e 100644
--- a/lib/lp/archivepublisher/tests/test_publishdistro.py
+++ b/lib/lp/archivepublisher/tests/test_publishdistro.py
@@ -445,6 +445,72 @@ class TestPublishDistro(TestNativePublishingBase):
445 pool_path = os.path.join(repo_path, "pool/main/b/baz/baz_666.dsc")445 pool_path = os.path.join(repo_path, "pool/main/b/baz/baz_666.dsc")
446 self.assertExists(pool_path)446 self.assertExists(pool_path)
447447
448 @defer.inlineCallbacks
449 def testForSingleArchive(self):
450 """Run publish-distro over a single archive specified by reference."""
451 ubuntutest = getUtility(IDistributionSet)["ubuntutest"]
452 name16 = getUtility(IPersonSet).getByName("name16")
453 archives = [
454 getUtility(IArchiveSet).new(
455 purpose=ArchivePurpose.PPA,
456 owner=name16,
457 name=name,
458 distribution=ubuntutest,
459 )
460 for name in (
461 self.factory.getUniqueUnicode(),
462 self.factory.getUniqueUnicode(),
463 )
464 ]
465 archive_references = [archive.reference for archive in archives]
466 pub_source_ids = [
467 self.getPubSource(archive=archive).id for archive in archives
468 ]
469
470 self.setUpRequireSigningKeys()
471 yield self.useFixture(InProcessKeyServerFixture()).start()
472 key_path = os.path.join(gpgkeysdir, "ppa-sample@canonical.com.sec")
473 for archive in archives:
474 yield IArchiveGPGSigningKey(archive).setSigningKey(
475 key_path, async_keyserver=True
476 )
477
478 self.layer.txn.commit()
479
480 self.assertEqual(
481 [PackagePublishingStatus.PENDING, PackagePublishingStatus.PENDING],
482 [
483 self.loadPubSource(pub_source_id).status
484 for pub_source_id in pub_source_ids
485 ],
486 )
487
488 self.runPublishDistro(["--archive", archive_references[0]])
489
490 self.assertEqual(
491 [
492 PackagePublishingStatus.PUBLISHED,
493 PackagePublishingStatus.PENDING,
494 ],
495 [
496 self.loadPubSource(pub_source_id).status
497 for pub_source_id in pub_source_ids
498 ],
499 )
500
501 self.runPublishDistro(["--archive", archive_references[1]])
502
503 self.assertEqual(
504 [
505 PackagePublishingStatus.PUBLISHED,
506 PackagePublishingStatus.PUBLISHED,
507 ],
508 [
509 self.loadPubSource(pub_source_id).status
510 for pub_source_id in pub_source_ids
511 ],
512 )
513
448 def testPublishToArtifactory(self):514 def testPublishToArtifactory(self):
449 """Publishing to Artifactory doesn't require generated signing keys."""515 """Publishing to Artifactory doesn't require generated signing keys."""
450 self.setUpRequireSigningKeys()516 self.setUpRequireSigningKeys()

Subscribers

People subscribed via source and target branches

to status/vote changes: