Jeroen T. Vermeulen has proposed merging 
lp:~jtv/launchpad/transitional-published into lp:launchpad with 
lp:~jtv/launchpad/bug-844577 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #844577 in Launchpad itself: "Gina imports should be Published, not 
Pending"
  https://bugs.launchpad.net/launchpad/+bug/844577

For more details, see:
https://code.launchpad.net/~jtv/launchpad/transitional-published/+merge/74720

= Summary =

Gina is learning to run domination on its Debian imports.  Yes, technically 
that makes Gina a dominatrix.

There is a catch: domination operates only on publication records in Published 
state.  So far, Gina has been creating Pending records.  That made sense for 
Ubuntu's import into Launchpad, way back when, which is what Gina was 
originally written for.  It does not make sense now, so we changed the code and 
updated the legacy data in batches.

Of course that does not take care of further legacy data that is created 
between the update we just did in the database and the time the Gina changes 
roll out.  But it should reduce the problem's size enough that we can afford to 
do the final update from inside Gina.  This will only need to run once, during 
“transitional domination” but will do nothing if run repeatedly.


== Pre-implementation notes ==

I had wanted to do a final patch-up update later, but William's points out that 
there is quite a substantial risk if domination runs on partially updated data. 
 So I inserted the change directly into the code; I have another branch waiting 
to remove the transitional code.


== Implementation details ==

The diff starts with some modernization of a doctest (using 
active_publishing_status instead of spelling out the Published and Pending 
statuses).  This is essentially cosmetic; it does not change the meaning of the 
code and really isn't relevant to the branch.  Buried in there however is one 
change where the test still assumes that Gina produces and maintains Pending 
publication records.  Before we remove transitional domination, we may have to 
re-think what happens there.


== Tests ==

Two new tests verify transitional behaviour.  One is very simple: an active 
SPPH is Pending but really ought to be Published.  Gina dominates and the 
SPPH's version is found in the simulated Sources list.  The Pending SPPH 
becomes Published.

Another test looks for the doom scenario that might occur when data is not 
properly migrated: an older release of a package is Published, but the version 
that is mentioned in the Sources list is still Pending.  It is essential that 
the live version be upgraded to Published before domination, or the dominator 
would decide that there is no newer published version to dominate the 
now-obsolete published SPPH, and mark it deleted.  Needless to say, the test 
shows doom being avoided.

{{{
./bin/test -vvc lp.soyuz.scripts.tests.test_gina
}}}

== Demo and Q/A ==


= Launchpad lint =

There is some pre-existing lint, especially in the doctest, that I can't afford 
to do too much about.  It's not just the effort (mostly a matter of running 
utilities/formatdoctest.py!) but also a matter of keeping the diff small.  I'm 
piling up quite a number of differences in a series of interdependent branches, 
and this would increase the risk of conflicts disproportionately.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/archivepublisher/domination.py
  lib/lp/soyuz/scripts/gina/dominate.py
  lib/lp/soyuz/model/publishing.py
  lib/lp/soyuz/doc/gina.txt
  lib/lp/soyuz/interfaces/publishing.py
  scripts/gina.py
  lib/lp/archivepublisher/tests/test_dominator.py
  lib/lp/soyuz/scripts/tests/test_gina.py
  lib/lp/soyuz/scripts/gina/handlers.py

./lib/lp/soyuz/doc/gina.txt
     113: narrative exceeds 78 characters.
     162: want exceeds 78 characters.
     179: want exceeds 78 characters.
     189: narrative uses a moin header.
     221: want exceeds 78 characters.
     234: want exceeds 78 characters.
     240: want exceeds 78 characters.
     295: source exceeds 78 characters.
     324: narrative uses a moin header.
     342: narrative exceeds 78 characters.
     354: narrative uses a moin header.
     360: narrative exceeds 78 characters.
     361: narrative exceeds 78 characters.
     459: narrative uses a moin header.
     461: narrative exceeds 78 characters.
     462: narrative exceeds 78 characters.
     477: narrative uses a moin header.
     563: narrative exceeds 78 characters.
     600: narrative uses a moin header.
     657: narrative uses a moin header.
     746: narrative uses a moin header.
     767: narrative uses a moin header.
     780: narrative uses a moin header.
./lib/lp/soyuz/interfaces/publishing.py
     381: E261 at least two spaces before inline comment
     478: E261 at least two spaces before inline comment
     511: E261 at least two spaces before inline comment
     681: E261 at least two spaces before inline comment
     767: E261 at least two spaces before inline comment
