Jeroen T. Vermeulen has proposed merging
lp:~jtv/launchpad/bug-849683-populators into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #849683 in Launchpad itself: "{B,S}PPH Populator are temporary"
https://bugs.launchpad.net/launchpad/+bug/849683
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-849683-populators/+merge/85461
= Summary =
Two new denormalized database columns have now been fully populated:
SourcePackagePublishingHistory.sourcepackagename
BinaryPackagePublishingHistory.binarypackagename
Database constraints (“NOT NULL”) to enforce this are on the way in. No nulls
have appeared in these columns over the past few weeks.
With that out of the way, we can retire the populators from garbo-hourly.
Which is what this branch does.
An earlier attempt to do this was nixed on the basis that the populators might
conceivably still be needed before the new constraints were deployed, to deal
with any remaining initialization bugs that had yet to manifest themselves. In
that event, still having the populators active might possibly increase the
chances that the database branch would be deployable despite being incompatible
with the running application, saving us holdups in the deployment pipeline; and
still having them in the codebase would also have saved us the trouble of
looking up and restoring the populator code from bzr history in the event that
a sudden burst of incorrect data would have been too large to clean up with
direct SQL. The certain cost of this caution would be a bit more obsolete
cruft to clean up, a bit more time and focus needed to complete this work, and
a bit more time spent on unnecessary garbo jobs.
A few weeks' hindsight since then tells us that this was all moot; we now know
that we could have deployed safely and moved ahead, as expected at the time,
and have this finished by now. Things may still break, but if they do, the
most likely cause is the delay introduced by our abundance of caution.
== Tests ==
{{{
./bin/test -vvc lp.scripts.tests.test_garbo
}}}
== Demo and Q/A ==
Not really the same thing, but keep an eye on anythng that might indicate that
nulls are being written to these columns. The most likely outcome would be
jobs failing with constraint violations.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/scripts/garbo.py
lib/lp/scripts/tests/test_garbo.py
--
https://code.launchpad.net/~jtv/launchpad/bug-849683-populators/+merge/85461
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/launchpad/bug-849683-populators into lp:launchpad.
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2011-12-09 02:19:10 +0000
+++ lib/lp/scripts/garbo.py 2011-12-13 10:52:29 +0000
@@ -85,10 +85,6 @@
SilentLaunchpadScriptFailure,
)
from lp.services.session.model import SessionData
-from lp.soyuz.model.publishing import (
- BinaryPackagePublishingHistory,
- SourcePackagePublishingHistory,
- )
from lp.translations.interfaces.potemplate import IPOTemplateSet
from lp.translations.model.potmsgset import POTMsgSet
from lp.translations.model.potranslation import POTranslation
@@ -965,86 +961,6 @@
transaction.commit()
-# XXX: StevenK 2011-09-14 bug=849683: This can be removed when done.
-class SourcePackagePublishingHistorySPNPopulator(TunableLoop):
- """Populate the new sourcepackagename column of SPPH."""
-
- done = False
- maximum_chunk_size = 5000
-
- SPPH = SourcePackagePublishingHistory
-
- def getStore(self):
- return IMasterStore(self.SPPH)
-
- def findSPPHs(self):
- SPPH = self.SPPH
- return self.getStore().find(SPPH.id, SPPH.sourcepackagename == None)
-
- def isDone(self):
- """See `TunableLoop`."""
- return self.done
-
- def __call__(self, chunk_size):
- """See `TunableLoop`."""
- spphs = list(self.findSPPHs()[:chunk_size])
- self.log.info("Populating %d SPPH(s).", len(spphs))
- if len(spphs) == 0:
- self.log.info("Finished populating SPPHs. Remove the populator.")
- self.done = True
- return
- self.getStore().execute("""
- UPDATE SourcePackagePublishingHistory AS SPPH
- SET sourcepackagename = SPR.sourcepackagename
- FROM SourcePackageRelease AS SPR
- WHERE
- SPR.id = SPPH.sourcepackagerelease AND
- SPPH.sourcepackagename IS NULL AND
- SPPH.id IN %s
- """ % sqlvalues(spphs))
- transaction.commit()
-
-
-# XXX: StevenK 2011-09-14 bug=849683: This can be removed when done.
-class BinaryPackagePublishingHistoryBPNPopulator(TunableLoop):
- """Populate the new binarypackagename column of BPPH."""
-
- done = False
- maximum_chunk_size = 5000
-
- BPPH = BinaryPackagePublishingHistory
-
- def getStore(self):
- return IMasterStore(self.BPPH)
-
- def findBPPHs(self):
- BPPH = self.BPPH
- return self.getStore().find(BPPH.id, BPPH.binarypackagename == None)
-
- def isDone(self):
- """See `TunableLoop`."""
- return self.done
-
- def __call__(self, chunk_size):
- """See `TunableLoop`."""
- bpphs = list(self.findBPPHs()[:chunk_size])
- self.log.info("Populating %d BPPH(s).", len(bpphs))
- if len(bpphs) == 0:
- self.log.info("Finished populating BPPHs. Remove the populator.")
- self.done = True
- return
- self.getStore().execute("""
- UPDATE BinaryPackagePublishingHistory AS BPPH
- SET binarypackagename = BPR.binarypackagename
- FROM BinaryPackageRelease AS BPR
- WHERE
- BPR.id = BPPH.binarypackagerelease AND
- BPPH.binarypackagename IS NULL AND
- BPPH.id IN %s
- """ % sqlvalues(bpphs))
- transaction.commit()
-
-
class BaseDatabaseGarbageCollector(LaunchpadCronScript):
"""Abstract base class to run a collection of TunableLoops."""
script_name = None # Script name for locking and database user. Override.
@@ -1294,8 +1210,6 @@
UnusedSessionPruner,
DuplicateSessionPruner,
BugHeatUpdater,
- SourcePackagePublishingHistorySPNPopulator,
- BinaryPackagePublishingHistoryBPNPopulator,
]
experimental_tunable_loops = []
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2011-12-09 02:19:10 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2011-12-13 10:52:29 +0000
@@ -1026,23 +1026,6 @@
self.runDaily()
self.assertEqual(0, unreferenced_msgsets.count())
- def test_SPPH_and_BPPH_populator(self):
- # If SPPHs (or BPPHs) do not have sourcepackagename (or
- # binarypackagename) set, the populator will set it.
- LaunchpadZopelessLayer.switchDbUser('testadmin')
- spph = self.factory.makeSourcePackagePublishingHistory()
- spn = spph.sourcepackagename
- removeSecurityProxy(spph).sourcepackagename = None
- bpph = self.factory.makeBinaryPackagePublishingHistory()
- bpn = bpph.binarypackagename
- removeSecurityProxy(bpph).binarypackagename = None
- transaction.commit()
- self.assertIs(None, spph.sourcepackagename)
- self.assertIs(None, bpph.binarypackagename)
- self.runHourly()
- self.assertEqual(spn, spph.sourcepackagename)
- self.assertEqual(bpn, bpph.binarypackagename)
-
class TestGarboTasks(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
