← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~abentley/launchpad/bulk-insert into lp:launchpad

 

Aaron Bentley has proposed merging lp:~abentley/launchpad/bulk-insert into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~abentley/launchpad/bulk-insert/+merge/94271

= Summary =
Partially address bug #808930: Timeout running branch scanner job

== Proposed fix ==
Optimize DB access in the branch scanner by doing bulk row insertions.

This takes a full scan of lp:~irar/gcc-linaro/slp-for-any-nops-4.6/ down to ~2 minutes locally.

== Pre-implementation notes ==
Discussed with gmb

== Implementation details ==
Change INewRevisionEvent to INewMainlineRevisions event, so that multiple revisions can be handled at once.  The INewRevisionEvent subscribers only cared about mainline revisions, so providing only mainline revisions simplifies the code path.

Similarly, change got_new_revision to got_new_mainline_revisions.

Change createBranchRevisionFromIDs to perform bulk insertions using new Storm syntax.

Change RevisionSet.acquireRevisionAuthor to acquireRevisionAuthors, so that it does efficient queries for existing RevisionAuthors.

Change RevisionSet.newFromBazaarRevision to newFromBazaarRevisions.  It now inserts multiple rows at once, using bulk.create.

Simplify RevisionSet.isPresent (The original did not give a performance advantage.)

syncOneRevision becomes syncRevisions.

Scanner loops use chunks of 10k, as this shows some performance advantage over 1k loops.  (100k does not.)


== Tests ==
bin/test -vt test_branchjob -t test_revision -t test_buglinks -t test_bzrsync

== Demo and Q/A ==
None


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/codehosting/scanner/tests/test_bzrsync.py
  lib/lp/code/configure.zcml
  lib/lp/codehosting/scanner/events.py
  lib/lp/code/model/tests/test_branchjob.py
  lib/lp/codehosting/scanner/buglinks.py
  lib/lp/testing/factory.py
  lib/lp/code/interfaces/revision.py
  lib/lp/code/model/branch.py
  lib/lp/codehosting/scanner/tests/test_buglinks.py
  lib/lp/codehosting/scanner/bzrsync.py
  lib/lp/code/model/tests/test_revision.py
  lib/lp/code/model/revision.py
  lib/lp/code/model/branchjob.py
-- 
https://code.launchpad.net/~abentley/launchpad/bulk-insert/+merge/94271
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/bulk-insert into lp:launchpad.
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2011-12-24 17:49:30 +0000
+++ lib/lp/code/configure.zcml	2012-02-22 20:37:22 +0000
@@ -514,8 +514,8 @@
       for="lp.codehosting.scanner.events.IRevisionsRemoved"
       handler="lp.codehosting.scanner.email.send_removed_revision_emails"/>
   <subscriber
-      for="lp.codehosting.scanner.events.INewRevision"
-      handler="lp.codehosting.scanner.buglinks.got_new_revision"/>
+      for="lp.codehosting.scanner.events.INewMainlineRevisions"
+      handler="lp.codehosting.scanner.buglinks.got_new_mainline_revisions"/>
   <subscriber
       for="lp.codehosting.scanner.events.IScanCompleted"
       handler="lp.codehosting.scanner.mergedetection.auto_merge_proposals"/>

=== modified file 'lib/lp/code/interfaces/revision.py'
--- lib/lp/code/interfaces/revision.py	2011-12-24 16:54:44 +0000
+++ lib/lp/code/interfaces/revision.py	2012-02-22 20:37:22 +0000
@@ -134,8 +134,19 @@
             parent_ids, properties):
         """Create a new Revision with the given revision ID."""
 