./scripts/gina.py
      26: '_pythonpath' imported but unused
-- 
https://code.launchpad.net/~jtv/launchpad/transitional-published/+merge/74720
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~jtv/launchpad/transitional-published into lp:launchpad.
=== modified file 'lib/lp/soyuz/doc/gina.txt'
--- lib/lp/soyuz/doc/gina.txt	2011-07-29 11:35:28 +0000
+++ lib/lp/soyuz/doc/gina.txt	2011-09-09 05:51:26 +0000
@@ -8,6 +8,7 @@
 Get the current counts of stuff in the database:
 
     >>> from canonical.launchpad.database.emailaddress import EmailAddress
+    >>> from lp.soyuz.interfaces.publishing import active_publishing_status
     >>> from lp.soyuz.model.publishing import (
     ...     BinaryPackagePublishingHistory,
     ...     SourcePackagePublishingHistory)
@@ -564,35 +565,34 @@
 that's what overrides actually do.
 
     >>> from canonical.database.sqlbase import sqlvalues
-    >>> from lp.soyuz.enums import PackagePublishingStatus
-    >>> x11_pub = SSPPH.select("""sourcepackagerelease = %s
-    ...                           AND distroseries = %s
-    ...                           AND status in (%s, %s)""" %
-    ...                         sqlvalues(x11p, breezy,
-    ...                          PackagePublishingStatus.PUBLISHED,
-    ...                          PackagePublishingStatus.PENDING),
-    ...                          orderBy=["-datecreated"])[0]
+    >>> x11_pub = SSPPH.select("""
+    ...     sourcepackagerelease = %s AND
+    ...     distroseries = %s AND
+    ...     status in %s
+    ...     """ % sqlvalues(
+    ...         x11p, breezy, active_publishing_status),
+    ...     orderBy=["-datecreated"])[0]
     >>> print x11_pub.section.name
     net
-    >>> ed_pub = SBPPH.select("""binarypackagerelease = %s
-    ...                           AND distroarchseries = %s
-    ...                           AND status in (%s, %s)""" %
-    ...                         sqlvalues(ed, breezy_i386,
-    ...                          PackagePublishingStatus.PUBLISHED,
-    ...                          PackagePublishingStatus.PENDING),
-    ...                          orderBy=["-datecreated"])[0]
+    >>> ed_pub = SBPPH.select("""
+    ...     binarypackagerelease = %s AND
+    ...     distroarchseries = %s AND
+    ...     status in %s
+    ...     """ % sqlvalues(
+    ...         ed, breezy_i386, active_publishing_status),
+    ...     orderBy=["-datecreated"])[0]
     >>> print ed_pub.priority
     Extra
     >>> n = SourcePackageName.selectOneBy(name="archive-copier")
     >>> ac = SourcePackageRelease.selectOneBy(sourcepackagenameID=n.id,
     ...         version="0.3.6")
-    >>> ac_pub = SSPPH.select("""sourcepackagerelease = %s
-    ...                          AND distroseries = %s
-    ...                          AND status in (%s, %s)""" %
-    ...                        sqlvalues(ac, breezy,
-    ...                         PackagePublishingStatus.PUBLISHED,
-    ...                         PackagePublishingStatus.PENDING),
-    ...                         orderBy=["-datecreated"])[0]
+    >>> ac_pub = SSPPH.select("""
+    ...     sourcepackagerelease = %s AND
+    ...     distroseries = %s AND
+    ...     status in %s
+    ...     """ % sqlvalues(
+    ...         ac, breezy, active_publishing_status),
+    ...     orderBy=["-datecreated"])[0]
     >>> print ac_pub.component.name
     universe
 
@@ -720,7 +720,7 @@
 
     >>> transaction.commit()
 
-There is now a number of source publications in PENDING status for the
+There is now a number of source publications in PUBLISHED status for the
 targetted distroseries, 'lenny'.
 
     >>> lenny_sources = SSPPH.select("distroseries = %s" % sqlvalues(lenny))
@@ -728,7 +728,7 @@
     12
 
     >>> print set([pub.status.name for pub in lenny_sources])
-    set(['PENDING'])
+    set(['PUBLISHED'])
 
 As mentioned before, lenny/i386 is empty, no binaries were imported.
 Also, the number of binaries published in the whole debian distribution

