Launchpad itself

Merge lp:~rockstar/launchpad/branch-scanner-prep into lp:launchpad/db-devel

  1. branch-scanner-prep
  2. Merge into db-devel
Proposed by Paul Hummer
Status: Merged
Approved by: Tim Penhey
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~rockstar/launchpad/branch-scanner-prep
Merge into: lp:launchpad/db-devel
Diff against target: 157 lines (+97/-0)
3 files modified
database/schema/security.cfg (+2/-0)
lib/canonical/launchpad/scripts/garbo.py (+35/-0)
lib/canonical/launchpad/scripts/tests/test_garbo.py (+60/-0)
To merge this branch: bzr merge lp:~rockstar/launchpad/branch-scanner-prep
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Edwin Grubbs Pending
Review via email: mp+16260@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) wrote :

Hi Edwin-

  This branch adds a JobPruner to the daily garbo script. It removes any jobs
that have a date_finished of more than thirty days. This is needed for the
next few branches that turn the scanner into a job. If we didn't do this, the
job table would get out of hand really quick.

  To test, do:
   bin/test -vvc canonical.launchpad.scripts.tests.test_garbo \
   TestGarbo.test_JobPruner

 reviewer edwin-grubbs

Cheers,
Paul

Revision history for this message
Tim Penhey (thumper) wrote :

Hi Paul,

I think the job pruner should be altered somewhat.

At the very least it will fail now as there are more than just branch jobs.

Also we may want to be a little more selective in the deletion and not delete
pending jobs.

If we just have a BranchJob pruner for now, we won't be enforcing our rules on
other jobs.

  review needs-fixing

review: Needs Fixing
Revision history for this message
Tim Penhey (thumper) wrote :

Hi Paul,

You should really add a test to make sure that branch jobs that are less than 30 days old are not deleted.

Also it seems weird that you set the date to 30 days, and the sql expects < 30 days, not <=. Is there a commit somewhere? That should at least be commented.

Revision history for this message
Tim Penhey (thumper) wrote :

Why do you create an actual tree? Surely this isn't needed to test that the job isn't deleted.

Revision history for this message
Tim Penhey (thumper) wrote :

You no longer need tempfile, and you are missing comments on your test methods.

You also have two lines between your tests, and only one at the end, c.f. PEP-8.