-    def newFromBazaarRevision(bzr_revision):
-        """Create a new Revision from the given Bazaar Revision object."""
+    def newFromBazaarRevisions(revision_batch):
+        """Create new Revisions from the given Bazaar Revisions.
+
+        This method allows us to insert Revisions in bulk, which is
+        important for larger branches.
+        """
+
+    def acquireRevisionAuthors(names):
+        """Return a dict of RevisionAuthors with the specified names.
+
+        If a RevisionAuthor is not present in the database, it is created
+        first.
+        """
 
     def getTipRevisionsForBranches(branches):
         """Get the tip branch revisions for the specified branches.

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2012-02-20 02:07:55 +0000
+++ lib/lp/code/model/branch.py	2012-02-22 20:37:22 +0000
@@ -28,6 +28,7 @@
     And,
     Count,
     Desc,
+    Insert,
     NamedFunc,
     Not,
     Or,
@@ -132,7 +133,9 @@
     validate_public_person,
     )
 from lp.services.config import config
-from lp.services.database.bulk import load_related
+from lp.services.database.bulk import (
+    load_related,
+    )
 from lp.services.database.constants import (
     DEFAULT,
     UTC_NOW,
@@ -883,13 +886,8 @@
             CREATE TEMPORARY TABLE RevidSequence
             (revision_id text, sequence integer)
             """)
