launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/faster-htpasswd-again into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/faster-htpasswd-again/+merge/108087

The third generate-ppa-htaccess optimisation for the day, this branch makes the core generation function about 30x faster. Loading many thousands of persons and archiveauthtokens is very slow, so I changed it to just pull out the relevant small fields. A test needed changing slightly to pass them in in the new format, but all other callsites use the internal token retrieval logic.

I also dropped the ValidPersonCache check that was added in the previous revision, as it can only make it slower.
-- 
https://code.launchpad.net/~wgrant/launchpad/faster-htpasswd-again/+merge/108087
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/faster-htpasswd-again into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2012-05-30 21:41:06 +0000
+++ database/schema/security.cfg	2012-05-31 02:13:21 +0000
@@ -2219,7 +2219,6 @@
 
 [generateppahtaccess]
 groups=script
-public.account                          = SELECT
 public.archive                          = SELECT
 public.archiveauthtoken                 = SELECT, UPDATE
 public.archivesubscriber                = SELECT, UPDATE

=== modified file 'lib/lp/archivepublisher/htaccess.py'
--- lib/lp/archivepublisher/htaccess.py	2012-05-30 22:53:50 +0000
+++ lib/lp/archivepublisher/htaccess.py	2012-05-31 02:13:21 +0000
@@ -17,13 +17,11 @@
 
 
 import crypt
-from operator import attrgetter
+from operator import itemgetter
 import os
 
-from zope.component import getUtility
-
-from lp.registry.interfaces.person import IPersonSet
-from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
+from lp.registry.model.person import Person
+from lp.services.database.lpstorm import IStore
 from lp.soyuz.model.archiveauthtoken import ArchiveAuthToken
 
 
@@ -81,21 +79,24 @@
     assert archive.private, "Archive %r must be private" % archive
 
     if tokens is None:
-        tokens = getUtility(IArchiveAuthTokenSet).getByArchive(archive)
+        tokens = IStore(ArchiveAuthToken).find(
+            (ArchiveAuthToken.person_id, ArchiveAuthToken.token),
+            ArchiveAuthToken.archive == archive,
+            ArchiveAuthToken.date_deactivated == None)
     # We iterate tokens more than once - materialise it.
     tokens = list(tokens)
 
     # The first .htpasswd entry is the buildd_secret.
     yield (BUILDD_USER_NAME, archive.buildd_secret, BUILDD_USER_NAME[:2])
 
-    person_ids = map(attrgetter('person_id'), tokens)
-    valid_persons = dict((person.id, person) for person in
-        getUtility(IPersonSet).getPrecachedPersonsFromIDs(
-            person_ids, need_validity=True) if person.is_valid_person)
-    usable_tokens = [(token, valid_persons[token.person_id])
-        for token in tokens if token.person_id in valid_persons]
+    person_ids = map(itemgetter(0), tokens)
+    names = dict(
+        IStore(Person).find(
+            (Person.id, Person.name), Person.id.is_in(set(person_ids))))
+    # Combine the token list with the person list, sorting by person ID
+    # so the file can be compared later.
+    sorted_tokens = [(token[1], names[token[0]]) for token in sorted(tokens)]
     # Iterate over tokens and write the appropriate htpasswd
-    # entries for them.  Use a consistent sort order so that the
-    # generated file can be compared to an existing one later.
-    for token, person in sorted(usable_tokens, key=lambda item:item[0].id):
-        yield (person.name, token.token, person.name[:2])
+    # entries for them.
+    for token, person_name in sorted_tokens:
+        yield (person_name, token, person_name[:2])

=== modified file 'lib/lp/archivepublisher/tests/test_htaccess.py'
--- lib/lp/archivepublisher/tests/test_htaccess.py	2012-05-30 16:23:55 +0000
+++ lib/lp/archivepublisher/tests/test_htaccess.py	2012-05-31 02:13:21 +0000
@@ -109,13 +109,15 @@
         self.ppa.newSubscription(name12, self.ppa.owner)
         self.ppa.newSubscription(name16, self.ppa.owner)
         first_created_token = self.ppa.newAuthToken(name16)
-        tokens = [self.ppa.newAuthToken(name12), first_created_token]
+        tokens = [
+            (token.person_id, token.token)
+            for token in [self.ppa.newAuthToken(name12), first_created_token]]
 
         credentials = list(htpasswd_credentials_for_archive(self.ppa, tokens))
 
         self.assertContentEqual(
             credentials, [
                 ("buildd", "geheim", "bu"),
-                ("name12", tokens[0].token, "na"),
-                ("name16", tokens[1].token, "na")
+                ("name12", tokens[0][1], "na"),
+                ("name16", tokens[1][1], "na")
                 ])