review: Needs Fixing
Revision history for this message
Tim Penhey (thumper) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2010-01-04 15:59:17 +0000
+++ database/schema/security.cfg 2010-01-06 04:41:15 +0000
@@ -1808,6 +1808,8 @@
1808public.mailinglistsubscription = SELECT, DELETE1808public.mailinglistsubscription = SELECT, DELETE
1809public.teamparticipation = SELECT, DELETE1809public.teamparticipation = SELECT, DELETE
1810public.emailaddress = SELECT, UPDATE1810public.emailaddress = SELECT, UPDATE
1811public.job = SELECT, DELETE
1812public.branchjob = SELECT, DELETE
18111813
1812[garbo_daily]1814[garbo_daily]
1813type=user1815type=user
18141816
=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
--- lib/canonical/launchpad/scripts/garbo.py 2009-09-15 02:22:07 +0000
+++ lib/canonical/launchpad/scripts/garbo.py 2010-01-06 04:41:15 +0000
@@ -32,10 +32,12 @@
32 IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)32 IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)
33from lp.bugs.model.bugnotification import BugNotification33from lp.bugs.model.bugnotification import BugNotification
34from lp.code.interfaces.revision import IRevisionSet34from lp.code.interfaces.revision import IRevisionSet
35from lp.code.model.branchjob import BranchJob
35from lp.code.model.codeimportresult import CodeImportResult36from lp.code.model.codeimportresult import CodeImportResult
36from lp.code.model.revision import RevisionAuthor, RevisionCache37from lp.code.model.revision import RevisionAuthor, RevisionCache
37from lp.registry.model.mailinglist import MailingListSubscription38from lp.registry.model.mailinglist import MailingListSubscription
38from lp.registry.model.person import Person39from lp.registry.model.person import Person
40from lp.services.job.model.job import Job
39from lp.services.scripts.base import (41from lp.services.scripts.base import (
40 LaunchpadCronScript, SilentLaunchpadScriptFailure)42 LaunchpadCronScript, SilentLaunchpadScriptFailure)
4143
@@ -657,6 +659,38 @@
657 self.log.debug("Removed %d rows" % num_removed)659 self.log.debug("Removed %d rows" % num_removed)
658660
659661
662class BranchJobPruner(TunableLoop):
663 """Prune `BranchJob`s that are in a final state and older than a month old.
664
665 When a BranchJob is completed, it gets set to a final state. These jobs
666 should be pruned from the database after a month.
667 """
668
669 maximum_chunk_size = 1000
670
671 def __init__(self, log, abort_time=None):
672 super(BranchJobPruner, self).__init__(log, abort_time)
673 self.job_store = IMasterStore(BranchJob)
674
675 def isDone(self):
676 return self._ids_to_remove().any() is None
677
678 def _ids_to_remove(self):
679 jobs = self.job_store.find(
680 BranchJob.id,
681 BranchJob.job == Job.id,
682 Job.date_finished < THIRTY_DAYS_AGO)
683 return jobs
684
685 def __call__(self, chunk_size):
686 chunk_size = int(chunk_size)
687 ids_to_remove = list(self._ids_to_remove()[:chunk_size])
688 num_removed = self.job_store.find(
689 BranchJob,
690 In(BranchJob.id, ids_to_remove)).remove()
691 transaction.commit()
692
693
660class BaseDatabaseGarbageCollector(LaunchpadCronScript):694class BaseDatabaseGarbageCollector(LaunchpadCronScript):
661 """Abstract base class to run a collection of TunableLoops."""695 """Abstract base class to run a collection of TunableLoops."""
662 script_name = None # Script name for locking and database user. Override.696 script_name = None # Script name for locking and database user. Override.
@@ -760,6 +794,7 @@
760 MailingListSubscriptionPruner,794 MailingListSubscriptionPruner,
761 PersonEmailAddressLinkChecker,795 PersonEmailAddressLinkChecker,
762 BugNotificationPruner,796 BugNotificationPruner,
797 BranchJobPruner,
763 ]798 ]
764 experimental_tunable_loops = [799 experimental_tunable_loops = [
765 PersonPruner,800 PersonPruner,
766801
=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
--- lib/canonical/launchpad/scripts/tests/test_garbo.py 2009-08-21 19:21:11 +0000
+++ lib/canonical/launchpad/scripts/tests/test_garbo.py 2010-01-06 04:41:15 +0000
@@ -38,9 +38,12 @@
38 DatabaseLayer, LaunchpadScriptLayer, LaunchpadZopelessLayer)38 DatabaseLayer, LaunchpadScriptLayer, LaunchpadZopelessLayer)
39from lp.bugs.model.bugnotification import (39from lp.bugs.model.bugnotification import (
40 BugNotification, BugNotificationRecipient)40 BugNotification, BugNotificationRecipient)
41from lp.code.bzr import BranchFormat, RepositoryFormat
42from lp.code.model.branchjob import BranchJob, BranchUpgradeJob
41from lp.code.model.codeimportresult import CodeImportResult43from lp.code.model.codeimportresult import CodeImportResult
42from lp.registry.interfaces.person import IPersonSet, PersonCreationRationale44from lp.registry.interfaces.person import IPersonSet, PersonCreationRationale
43from lp.registry.model.person import Person45from lp.registry.model.person import Person
46from lp.services.job.model.job import Job
4447
4548
46class TestGarboScript(TestCase):49class TestGarboScript(TestCase):
@@ -510,6 +513,63 @@
510 "'mark@example.com' and 'mark' reference different Accounts")513 "'mark@example.com' and 'mark' reference different Accounts")
511 self.assertNotEqual(log_output.find(error_message_2), -1)514 self.assertNotEqual(log_output.find(error_message_2), -1)
512515
516 def test_BranchJobPruner(self):
517 # Garbo should remove jobs completed over 30 days ago.
518 self.useBzrBranches()
519 LaunchpadZopelessLayer.switchDbUser('testadmin')
520 store = IMasterStore(Job)
521
522 db_branch = self.factory.makeAnyBranch()
523 db_branch.branch_format = BranchFormat.BZR_BRANCH_5
524 db_branch.repository_format = RepositoryFormat.BZR_KNIT_1
525
526 branch_job = BranchUpgradeJob.create(db_branch)
527 branch_job.job.date_finished = THIRTY_DAYS_AGO
528 job_id = branch_job.job.id
529
530 self.assertEqual(
531 store.find(
532 BranchJob,
533 BranchJob.branch == db_branch.id).count(),
534 1)
535 transaction.commit()
536
537 collector = self.runDaily()
538
539 LaunchpadZopelessLayer.switchDbUser('testadmin')
540 self.assertEqual(
541 store.find(
542 BranchJob,
543 BranchJob.branch == db_branch.id).count(),
544 0)
545
546 def test_BranchJobPruner_doesnt_prune_recent_jobs(self):
547 # Check to make sure the garbo doesn't remove jobs that aren't more
548 # than thirty days old.
549 self.useBzrBranches()
550 LaunchpadZopelessLayer.switchDbUser('testadmin')
551 store = IMasterStore(Job)
552
553 db_branch = self.factory.makeAnyBranch()
554 db_branch.branch_format = BranchFormat.BZR_BRANCH_5
555 db_branch.repository_format = RepositoryFormat.BZR_KNIT_1
556
557 branch_job = BranchUpgradeJob.create(db_branch)
558 branch_job.job.date_finished = THIRTY_DAYS_AGO
559 job_id = branch_job.job.id
560
561 branch_job_newer = BranchUpgradeJob.create(db_branch)
562 job_id_newer = branch_job_newer.job.id
563 transaction.commit()
564
565 collector = self.runDaily()
566
567 LaunchpadZopelessLayer.switchDbUser('testadmin')
568 self.assertEqual(
569 store.find(
570 BranchJob).count(),
571 1)
572
513573
514def test_suite():574def test_suite():
515 return unittest.TestLoader().loadTestsFromName(__name__)575 return unittest.TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: