launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/recipe-vocab-many-ppas-timeout into lp:launchpad.

Commit message:
Optimise BuildableDistroSeries.findSeries.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1778732 in Launchpad itself: "Recipe creation times out"
  https://bugs.launchpad.net/launchpad/+bug/1778732

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/recipe-vocab-many-ppas-timeout/+merge/348569

I'm not sure whether this will be all of it or whether target_ppas_vocabulary will need some work too, but it's a start.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/recipe-vocab-many-ppas-timeout into lp:launchpad.

=== modified file 'lib/lp/code/vocabularies/sourcepackagerecipe.py'
--- lib/lp/code/vocabularies/sourcepackagerecipe.py	2015-07-08 16:05:11 +0000
+++ lib/lp/code/vocabularies/sourcepackagerecipe.py	2018-06-27 01:01:06 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Source Package Recipe vocabularies used in the lp/code modules."""
@@ -37,8 +37,8 @@
 
     @classmethod
     def findSeries(self, user):
-        ppas = getUtility(IArchiveSet).getPPAsForUser(user)
-        supported_distros = set([ppa.distribution for ppa in ppas])
+        supported_distros = set(
+            getUtility(IArchiveSet).getPPADistributionsForUser(user))
         # Now add in Ubuntu.
         supported_distros.add(getUtility(ILaunchpadCelebrities).ubuntu)
         all_series = getUtility(IDistroSeriesSet).search()

=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt	2018-05-27 18:32:33 +0000
+++ lib/lp/soyuz/doc/archive.txt	2018-06-27 01:01:06 +0000
@@ -1208,6 +1208,22 @@
     >>> jblack_ppas.count()
     0
 
+'getPPADistributionsForUser' returns the distinct distributions for all the
+PPAs that a given user participates in.
+
+    >>> for distribution in archive_set.getPPADistributionsForUser(cprov):
+    ...     print(distribution.display_name)
+    Ubuntu
+    >>> for distribution in archive_set.getPPADistributionsForUser(no_priv):
+    ...     print(distribution.display_name)
+    Ubuntu
+    >>> for distribution in archive_set.getPPADistributionsForUser(
+    ...         indirect_uploader):
+    ...     print(distribution.display_name)
+    Ubuntu
+    >>> for distribution in archive_set.getPPADistributionsForUser(jblack):
+    ...     print(distribution.display_name)
+
 The method getPrivatePPAs() will return a result set of all PPAs that are
 private.
 

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2018-05-04 21:59:32 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2018-06-27 01:01:06 +0000
@@ -2383,6 +2383,9 @@
         The result is ordered by PPA displayname.
         """
 
+    def getPPADistributionsForUser(user):
+        """Return the `Distribution`s of all PPAs for the given user."""
+
     def getPPAsPendingSigningKey():
         """Return all PPAs pending signing key generation.
 

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2018-05-04 21:59:32 +0000
+++ lib/lp/soyuz/model/archive.py	2018-06-27 01:01:06 +0000
@@ -33,6 +33,7 @@
     Or,
     Select,
     Sum,
+    Union,
     )
 from storm.properties import (
     Int,
@@ -40,7 +41,10 @@
     Unicode,
     )
 from storm.references import Reference
-from storm.store import Store
+from storm.store import (
+    EmptyResultSet,
+    Store,
+    )
 from zope.component import (
     getAdapter,
     getUtility,
@@ -2747,29 +2751,35 @@
                     SourcePackagePublishingHistory.archive == Archive.id)
         return store.find(Archive, *clause).order_by(Archive.id).first()
 
+    def _getPPAsForUserClause(self, user):
+        """Base clause for getPPAsForUser and getPPADistributionsForUser."""
+        direct_membership = Select(
+            Archive.id,
+            where=And(
+                Archive._enabled == True,
+                Archive.purpose == ArchivePurpose.PPA,
+                TeamParticipation.team == Archive.ownerID,
+                TeamParticipation.person == user,
+                ))
+        third_party_upload_acl = Select(
+            Archive.id,
+            where=And(
+                Archive.purpose == ArchivePurpose.PPA,
+                ArchivePermission.archiveID == Archive.id,
+                TeamParticipation.person == user,
+                TeamParticipation.team == ArchivePermission.personID,
+                ))
+        return Archive.id.is_in(
+            Union(direct_membership, third_party_upload_acl))
+
     def getPPAsForUser(self, user):
         """See `IArchiveSet`."""
         from lp.registry.model.person import Person
-        # If there's no user logged in, then there's no archives.
+        # If there's no user logged in, then there are no archives.
         if user is None:
-            return []
+            return EmptyResultSet()
         store = Store.of(user)
-        direct_membership = store.find(
-            Archive,
-            Archive._enabled == True,
-            Archive.purpose == ArchivePurpose.PPA,
-            TeamParticipation.team == Archive.ownerID,
-            TeamParticipation.person == user,
-            )
-        third_party_upload_acl = store.find(
-            Archive,
-            Archive.purpose == ArchivePurpose.PPA,
-            ArchivePermission.archiveID == Archive.id,
-            TeamParticipation.person == user,
-            TeamParticipation.team == ArchivePermission.personID,
-            )
-
-        result = direct_membership.union(third_party_upload_acl)
+        result = store.find(Archive, self._getPPAsForUserClause(user))
         result.order_by(Archive.displayname)
 
         def preload_owners(rows):
@@ -2777,6 +2787,19 @@
 
         return DecoratedResultSet(result, pre_iter_hook=preload_owners)
 
+    def getPPADistributionsForUser(self, user):
+        """See `IArchiveSet`."""
+        from lp.registry.model.distribution import Distribution
+        # If there's no user logged in, then there are no archives.
+        if user is None:
+            return EmptyResultSet()
+        store = Store.of(user)
+        result = store.find(
+            Distribution,
+            Distribution.id == Archive.distributionID,
+            self._getPPAsForUserClause(user))
+        return result.config(distinct=True)
+
     def getPPAsPendingSigningKey(self):
         """See `IArchiveSet`."""
         origin = (