-        data = []
-        for revid, sequence in revision_id_sequence_pairs:
-            data.append('(%s, %s)' % sqlvalues(revid, sequence))
-        data = ', '.join(data)
-        store.execute(
-            "INSERT INTO RevidSequence (revision_id, sequence) VALUES %s"
-            % data)
+        store.execute(Insert(('revision_id', 'sequence'),
+            table=['RevidSequence'], expr=revision_id_sequence_pairs))
         store.execute(
             """
             INSERT INTO BranchRevision (branch, revision, sequence)

=== modified file 'lib/lp/code/model/branchjob.py'
--- lib/lp/code/model/branchjob.py	2012-02-15 22:01:27 +0000
+++ lib/lp/code/model/branchjob.py	2012-02-22 20:37:22 +0000
@@ -657,11 +657,10 @@
             merged_revisions = self.getMergedRevisionIDs(revision_id, graph)
             authors = self.getAuthors(merged_revisions, graph)
             revision_set = RevisionSet()
-            rev_authors = set(revision_set.acquireRevisionAuthor(author) for
-                              author in authors)
+            rev_authors = revision_set.acquireRevisionAuthors(authors)
             outf = StringIO()
             pretty_authors = []
-            for rev_author in rev_authors:
+            for rev_author in rev_authors.values():
                 if rev_author.person is None:
                     displayname = rev_author.name
                 else:

=== modified file 'lib/lp/code/model/revision.py'
--- lib/lp/code/model/revision.py	2011-12-30 06:14:56 +0000
+++ lib/lp/code/model/revision.py	2012-02-22 20:37:22 +0000
@@ -25,7 +25,6 @@
     ForeignKey,
     IntCol,
     SQLMultipleJoin,
-    SQLObjectNotFound,
     StringCol,
     )
 from storm.expr import (
@@ -61,6 +60,7 @@
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.projectgroup import IProjectGroup
 from lp.registry.model.person import ValidPersonCache
+from lp.services.database.bulk import create
 from lp.services.database.constants import (
     DEFAULT,
     UTC_NOW,
@@ -73,18 +73,12 @@
 from lp.services.database.sqlbase import (
     quote,
     SQLBase,
-    sqlvalues,
     )
 from lp.services.helpers import shortlist
 from lp.services.identity.interfaces.emailaddress import (
     EmailAddressStatus,
     IEmailAddressSet,
     )
-from lp.services.webapp.interfaces import (
-    DEFAULT_FLAVOR,
-    IStoreSelector,
-    MAIN_STORE,
-    )
 
 
 class Revision(SQLBase):
@@ -270,6 +264,9 @@
         author.linkToLaunchpadPerson()
         return author
 
+    def _extractRevisionData(self, revision):
+        """Extract and return correctly-formatted revision data."""
+
     def new(self, revision_id, log_body, revision_date, revision_author,
             parent_ids, properties, _date_created=None):
         """See IRevisionSet.new()"""
@@ -277,13 +274,13 @@
             properties = {}
         if _date_created is None:
             _date_created = UTC_NOW
-        author = self.acquireRevisionAuthor(revision_author)
+        authors = self.acquireRevisionAuthors([revision_author])
 
         revision = Revision(
             revision_id=revision_id,
             log_body=log_body,
             revision_date=revision_date,
-            revision_author=author,
+            revision_author=authors[revision_author],
             date_created=_date_created)
         # Don't create future revisions.
         if revision.revision_date > revision.date_created:
@@ -303,22 +300,29 @@
 
         return revision
 
-    def acquireRevisionAuthor(self, name):
-        """Find or create the RevisionAuthor with the specified name.
+    def acquireRevisionAuthors(self, author_names):
+        """Find or create the RevisionAuthors with the specified names.
 
-        Name may be any arbitrary string, but if it is an email-id, and
+        A name may be any arbitrary string, but if it is an email-id, and
         its email address is a verified email address, it will be
         automatically linked to the corresponding Person.
 
         Email-ids come in two major forms:
             "Foo Bar" <foo@xxxxxxx>
             foo@xxxxxxx (Foo Bar)
+        :return: a dict of name -> RevisionAuthor
         """
-        # create a RevisionAuthor if necessary:
-        try:
-            return RevisionAuthor.byName(name)
-        except SQLObjectNotFound:
-            return self._createRevisionAuthor(name)
+        store = IMasterStore(Revision)
+        author_names = set(author_names)
+        authors = {}
+        for author in store.find(RevisionAuthor,
+                RevisionAuthor.name.is_in(author_names)):
+            authors[author.name] = author
+        missing = author_names - set(authors.keys())
+        # create missing RevisionAuthors
+        for name in missing:
+            authors[name] = self._createRevisionAuthor(name)
+        return authors
 
     def _timestampToDatetime(self, timestamp):
         """Convert the given timestamp to a datetime object.
@@ -339,54 +343,64 @@
         revision_date += timedelta(seconds=timestamp - int_timestamp)
         return revision_date
 
-    def newFromBazaarRevision(self, bzr_revision):
+    def newFromBazaarRevisions(self, revisions):
         """See `IRevisionSet`."""
-        revision_id = bzr_revision.revision_id
-        revision_date = self._timestampToDatetime(bzr_revision.timestamp)
-        authors = bzr_revision.get_apparent_authors()
-        # XXX: JonathanLange 2009-05-01 bug=362686: We can only have one
-        # author per revision, so we use the first on the assumption that
-        # this is the primary author.
-        try:
-            author = authors[0]
-        except IndexError:
-            author = None
-        return self.new(
-            revision_id=revision_id,
-            log_body=bzr_revision.message,
-            revision_date=revision_date,
-            revision_author=author,
-            parent_ids=bzr_revision.parent_ids,
-            properties=bzr_revision.properties)
+        data = []
+
+        author_names = []
+        for bzr_revision in revisions:
+            authors = bzr_revision.get_apparent_authors()
+            try:
+                author = authors[0]
+            except IndexError:
+                author = None
+            author_names.append(author)
+        revision_authors = dict(
+            (name, author.id) for name, author in
+            self.acquireRevisionAuthors(author_names).items())
+
+        property_data = []
+        parent_data = []
+        for bzr_revision, author_name in zip(revisions, author_names):
+            revision_id = bzr_revision.revision_id
+            revision_date = self._timestampToDatetime(bzr_revision.timestamp)
+            revision_author = revision_authors[author_name]
+
+            data.append(
+                (revision_id, bzr_revision.message, revision_date,
+                revision_author))
+        db_revisions = create((
+            Revision.revision_id, Revision.log_body, Revision.revision_date,
+            Revision.revision_author_id), data)
+
+        revision_db_id = dict(
+            (rev.revision_id, rev.id) for rev in db_revisions)
+
+        for bzr_revision in revisions:
+            db_id = revision_db_id[bzr_revision.revision_id]
+            for name, value in bzr_revision.properties.iteritems():
+                property_data.append((db_id, name, value))
+            parent_ids = bzr_revision.parent_ids
+            seen_parents = set()
+            for sequence, parent_id in enumerate(parent_ids):
+                if parent_id in seen_parents:
+                    continue
+                seen_parents.add(parent_id)
+                parent_data.append((db_id, sequence, parent_id))
+        create((
+            RevisionParent.revisionID, RevisionParent.sequence,
+            RevisionParent.parent_id), parent_data, return_created=False)
+        create((
+            RevisionProperty.revisionID, RevisionProperty.name,
+            RevisionProperty.value), property_data, return_created=False)
 
     @staticmethod
     def onlyPresent(revids):
         """See `IRevisionSet`."""
-        if not revids:
-            return set()
-        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-        store.execute(
-            """
-            CREATE TEMPORARY TABLE Revids
-            (revision_id text)
-            """)
-        data = []
-        for revid in revids:
-            data.append('(%s)' % sqlvalues(revid))
-        data = ', '.join(data)
-        store.execute(
-            "INSERT INTO Revids (revision_id) VALUES %s" % data)
-        result = store.execute(
-            """
-            SELECT Revids.revision_id
-            FROM Revids, Revision
-            WHERE Revids.revision_id = Revision.revision_id
-            """)
-        present = set()
-        for row in result.get_all():
-            present.add(row[0])
-        store.execute("DROP TABLE Revids")
-        return present
+        store = IStore(Revision)
+        clause = Revision.revision_id.is_in(revids)
+        present = store.find(Revision.revision_id, clause)
+        return set(present)
 
     def checkNewVerifiedEmail(self, email):
         """See `IRevisionSet`."""

=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
--- lib/lp/code/model/tests/test_branchjob.py	2012-02-15 22:01:27 +0000
+++ lib/lp/code/model/tests/test_branchjob.py	2012-02-22 20:37:22 +0000
@@ -392,10 +392,9 @@
             existing = branch.getBranchRevision(
                 revision_id=bzr_revision.revision_id)
             if existing is None:
-                revision = RevisionSet().newFromBazaarRevision(bzr_revision)
-            else:
-                revision = RevisionSet().getByRevisionId(
-                    bzr_revision.revision_id)
+                RevisionSet().newFromBazaarRevisions([bzr_revision])
+            revision = RevisionSet().getByRevisionId(
+                bzr_revision.revision_id)
             try:
                 revno = bzr_branch.revision_id_to_revno(revision.revision_id)
             except bzr_errors.NoSuchRevision:

=== modified file 'lib/lp/code/model/tests/test_revision.py'
--- lib/lp/code/model/tests/test_revision.py	2011-12-30 06:14:56 +0000
+++ lib/lp/code/model/tests/test_revision.py	2012-02-22 20:37:22 +0000
@@ -9,6 +9,7 @@
     datetime,
     timedelta,
     )
+from testtools.matchers import Equals
 import time
 from unittest import TestCase
 
@@ -27,7 +28,9 @@
     )
 from lp.registry.model.karma import Karma
 from lp.scripts.garbo import RevisionAuthorEmailLinker
-from lp.services.database.lpstorm import IMasterObject
+from lp.services.database.lpstorm import (
+    IMasterObject,
+    )
 from lp.services.database.sqlbase import cursor
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.log.logger import DevNullLogger
@@ -39,11 +42,13 @@
 from lp.testing import (
     login,
     logout,
+    StormStatementRecorder,
     TestCaseWithFactory,
     time_counter,
     )
 from lp.testing.factory import LaunchpadObjectFactory
 from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.matchers import HasQueryCount
 
 
 class TestRevisionCreationDate(TestCaseWithFactory):
@@ -257,6 +262,41 @@
         found = self.revision_set.getByRevisionId('nonexistent')
         self.assertIs(None, found)
 
+    def test_newFromBazaarRevisions(self):
+        # newFromBazaarRevisions behaves as expected.
+        # only branchscanner can SELECT revisionproperties.
+        self.becomeDbUser('branchscanner')
+        bzr_revisions = [
+            self.factory.makeBzrRevision('rev-1', prop1="foo"),
+            self.factory.makeBzrRevision('rev-2', parent_ids=['rev-1'])
+        ]
+        with StormStatementRecorder() as recorder:
+            self.revision_set.newFromBazaarRevisions(bzr_revisions)
+        rev_1 = self.revision_set.getByRevisionId('rev-1')
+        self.assertEqual(
+            bzr_revisions[0].committer, rev_1.revision_author.name)
+        self.assertEqual(
+            bzr_revisions[0].message, rev_1.log_body)
+        self.assertEqual(
+            datetime(1970, 1, 1, 0, 0, tzinfo=pytz.UTC), rev_1.revision_date)
+        self.assertEqual([], rev_1.parents)
+        self.assertEqual({'prop1': 'foo'}, rev_1.getProperties())
+        rev_2 = self.revision_set.getByRevisionId('rev-2')
+        self.assertEqual(['rev-1'], rev_2.parent_ids)
+        # Really, less than 9 is great, but if the count improves, we should
+        # tighten this restriction.
+        self.assertThat(recorder, HasQueryCount(Equals(8)))
+
+    def test_acquireRevisionAuthors(self):
+        # AcquireRevisionAuthors creates new authors only if none exists with
+        # that name.
+        author1 = self.revision_set.acquireRevisionAuthors(['name1'])['name1']
+        self.assertEqual(author1.name, 'name1')
+        Store.of(author1).flush()
+        author2 = self.revision_set.acquireRevisionAuthors(['name1'])['name1']
+        self.assertEqual(
+            removeSecurityProxy(author1).id, removeSecurityProxy(author2).id)
+
 
 class TestRevisionGetBranch(TestCaseWithFactory):
     """Test the `getBranch` method of the revision."""

=== modified file 'lib/lp/codehosting/scanner/buglinks.py'
--- lib/lp/codehosting/scanner/buglinks.py	2011-05-27 21:12:25 +0000
+++ lib/lp/codehosting/scanner/buglinks.py	2012-02-22 20:37:22 +0000
@@ -89,8 +89,7 @@
                     registrant=getUtility(ILaunchpadCelebrities).janitor)
 
 
-def got_new_revision(new_revision):
-    if new_revision.isMainline():
-        linker = BugBranchLinker(new_revision.db_branch)
-        linker.createBugBranchLinksForRevision(new_revision.bzr_revision)
-
+def got_new_mainline_revisions(new_mainline_revisions):
+    linker = BugBranchLinker(new_mainline_revisions.db_branch)
+    for bzr_revision in new_mainline_revisions.bzr_revisions:
+        linker.createBugBranchLinksForRevision(bzr_revision)

=== modified file 'lib/lp/codehosting/scanner/bzrsync.py'
--- lib/lp/codehosting/scanner/bzrsync.py	2012-02-15 17:29:54 +0000
+++ lib/lp/codehosting/scanner/bzrsync.py	2012-02-22 20:37:22 +0000
@@ -48,6 +48,7 @@
         if logger is None:
             logger = logging.getLogger(self.__class__.__name__)
         self.logger = logger
+        self.revision_set = getUtility(IRevisionSet)
 
     def syncBranchAndClose(self, bzr_branch=None):
         """Synchronize the database with a Bazaar branch, handling locking.
@@ -97,15 +98,9 @@
         new_db_revs = (
             new_ancestry - getUtility(IRevisionSet).onlyPresent(new_ancestry))
         self.logger.info("Adding %s new revisions.", len(new_db_revs))
-        for revids in iter_list_chunks(list(new_db_revs), 1000):
+        for revids in iter_list_chunks(list(new_db_revs), 10000):
             revisions = self.getBazaarRevisions(bzr_branch, revids)
-            for revision in revisions:
-                # This would probably go much faster if we found some way to
-                # bulk-load multiple revisions at once, but as this is only
-                # executed for revisions new to Launchpad, it doesn't seem
-                # worth it at this stage.
-                self.syncOneRevision(
-                    bzr_branch, revision, revids_to_insert)
+            self.syncRevisions(bzr_branch, revisions, revids_to_insert)
         self.deleteBranchRevisions(branchrevisions_to_delete)
         self.insertBranchRevisions(bzr_branch, revids_to_insert)
         transaction.commit()
@@ -239,24 +234,23 @@
         revisions = bzr_branch.repository.get_parent_map(revisions)
         return bzr_branch.repository.get_revisions(revisions.keys())
 
-    def syncOneRevision(self, bzr_branch, bzr_revision, revids_to_insert):
-        """Import the revision with the given revision_id.
+    def syncRevisions(self, bzr_branch, bzr_revisions, revids_to_insert):
+        """Import the supplied revisions.
 
         :param bzr_branch: The Bazaar branch that's being scanned.
-        :param bzr_revision: the revision to import
+        :param bzr_revisions: the revisions to import
         :type bzr_revision: bzrlib.revision.Revision
         :param revids_to_insert: a dict of revision ids to integer
             revno. Non-mainline revisions will be mapped to None.
         """
-        revision_id = bzr_revision.revision_id
-        revision_set = getUtility(IRevisionSet)
-        # Revision not yet in the database. Load it.
-        self.logger.debug("Inserting revision: %s", revision_id)
-        db_revision = revision_set.newFromBazaarRevision(bzr_revision)
-        notify(
-            events.NewRevision(
-                self.db_branch, bzr_branch, db_revision, bzr_revision,
-                revids_to_insert[revision_id]))
+        self.revision_set.newFromBazaarRevisions(bzr_revisions)
+        mainline_revisions = []
+        for bzr_revision in bzr_revisions:
+            if revids_to_insert[bzr_revision.revision_id] is None:
+                continue
+            mainline_revisions.append(bzr_revision)
+        notify(events.NewMainlineRevisions(
+            self.db_branch, bzr_branch, mainline_revisions))
 
     @staticmethod
     def revisionsToInsert(added_history, last_revno, added_ancestry):
@@ -293,7 +287,7 @@
         self.logger.info("Inserting %d branchrevision records.",
             len(revids_to_insert))
         revid_seq_pairs = revids_to_insert.items()
-        for revid_seq_pair_chunk in iter_list_chunks(revid_seq_pairs, 1000):
+        for revid_seq_pair_chunk in iter_list_chunks(revid_seq_pairs, 10000):
             self.db_branch.createBranchRevisionFromIDs(revid_seq_pair_chunk)
 
     def updateBranchStatus(self, bzr_history):

=== modified file 'lib/lp/codehosting/scanner/events.py'
--- lib/lp/codehosting/scanner/events.py	2010-10-15 18:33:07 +0000
+++ lib/lp/codehosting/scanner/events.py	2012-02-22 20:37:22 +0000
@@ -5,7 +5,7 @@
 
 __metaclass__ = type
 __all__ = [
-    'NewRevision',
+    'NewMainlineRevisions',
     'RevisionsRemoved',
     'TipChanged',
     ]
@@ -32,18 +32,17 @@
         self.bzr_branch = bzr_branch
 
 
-class INewRevision(IObjectEvent):
-    """A new revision has been found in the branch."""
-
-
-class NewRevision(ScannerEvent):
-    """A new revision has been found in the branch."""
-
-    implements(INewRevision)
-
-    def __init__(self, db_branch, bzr_branch, db_revision, bzr_revision,
-                 revno):
-        """Construct a `NewRevision` event.
+class INewMainlineRevisions(IObjectEvent):
+    """A new revision has been found in the branch."""
+
+
+class NewMainlineRevisions(ScannerEvent):
+    """A new revision has been found in the branch."""
+
+    implements(INewMainlineRevisions)
+
+    def __init__(self, db_branch, bzr_branch, bzr_revisions):
+        """Construct a `NewRevisions` event.
 
         :param db_branch: The database branch.
         :param bzr_branch: The Bazaar branch.
@@ -53,13 +52,7 @@
             mainline.
         """
         ScannerEvent.__init__(self, db_branch, bzr_branch)
-        self.db_revision = db_revision
-        self.bzr_revision = bzr_revision
-        self.revno = revno
-
-    def isMainline(self):
-        """Is the new revision a mainline one?"""
-        return self.revno is not None
+        self.bzr_revisions = bzr_revisions
 
 
 class ITipChanged(IObjectEvent):

=== modified file 'lib/lp/codehosting/scanner/tests/test_buglinks.py'
--- lib/lp/codehosting/scanner/tests/test_buglinks.py	2012-02-07 06:48:22 +0000
+++ lib/lp/codehosting/scanner/tests/test_buglinks.py	2012-02-22 20:37:22 +0000
@@ -8,7 +8,6 @@
 from bzrlib.revision import Revision
 from zope.component import getUtility
 from zope.event import notify
-from zope.security.proxy import removeSecurityProxy
 
 from lp.app.errors import NotFoundError
 from lp.bugs.interfaces.bug import IBugSet
@@ -270,10 +269,9 @@
                 revprops={
                     'bugs': 'https://launchpad.net/bugs/%d fixed' % bug.id})
         bzr_revision = tree.branch.repository.get_revision(revision_id)
