launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04911
[Merge] lp:~jtv/launchpad/transitional-published into lp:launchpad
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):
Follow ups
