launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/ppa-genkeys-specific-archive into lp:launchpad.

Commit message:
Make ppa-generate-keys take an archive reference rather than an owner name.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/ppa-genkeys-specific-archive/+merge/272510

ppa-generate-keys predates the ability to have multiple PPAs per person, and only offers the ability to generate keys for a person's default archive.  This is rather inconvenient if you wanted to discard and regenerate the signing key for a non-default archive (useful on dogfood when the signing key came from the production database).

I didn't see a particular need to preserve the (nowadays) weird default-archive-for-person option.  ArchiveSigningKey.generateSigningKey takes care of making sure that the default PPA for a person gets a signing key even if you ask it to generate a key for a different one.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/ppa-genkeys-specific-archive into lp:launchpad.

=== modified file 'lib/lp/soyuz/scripts/ppakeygenerator.py'
--- lib/lp/soyuz/scripts/ppakeygenerator.py	2012-07-05 09:43:58 +0000
+++ lib/lp/soyuz/scripts/ppakeygenerator.py	2015-09-26 02:09:41 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
@@ -10,7 +10,6 @@
 from lp.archivepublisher.interfaces.archivesigningkey import (
     IArchiveSigningKey,
     )
-from lp.registry.interfaces.person import IPersonSet
 from lp.services.scripts.base import (
     LaunchpadCronScript,
     LaunchpadScriptFailure,
@@ -25,8 +24,8 @@
 
     def add_my_options(self):
         self.parser.add_option(
-            "-p", "--ppa", dest="archive_owner_name",
-            help="Name of the PPA owner to create the key.")
+            "-A", "--archive",
+            help="The reference of the archive whose key should be generated.")
 
     def generateKey(self, archive):
         """Generate a signing key for the given archive."""
@@ -38,22 +37,18 @@
 
     def main(self):
         """Generate signing keys for the selected PPAs."""
-        owner_name = self.options.archive_owner_name
-
-        if owner_name is not None:
-            owner = getUtility(IPersonSet).getByName(owner_name)
-            if owner is None:
-                raise LaunchpadScriptFailure(
-                    "No person named '%s' could be found." % owner_name)
-            if owner.archive is None:
-                raise LaunchpadScriptFailure(
-                    "Person named '%s' has no PPA." % owner_name)
-            if owner.archive.signing_key is not None:
+        if self.options.archive is not None:
+            archive = getUtility(IArchiveSet).getByReference(
+                self.options.archive)
+            if archive is None:
+                raise LaunchpadScriptFailure(
+                    "No archive named '%s' could be found."
+                    % self.options.archive)
+            if archive.signing_key is not None:
                 raise LaunchpadScriptFailure(
                     "%s already has a signing_key (%s)"
-                    % (owner.archive.displayname,
-                       owner.archive.signing_key.fingerprint))
-            archives = [owner.archive]
+                    % (archive.displayname, archive.signing_key.fingerprint))
+            archives = [archive]
         else:
             archive_set = getUtility(IArchiveSet)
             archives = list(archive_set.getPPAsPendingSigningKey())

=== modified file 'lib/lp/soyuz/scripts/tests/test_ppakeygenerator.py'
--- lib/lp/soyuz/scripts/tests/test_ppakeygenerator.py	2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/scripts/tests/test_ppakeygenerator.py	2015-09-26 02:09:41 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """`PPAKeyGenerator` script class tests."""
@@ -30,7 +30,7 @@
         ubuntutest = getUtility(IDistributionSet).getByName('ubuntutest')
         archive.distribution = ubuntutest
 
-    def _getKeyGenerator(self, ppa_owner_name=None, txn=None):
+    def _getKeyGenerator(self, archive_reference=None, txn=None):
         """Return a `PPAKeyGenerator` instance.
 
         Monkey-patch the script object with a fake transaction manager
@@ -39,8 +39,8 @@
         """
         test_args = []
 
-        if ppa_owner_name is not None:
-            test_args.extend(['-p', ppa_owner_name])
+        if archive_reference is not None:
+            test_args.extend(['-A', archive_reference])
 
         key_generator = PPAKeyGenerator(
             name='ppa-generate-keys', test_args=test_args)
@@ -57,20 +57,12 @@
 
         return key_generator
 
-    def testPersonNotFound(self):
-        """Raises an error if the specified person does not exist."""
-        key_generator = self._getKeyGenerator(ppa_owner_name='biscuit')
-        self.assertRaisesWithContent(
-            LaunchpadScriptFailure,
-            "No person named 'biscuit' could be found.",
-            key_generator.main)
-
-    def testPersonHasNoPPA(self):
-        """Raises an error if the specified person does not have a PPA. """
-        key_generator = self._getKeyGenerator(ppa_owner_name='name16')
-        self.assertRaisesWithContent(
-            LaunchpadScriptFailure,
-            "Person named 'name16' has no PPA.",
+    def testArchiveNotFound(self):
+        """Raises an error if the specified archive does not exist."""
+        key_generator = self._getKeyGenerator(archive_reference='~biscuit')
+        self.assertRaisesWithContent(
+            LaunchpadScriptFailure,
+            "No archive named '~biscuit' could be found.",
             key_generator.main)
 
     def testPPAAlreadyHasSigningKey(self):
@@ -79,7 +71,8 @@
         a_key = getUtility(IGPGKeySet).get(1)
         cprov.archive.signing_key = a_key
 
-        key_generator = self._getKeyGenerator(ppa_owner_name='cprov')
+        key_generator = self._getKeyGenerator(
+            archive_reference='~cprov/ubuntu/ppa')
         self.assertRaisesWithContent(
             LaunchpadScriptFailure,
             ("PPA for Celso Providelo already has a signing_key (%s)" %
@@ -98,7 +91,8 @@
         self.assertTrue(cprov.archive.signing_key is None)
 
         txn = FakeTransaction()
-        key_generator = self._getKeyGenerator(ppa_owner_name='cprov', txn=txn)
+        key_generator = self._getKeyGenerator(
+            archive_reference='~cprov/ubuntutest/ppa', txn=txn)
         key_generator.main()
 
         self.assertTrue(cprov.archive.signing_key is not None)