-        revno = 1
         revision_set = getUtility(IRevisionSet)
-        db_revision = revision_set.newFromBazaarRevision(bzr_revision)
-        notify(events.NewRevision(
-            db_branch, tree.branch, db_revision, bzr_revision, revno))
+        revision_set.newFromBazaarRevisions([bzr_revision])
+        notify(events.NewMainlineRevisions(
+            db_branch, tree.branch, [bzr_revision]))
         bug_branch = getUtility(IBugBranchSet).getBugBranch(bug, db_branch)
         self.assertIsNot(None, bug_branch)

=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
--- lib/lp/codehosting/scanner/tests/test_bzrsync.py	2012-02-21 22:46:28 +0000
+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py	2012-02-22 20:37:22 +0000
@@ -582,8 +582,8 @@
         self.assertIn(merge_id, branchrevisions_to_delete)
 
 
-class TestBzrSyncOneRevision(BzrSyncTestCase):
-    """Tests for `BzrSync.syncOneRevision`."""
+class TestBzrSyncRevisions(BzrSyncTestCase):
+    """Tests for `BzrSync.syncRevisions`."""
 
     def setUp(self):
         BzrSyncTestCase.setUp(self)
@@ -606,7 +606,7 @@
 
         # Sync the revision.  The second parameter is a dict of revision ids
         # to revnos, and will error if the revision id is not in the dict.
