Merge lp:~thumper/launchpad/revision-karma-fix into lp:launchpad

Proposed by Tim Penhey
Status: Merged
Merged at revision: not available
Proposed branch: lp:~thumper/launchpad/revision-karma-fix
Merge into: lp:launchpad
Diff against target: 37 lines (+13/-7)
1 file modified
lib/lp/code/scripts/revisionkarma.py (+13/-7)
To merge this branch: bzr merge lp:~thumper/launchpad/revision-karma-fix
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+16890@code.launchpad.net

Commit message

Break out of the loop if karma is ever not allocated.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

The count added before is too inefficient so removed it.

Changed the looping logic, and stop if we ever fail to allocate karma as it will infinite-loop otherwise.

If it does die, it gives us the revision_id from which to start debugging.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I guess you could even say logger.critical(), but it looks fine.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/scripts/revisionkarma.py'
2--- lib/lp/code/scripts/revisionkarma.py 2010-01-05 01:51:56 +0000
3+++ lib/lp/code/scripts/revisionkarma.py 2010-01-06 04:11:20 +0000
4@@ -39,20 +39,26 @@
5 count = 0
6 revision_set = getUtility(IRevisionSet)
7 # Break into bits.
8- result_set = revision_set.getRevisionsNeedingKarmaAllocated()
9- self.logger.info("%d revisions to update", result_set.count())
10- revisions = list(result_set[:100])
11- while len(revisions) > 0:
12+ while True:
13+ revisions = list(
14+ revision_set.getRevisionsNeedingKarmaAllocated()[:100])
15+ if len(revisions) == 0:
16+ break
17 for revision in revisions:
18 # Find the appropriate branch, and allocate karma to it.
19 # Make sure we don't grab a junk branch though, as we don't
20 # allocate karma for junk branches.
21 branch = revision.getBranch(
22 allow_private=True, allow_junk=False)
23- revision.allocateKarma(branch)
24+ karma = revision.allocateKarma(branch)
25+ if karma is None:
26+ error_msg = (
27+ 'No karma generated for revid: %s (%s)' %
28+ (revision.revision_id, revision.id))
29+ self.logger.critical(error_msg)
30+ # Stop now to avoid infinite loop.
31+ raise AssertionError(error_msg)
32 count += 1
33 self.logger.debug("%s processed", count)
34 transaction.commit()
35- revisions = list(
36- revision_set.getRevisionsNeedingKarmaAllocated()[:100])
37 self.logger.info("Finished updating revision karma")