=== modified file 'lib/lp/soyuz/scripts/gina/dominate.py'
--- lib/lp/soyuz/scripts/gina/dominate.py	2011-09-09 05:51:25 +0000
+++ lib/lp/soyuz/scripts/gina/dominate.py	2011-09-09 05:51:26 +0000
@@ -27,6 +27,24 @@
     series = getUtility(IDistributionSet)[distro_name].getSeries(series_name)
     dominator = Dominator(logger, series.main_archive)
 
+    # XXX JeroenVermeulen 2011-09-08, bug=844550: This is a transitional
+    # hack.  Gina used to create SPPHs in Pending state.  We cleaned up
+    # the bulk of them, and changed the code to create Published ones, but
+    # some new ones will have been created since.
+    # Update those to match what the new Gina does.
+    from canonical.launchpad.interfaces.lpstorm import IStore
+    from lp.soyuz.enums import PackagePublishingStatus
+    from lp.soyuz.model.publishing import SourcePackagePublishingHistory
+    SPPH = SourcePackagePublishingHistory
+    store = IStore(SPPH)
+    spphs = store.find(
+        SPPH,
+        SPPH.archive == series.main_archive,
+        SPPH.distroseries == series,
+        SPPH.pocket == pocket,
+        SPPH.status == PackagePublishingStatus.PENDING)
+    spphs.set(status=PackagePublishingStatus.PUBLISHED)
+
     # Dominate packages found in the Sources list we're importing.
     package_names = dominator.findPublishedSourcePackageNames(series, pocket)
     for package_name in package_names:

=== modified file 'lib/lp/soyuz/scripts/tests/test_gina.py'
--- lib/lp/soyuz/scripts/tests/test_gina.py	2011-09-09 05:51:25 +0000
+++ lib/lp/soyuz/scripts/tests/test_gina.py	2011-09-09 05:51:26 +0000
@@ -77,6 +77,59 @@
             ],
             [pub.status for pub in pubs])
 
+    def test_dominate_imported_source_packages_cleans_up_pending_spphs(self):
+        # XXX JeroenVermeulen 2011-09-08, bug=844550: For transition to
+        # Gina domination, dominate_imported_source_packages turns any
+        # remaining Pending SPPHS into Published ones.
+        series = self.factory.makeDistroSeries()
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=series, archive=series.main_archive,
+            status=PackagePublishingStatus.PENDING)
+        spr = spph.sourcepackagerelease
+        package_name = spr.sourcepackagename.name
+        logger = DevNullLogger()
+        dominate_imported_source_packages(
+            logger, series.distribution.name, series.name, spph.pocket,
+            FakePackagesMap({package_name: [{"Version": spr.version}]}))
+        self.assertEqual(PackagePublishingStatus.PUBLISHED, spph.status)
+
+    def test_dominate_imported_source_packages_cleans_up_first(self):
+        # XXX JeroenVermeulen 2011-09-08, bug=844550: For transition to
+        # Gina domination, dominate_imported_source_packages turns any
+        # remaining Pending SPPHS into Published ones.  It does this
+        # *before* dominating, so no domination happens while some of
+        # the SPPHs are still mistakenly Pending (which would result in
+        # mistaken deletions).
+        series = self.factory.makeDistroSeries()
+        package = self.factory.makeSourcePackageName()
+        pocket = PackagePublishingPocket.RELEASE
+        versions = ['1.0', '1.1']
+        statuses_before = [
+            PackagePublishingStatus.PUBLISHED,
+            PackagePublishingStatus.PENDING,
+            ]
+        statuses_after = [
+            PackagePublishingStatus.SUPERSEDED,
+            PackagePublishingStatus.PUBLISHED,
+            ]
+        live_version = versions[-1]
+        sprs = [
+            self.factory.makeSourcePackageRelease(
+                sourcepackagename=package, version=version)
+            for version in versions]
+        spphs = [
+            self.factory.makeSourcePackagePublishingHistory(
+                archive=series.main_archive, distroseries=series,
+                sourcepackagerelease=spr, pocket=pocket, status=status)
+            for spr, status in zip(sprs, statuses_before)]
+
+        logger = DevNullLogger()
+        dominate_imported_source_packages(
+            logger, series.distribution.name, series.name, pocket,
+            FakePackagesMap({package.name: [{"Version": live_version}]}))
+
+        self.assertEqual(statuses_after, [spph.status for spph in spphs])
+
 
 class TestSourcePackagePublisher(TestCaseWithFactory):
 

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to