-        self.bzrsync.syncOneRevision(None, fake_rev, {'rev42': None})
+        self.bzrsync.syncRevisions(None, [fake_rev], {'rev42': None})
 
         # Find the revision we just synced and check that it has the correct
         # date.

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-02-20 02:07:55 +0000
+++ lib/lp/testing/factory.py	2012-02-22 20:37:22 +0000
@@ -48,6 +48,7 @@
 
 from bzrlib.merge_directive import MergeDirective2
 from bzrlib.plugins.builder.recipe import BaseRecipeBranch
+from bzrlib.revision import Revision as BzrRevision
 import pytz
 from pytz import UTC
 import simplejson
@@ -1545,6 +1546,18 @@
         return merge_proposal.generateIncrementalDiff(
             old_revision, new_revision, diff)
 
+    def makeBzrRevision(self, revision_id=None, parent_ids=None, **kwargs):
+        if revision_id is None:
+            revision_id = self.getUniqueString('revision-id')
+        if parent_ids is None:
+            parent_ids = []
+        return BzrRevision(
+            message=self.getUniqueString('message'),
+            revision_id=revision_id,
+            committer=self.getUniqueString('committer'),
+            parent_ids=parent_ids,
+            timestamp=0, timezone=0, properties=kwargs)
+
     def makeRevision(self, author=None, revision_date=None, parent_ids=None,
                      rev_id=None, log_body=None, date_created=None):
         """Create a single `Revision`."""


Follow ups