Launchpad itself

Merge lp:~gmb/launchpad/bug-heat-infrastructure-bug-503745 into lp:launchpad

  1. bug-heat-infrastructure-bug-503745
  2. Merge into devel
Proposed by Graham Binns
Status: Merged
Approved by: Henning Eggers
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~gmb/launchpad/bug-heat-infrastructure-bug-503745
Merge into: lp:launchpad
Diff against target: 1581 lines (+833/-190)
36 files modified
database/replication/Makefile (+3/-1)
database/replication/helpers.py (+20/-1)
database/replication/initialize.py (+2/-1)
database/replication/sync.py (+26/-0)
database/sampledata/current-dev.sql (+2/-2)
database/sampledata/current.sql (+2/-2)
database/schema/README (+1/-131)
database/schema/comments.sql (+2/-0)
database/schema/patch-2207-19-1.sql (+35/-0)
database/schema/patch-2207-20-0.sql (+13/-0)
database/schema/patch-2207-21-0.sql (+8/-0)
database/schema/patch-2207-24-0.sql (+20/-0)
database/schema/security.cfg (+8/-0)
database/schema/security.py (+3/-0)
lib/canonical/launchpad/scripts/garbo.py (+63/-0)
lib/lp/bugs/configure.zcml (+4/-2)
lib/lp/bugs/doc/bug-heat.txt (+54/-0)
lib/lp/bugs/interfaces/bug.py (+20/-0)
lib/lp/bugs/model/bug.py (+15/-1)
lib/lp/bugs/scripts/bugheat.py (+75/-0)
lib/lp/bugs/scripts/tests/test_bugheat.py (+183/-0)
lib/lp/bugs/tests/test_doc.py (+6/-0)
lib/lp/code/browser/codereviewvote.py (+11/-7)
lib/lp/code/errors.py (+5/-0)
lib/lp/code/interfaces/codereviewvote.py (+35/-1)
lib/lp/code/mail/tests/test_codehandler.py (+1/-1)
lib/lp/code/model/codereviewvote.py (+36/-13)
lib/lp/code/model/tests/test_branchjob.py (+9/-3)
lib/lp/code/model/tests/test_codereviewvote.py (+81/-6)
lib/lp/code/stories/branches/xx-branchmergeproposals.txt (+15/-10)
lib/lp/soyuz/interfaces/buildqueue.py (+18/-5)
lib/lp/soyuz/model/build.py (+2/-1)
lib/lp/soyuz/model/buildqueue.py (+3/-0)
lib/lp/soyuz/tests/test_buildqueue.py (+47/-0)
lib/lp/testing/__init__.py (+4/-1)
scripts/librarian-report.py (+1/-1)
To merge this branch: bzr merge lp:~gmb/launchpad/bug-heat-infrastructure-bug-503745
Reviewer Review Type Date Requested Status
Henning Eggers (community) code Approve
Review via email: mp+17137@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) wrote :

This branch does the initial infrastructure work for enabling us to calculate Bug heat.

It adds several classes which will be used by a cronscript to calculate the hotness values for all the bugs in Launchpad. It also exposes the hotness field of the Bug table via the IBug interface and sets up permissions for altering it appropriately.

Revision history for this message
Stuart Bishop (stub) wrote :

Please use DBLoopTuner, not LoopTuner.

This way the script will block if the database is in a state rather than make things worse.

Revision history for this message
Henning Eggers (henninge) wrote :
Download full text (13.3 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 11.01.2010 12:29, Graham Binns schrieb:
> This branch does the initial infrastructure work for enabling us to calculate Bug heat.

Thank you, Graham, for getting this feature started!

>
> It adds several classes which will be used by a cronscript to calculate the hotness values for all the bugs in Launchpad. It also exposes the hotness field of the Bug table via the IBug interface and sets up permissions for altering it appropriately.
>

The branch looks really good and straightforward, so I forgive you the
short cover letter ... ;-) I do have a few comments, though, two of them
very strong. Therefore:

  review needs-fixing

1. Using "heat" consistently as the term and not use "hotness" will
avoid confusion. We already agreed on that on IRC. Could you please also
file a bug about renaming the db column?

2. Using constants for heat calculations. Whenever I see constant values
used in code, some alarm goes off inside of me. With a collection of
values like this, multiple alarms where ringing ... ;-) Please see my
suggestion below.

Apart from that, just a few minor things.

Again, thanks for your work.

Henning

> === modified file 'lib/lp/bugs/configure.zcml'
> + hotness"/>
> + setHotness"/>

See above.

> === added file 'lib/lp/bugs/doc/bug-heat.txt'
> +
> +A new bug will have a hotness of zero.
> +
> + >>> from lp.testing.factory import LaunchpadObjectFactory
> +
> + >>> factory = LaunchpadObjectFactory()

You can delete those two lines, "factory" is already defined in doc tests.

> + >>> bug_owner = factory.makePerson()
> + >>> bug = factory.makeBug(owner=bug_owner)
> + >>> bug.hotness
> + 0
> +

[...]

> +The heat generated by duplicates is n*6, where n is the number of
> +duplicates that the bug has. With one duplicate,
> +_getHeatFromDuplicates() will return 6.

OK, this weighting value should definitely go into a constant of some
kind. I think a BugHeatConstants class in the interface file would be
appropriate.

class BugHeatConstants:
    """Constants needed to calculate bug heat."""

    DUPLICATES = 6

etc. See lib/lp/translations/interfaces/translations.py for an example.

Now, the question is what to do about the test. If you change the
weighting later on, do you want to update your test, too? In the
documentation and in the test code? You can decide on that but I think I
would do something like this:

Weighting parameters are found in BugHeatConstants.
The heat generated by duplicates is n*DUPLICATES, where n is the number of
duplicates that the bug has.

> +
> + >>> dupe = factory.makeBug()
> + >>> dupe.duplicateof = bug
> +
> + >>> bug_heat_calculator._getHeatFromDuplicates()
> + 6

>>> heat = bug_heat_calculator._getHeatFromDuplicates()
>>> print heat == BugHeatConstants.DUPLICATES
True

> +
> +With 5 duplicates, _getHeatFromDuplicates() will return 30.

With 5 duplicates, the value will be 5 times as high.

> +
> + >>> for i in range(4):
> + ... dupe = factory.makeBug()
> + ... dupe.duplicateof = bug
> + >>> transaction.commit()
> +
> + >>> bug_heat_calculator._getHeatFromDuplica...

review: Needs Fixing
Revision history for this message
Graham Binns (gmb) wrote :
Download full text (14.3 KiB)

Hi Henning, thanks for a very thorough review.

I've made most of the changes you suggested, except where we agreed
otherwise. Note that, in accordance with Stuart's recommendation I've
moved the tunable loop out of bugheat.py and into garbo.py as a
DBTunableLoop.

An incremental diff is attached.

On Mon, Jan 11, 2010 at 01:06:16PM -0000, Henning Eggers wrote:
> 1. Using "heat" consistently as the term and not use "hotness" will
> avoid confusion. We already agreed on that on IRC. Could you please also
> file a bug about renaming the db column?

I've changed the term in the code and filed bug 506514.

> 2. Using constants for heat calculations. Whenever I see constant values
> used in code, some alarm goes off inside of me. With a collection of
> values like this, multiple alarms where ringing ... ;-) Please see my
> suggestion below.

Understood. I've changed the class to use named constants.

> Apart from that, just a few minor things.
>
> Again, thanks for your work.
>
> Henning
>
>
>

> > + >>> factory = LaunchpadObjectFactory()
>
> You can delete those two lines, "factory" is already defined in doc tests.

Done.

> > +The heat generated by duplicates is n*6, where n is the number of
> > +duplicates that the bug has. With one duplicate,
> > +_getHeatFromDuplicates() will return 6.
>
> OK, this weighting value should definitely go into a constant of some
> kind. I think a BugHeatConstants class in the interface file would be
> appropriate.
>
> class BugHeatConstants:
> """Constants needed to calculate bug heat."""
>
> DUPLICATES = 6
>
> etc. See lib/lp/translations/interfaces/translations.py for an example.
>
> Now, the question is what to do about the test. If you change the
> weighting later on, do you want to update your test, too? In the
> documentation and in the test code? You can decide on that but I think I
> would do something like this:
>
>
> Weighting parameters are found in BugHeatConstants.
> The heat generated by duplicates is n*DUPLICATES, where n is the number of
> duplicates that the bug has.
>
> > +
> > + >>> dupe = factory.makeBug()
> > + >>> dupe.duplicateof = bug
> > +
> > + >>> bug_heat_calculator._getHeatFromDuplicates()
> > + 6
>
> >>> heat = bug_heat_calculator._getHeatFromDuplicates()
> >>> print heat == BugHeatConstants.DUPLICATES
> True
>
> > +
> > +With 5 duplicates, _getHeatFromDuplicates() will return 30.
>
> With 5 duplicates, the value will be 5 times as high.
>
> > +
> > + >>> for i in range(4):
> > + ... dupe = factory.makeBug()
> > + ... dupe.duplicateof = bug
> > + >>> transaction.commit()
> > +
> > + >>> bug_heat_calculator._getHeatFromDuplicates()
> > + 30
>
> >>> heat = bug_heat_calculator._getHeatFromDuplicates()
> >>> print heat == 5 * BugHeatConstants.DUPLICATES
> True
>
>
> But as I said, I leave that up to you to decide.
>
> This goes for all heat calculations, obviously, so I am not repeating
> that here.

I've done as you suggested and, as we discussed on IRC, transformed
these tests into unit tests.

> > +BugHeatCalculator.getBugHeat()
> > +------------------------------
> > +
> > +BugHeatCalculator.getBugHeat() returns ...

inc.diff
=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
--- lib/canonical/launchpad/scripts/garbo.py 2009-12-17 02:44:39 +0000
+++ lib/canonical/launchpad/scripts/garbo.py 2010-01-12 16:51:27 +0000
@@ -30,7 +30,9 @@
30from canonical.launchpad.utilities.looptuner import DBLoopTuner30from canonical.launchpad.utilities.looptuner import DBLoopTuner
31from canonical.launchpad.webapp.interfaces import (31from canonical.launchpad.webapp.interfaces import (
32 IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)32 IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)
33from lp.bugs.interfaces.bug import IBugSet
33from lp.bugs.model.bugnotification import BugNotification34from lp.bugs.model.bugnotification import BugNotification
35from lp.bugs.scripts.bugheat import BugHeatCalculator
34from lp.code.interfaces.revision import IRevisionSet36from lp.code.interfaces.revision import IRevisionSet
35from lp.code.model.branchjob import BranchJob37from lp.code.model.branchjob import BranchJob
36from lp.code.model.codeimportresult import CodeImportResult38from lp.code.model.codeimportresult import CodeImportResult
@@ -691,6 +693,66 @@
691 transaction.commit()693 transaction.commit()
692694
693695
696class BugHeatUpdater(TunableLoop):
697 """A `TunableLoop` for bug heat calculations."""
698
699 maximum_chunk_size = 1000
700
701 def __init__(self, log, abort_time=None):
702 super(BugHeatUpdater, self).__init__(log, abort_time)
703 self.transaction = transaction
704 self.offset = 0
705 self.total_updated = 0
706
707 def isDone(self):
708 """See `ITunableLoop`."""
709 # When the main loop has no more Bugs to process it sets
710 # offset to None. Until then, it always has a numerical
711 # value.
712 return self.offset is None
713
714 def __call__(self, chunk_size):
715 """Retrieve a batch of Bugs and update their heat.
716
717 See `ITunableLoop`.
718 """
719 # XXX 2010-01-08 gmb bug=198767:
720 # We cast chunk_size to an integer to ensure that we're not
721 # trying to slice using floats or anything similarly
722 # foolish. We shouldn't have to do this.
723 chunk_size = int(chunk_size)
724
725 start = self.offset
726 end = self.offset + chunk_size
727
728 transaction.begin()
729 # XXX 2010-01-08 gmb bug=505850:
730 # This method call should be taken out and shot as soon as
731 # we have a proper permissions system for scripts.
732 bugs = getUtility(IBugSet).dangerousGetAllBugs()[start:end]
733
734 self.offset = None
735 bug_count = bugs.count()
736 if bug_count > 0:
737 starting_id = bugs.first().id
738 self.log.debug("Updating %i Bugs (starting id: %i)" %
739 (bug_count, starting_id))
740
741 for bug in bugs:
742 # We set the starting point of the next batch to the Bug
743 # id after the one we're looking at now. If there aren't any
744 # bugs this loop will run for 0 iterations and starting_id
745 # will remain set to None.
746 start += 1
747 self.offset = start
748 self.log.debug("Updating heat for bug %s" % bug.id)
749 bug_heat_calculator = BugHeatCalculator(bug)
750 heat = bug_heat_calculator.getBugHeat()
751 bug.setHeat(heat)
752 self.total_updated += 1
753 transaction.commit()
754
755
694class BaseDatabaseGarbageCollector(LaunchpadCronScript):756class BaseDatabaseGarbageCollector(LaunchpadCronScript):
695 """Abstract base class to run a collection of TunableLoops."""757 """Abstract base class to run a collection of TunableLoops."""
696 script_name = None # Script name for locking and database user. Override.758 script_name = None # Script name for locking and database user. Override.
@@ -795,6 +857,7 @@
795 PersonEmailAddressLinkChecker,857 PersonEmailAddressLinkChecker,
796 BugNotificationPruner,858 BugNotificationPruner,
797 BranchJobPruner,859 BranchJobPruner,
860 BugHeatUpdater,
798 ]861 ]
799 experimental_tunable_loops = [862 experimental_tunable_loops = [
800 PersonPruner,863 PersonPruner,
801864
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2010-01-08 14:48:25 +0000
+++ lib/lp/bugs/configure.zcml 2010-01-11 12:00:48 +0000
@@ -573,7 +573,7 @@
573 personIsDirectSubscriber573 personIsDirectSubscriber
574 personIsAlsoNotifiedSubscriber574 personIsAlsoNotifiedSubscriber
575 personIsSubscribedToDuplicate575 personIsSubscribedToDuplicate
576 hotness"/>576 heat"/>
577 <require577 <require
578 permission="launchpad.View"578 permission="launchpad.View"
579 attributes="579 attributes="
@@ -670,7 +670,7 @@
670 permission="launchpad.Admin"670 permission="launchpad.Admin"
671 attributes="671 attributes="
672 setCommentVisibility672 setCommentVisibility
673 setHotness"/>673 setHeat"/>
674 </class>674 </class>
675 <adapter675 <adapter
676 for="lp.bugs.interfaces.bug.IBug"676 for="lp.bugs.interfaces.bug.IBug"
677677
=== modified file 'lib/lp/bugs/doc/bug-heat.txt'
--- lib/lp/bugs/doc/bug-heat.txt 2010-01-11 11:16:48 +0000
+++ lib/lp/bugs/doc/bug-heat.txt 2010-01-12 16:41:23 +0000
@@ -1,217 +1,54 @@
1Calculating bug heat1Calculating bug heat
2====================2====================
33
4Launchpad bugs each have a 'hotness' rating. This is an indicator of how4Launchpad bugs each have a 'heat' rating. This is an indicator of how
5problematic a given bug is to the community and can be used to determine5problematic a given bug is to the community and can be used to determine
6which bugs should be tackled first.6which bugs should be tackled first.
77
8A new bug will have a hotness of zero.8A new bug will have a heat of zero.
99
10 >>> from lp.testing.factory import LaunchpadObjectFactory
11
12 >>> factory = LaunchpadObjectFactory()
13 >>> bug_owner = factory.makePerson()10 >>> bug_owner = factory.makePerson()
14 >>> bug = factory.makeBug(owner=bug_owner)11 >>> bug = factory.makeBug(owner=bug_owner)
15 >>> bug.hotness12 >>> bug.heat
16 013 0
1714
18The bug's hotness can be set by calling its setHotness() method.15The bug's heat can be set by calling its setHeat() method.
1916
20 >>> bug.setHotness(42)17 >>> bug.setHeat(42)
21 >>> bug.hotness18 >>> bug.heat
22 4219 42
2320
24We'll reset it to zero for the purposes of testing.
25
26 >>> bug.setHotness(0)
27
28
29The BugHeatCalculator class
30---------------------------
31
32To calculate the heat for a particular bug, we use the BugHeatCalculator
33class. This class contains all the methods necessary to get the heat for
34a particular bug.
35
36 >>> from lp.bugs.scripts.bugheat import BugHeatCalculator
37 >>> bug_heat_calculator = BugHeatCalculator(bug)
38
39BugHeatCalculator has a number of private methods, each of which is
40called in turn to get the heat generated by one of the bug's attributes.
41
42
43BugHeatCalculator._getHeatFromDuplicates():
44------------------------------------------
45
46Each time a bug is duplicated its heat increases. The heat generated by
47duplicates is calculated using _getHeatFromDuplicates().
48_getHeatFromDuplicates() will return 0 for a bug with no duplicates.
49
50 >>> bug_heat_calculator._getHeatFromDuplicates()
51 0
52
53The heat generated by duplicates is n*6, where n is the number of
54duplicates that the bug has. With one duplicate,
55_getHeatFromDuplicates() will return 6.
56
57 >>> dupe = factory.makeBug()
58 >>> dupe.duplicateof = bug
59
60 >>> bug_heat_calculator._getHeatFromDuplicates()
61 6
62
63With 5 duplicates, _getHeatFromDuplicates() will return 30.
64
65 >>> for i in range(4):
66 ... dupe = factory.makeBug()
67 ... dupe.duplicateof = bug
68 >>> transaction.commit()
69
70 >>> bug_heat_calculator._getHeatFromDuplicates()
71 30
72
73
74BugHeatCalculator._getHeatFromAffectedUsers()
75---------------------------------------------
76
77The number of users affected by a bug also generates heat. The heat
78generated is returned by BugHeatCalculator._getHeatFromAffectedUsers()
79as n*4, where n is the number of affected users.
80
81The heat returned by _getHeatFromAffectedUsers() for our bug will be 4,
82since there is one affected user - the person who filed the bug.
83
84 >>> bug_heat_calculator._getHeatFromAffectedUsers()
85 4
86
87Adding some more affected users will increase the bug's heat.
88
89 >>> for i in range(4):
90 ... user = factory.makePerson()
91 ... bug.markUserAffected(user)
92
93 >>> bug_heat_calculator._getHeatFromAffectedUsers()
94 20
95
96
97BugHeatCalculator._getHeatFromSubscribers()
98-------------------------------------------
99
100The number of subscribers a bug has generates heat, too. This heat is
101calculated by BugHeatCalculator._getHeatFromSubscribers() and is
102returned as n*2, where n is the number of subscribers the bug has.
103
104The number of subscribers includes those subscribed to duplicates of the
105bug. In the case of our bug, therefore, the initial subscriber-generated
106heat will be 12 because there are six subscribers: the bug owner and
107five subscribers-from-duplicates.
108
109Note that indirect subscribers (assignees, structural subscribers, etc.)
110aren't included when generating heat.
111
112 >>> bug_heat_calculator._getHeatFromSubscribers()
113 12
114
115Adding some subscribers will increase the bug's heat.
116
117 >>> for i in range(3):
118 ... user = factory.makePerson()
119 ... bug.subscribe(user, user)
120 <BugSubscription...
121 <BugSubscription...
122 <BugSubscription...
123
124 >>> bug_heat_calculator._getHeatFromSubscribers()
125 18
126
127
128BugHeatCalculator._getHeatFromPrivacy()
129---------------------------------------
130
131BugHeatCalculator._getHeatFromPrivacy() returns the heat generated by
132the bug's private attribute. If the bug is public, this will be 0.
133
134 >>> bug_heat_calculator._getHeatFromPrivacy()
135 0
136
137However, if the bug is private, _getHeatFromPrivacy() will return 150.
138
139 >>> bug.setPrivate(True, bug_owner)
140 True
141
142 >>> bug_heat_calculator._getHeatFromPrivacy()
143 150
144
145Note that since the bug is now private its heat from subscribers has
146changed from 18 to 20. This is because the project owner, who previously
147had an indirect subscription to the bug, now as a direct subscription as
148a result of the behaviour of Bug.setPrivate()
149
150 >>> bug_heat_calculator._getHeatFromSubscribers()
151 20
152
153
154BugHeatCalculator._getHeatFromSecurity()
155----------------------------------------
156
157BugHeatCalculator._getHeatFromSecurity() returns the heat generated by
158the bug's security_related attribute. If the bug is not security
159related, _getHeatFromSecurity() will return 0.
160
161 >>> bug_heat_calculator._getHeatFromSecurity()
162 0
163
164If, on the other hand, the bug is security_related,
165_getHeatFromSecurity() will return 250.
166
167 >>> bug.security_related = True
168 >>> bug_heat_calculator._getHeatFromSecurity()
169 250
170
171
172BugHeatCalculator.getBugHeat()
173------------------------------
174
175BugHeatCalculator.getBugHeat() returns the total heat for a given bug as
176the sum of the results of all _getHeatFrom*() methods.
177
178For our bug, the total heat will be 470:
179
180 * From duplicates 30
181 * From subscribers 20
182 * From privacy 150
183 * From security 250
184 * From affected users 20
185
186 >>> bug_heat_calculator.getBugHeat()
187 470
188
18921
190The BugHeatUpdater class22The BugHeatUpdater class
191---------------------------23---------------------------
19224
193In order to calculate bug heat we need to use the BugHeatUpdater25In order to calculate bug heat we need to use the BugHeatUpdater
194class, which is designed precisely for that task.26class, which is designed precisely for that task. It's part of the garbo
27module and runs as part of the garbo-daily cronjob.
19528
196 >>> from lp.bugs.scripts.bugheat import BugHeatUpdater29 >>> from canonical.launchpad.scripts.garbo import BugHeatUpdater
197 >>> from canonical.launchpad.scripts import FakeLogger30 >>> from canonical.launchpad.scripts import FakeLogger
19831
199 >>> bug_heat_updater = BugHeatUpdater(transaction, logger=FakeLogger())32 >>> update_bug_heat = BugHeatUpdater(FakeLogger())
20033
201BugHeatUpdater's updateBugHeat() method will update the heat for all the34BugHeatUpdater implements ITunableLoop and as such is callable. Calling
202bugs currently held in Launchpad. We'll commit our transaction now so35it as a method will update the heat for all the bugs currently held in
203that all the updates we've made are in the database.36Launchpad.
20437
205 >>> transaction.commit()38Before update_bug_heat is called, bug 1 will have no heat.
20639
207 >>> bug_heat_updater.updateBugHeat()40 >>> from zope.component import getUtility
208 INFO Updating heat scores for all bugs41 >>> from lp.bugs.interfaces.bug import IBugSet
209 INFO Updating 1 Bugs (starting id: ...)42 >>> bug_1 = getUtility(IBugSet).get(1)
43
44 >>> bug_1.heat
45 0
46
47 >>> update_bug_heat(chunk_size=1)
48 DEBUG Updating 1 Bugs (starting id: ...)
210 ...49 ...
211 INFO Done updating heat for ... bugs50
21251Bug 1's heat will now be greater than 0.
213The hotness of the bug that we created at the start of this document52
214should now be 470.53 >>> bug_1.heat > 0
21554 True
216 >>> bug.hotness
217 470
21855
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2010-01-11 11:16:48 +0000
+++ lib/lp/bugs/interfaces/bug.py 2010-01-11 12:00:48 +0000
@@ -296,8 +296,8 @@
296 value_type=Reference(schema=IPerson),296 value_type=Reference(schema=IPerson),
297 readonly=True))297 readonly=True))
298298
299 hotness = Int(299 heat = Int(
300 title=_("The 'hotness' of the bug"),300 title=_("The 'heat' of the bug"),
301 required=False, readonly=True)301 required=False, readonly=True)
302302
303 # Adding related BugMessages provides a hook for getting at303 # Adding related BugMessages provides a hook for getting at
304304
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2010-01-08 14:48:25 +0000
+++ lib/lp/bugs/model/bug.py 2010-01-11 12:00:48 +0000
@@ -254,7 +254,10 @@
254 message_count = IntCol(notNull=True, default=0)254 message_count = IntCol(notNull=True, default=0)
255 users_affected_count = IntCol(notNull=True, default=0)255 users_affected_count = IntCol(notNull=True, default=0)
256 users_unaffected_count = IntCol(notNull=True, default=0)256 users_unaffected_count = IntCol(notNull=True, default=0)
257 hotness = IntCol(notNull=True, default=0)257
258 # This is called 'hotness' in the database but the canonical name in
259 # the code is 'heat', so we use that name here.
260 heat = IntCol(dbName='hotness', notNull=True, default=0)
258261
259 @property262 @property
260 def comment_count(self):263 def comment_count(self):
@@ -1426,9 +1429,9 @@
14261429
1427 return not subscriptions_from_dupes.is_empty()1430 return not subscriptions_from_dupes.is_empty()
14281431
1429 def setHotness(self, hotness):1432 def setHeat(self, heat):
1430 """See `IBug`."""1433 """See `IBug`."""
1431 self.hotness = hotness1434 self.heat = heat
14321435
14331436
1434class BugSet:1437class BugSet:
14351438
=== modified file 'lib/lp/bugs/scripts/bugheat.py'
--- lib/lp/bugs/scripts/bugheat.py 2010-01-11 11:24:40 +0000
+++ lib/lp/bugs/scripts/bugheat.py 2010-01-12 16:41:23 +0000
@@ -11,9 +11,17 @@
11from zope.interface import implements11from zope.interface import implements
1212
13from canonical.launchpad.interfaces.looptuner import ITunableLoop13from canonical.launchpad.interfaces.looptuner import ITunableLoop
14from canonical.launchpad.utilities.looptuner import LoopTuner14from canonical.launchpad.utilities.looptuner import DBLoopTuner
1515
16from lp.bugs.interfaces.bug import IBugSet16
17class BugHeatConstants:
18
19 PRIVACY = 150
20 SECURITY = 250
21 DUPLICATE = 6
22 AFFECTED_USER = 4
23 SUBSCRIBER = 2
24
1725
18class BugHeatCalculator:26class BugHeatCalculator:
19 """A class to calculate the heat for a bug."""27 """A class to calculate the heat for a bug."""
@@ -24,130 +32,44 @@
24 def _getHeatFromPrivacy(self):32 def _getHeatFromPrivacy(self):
25 """Return the heat generated by the bug's `private` attribute."""33 """Return the heat generated by the bug's `private` attribute."""
26 if self.bug.private:34 if self.bug.private:
27 return 15035 return BugHeatConstants.PRIVACY
28 else:36 else:
29 return 037 return 0
3038
31 def _getHeatFromSecurity(self):39 def _getHeatFromSecurity(self):
32 """Return the heat generated if the bug is security related."""40 """Return the heat generated if the bug is security related."""
33 if self.bug.security_related:41 if self.bug.security_related:
34 return 25042 return BugHeatConstants.SECURITY
35 else:43 else:
36 return 044 return 0
3745
38 def _getHeatFromDuplicates(self):46 def _getHeatFromDuplicates(self):
39 """Return the heat generated by the bug's duplicates."""47 """Return the heat generated by the bug's duplicates."""
40 return self.bug.duplicates.count() * 648 return self.bug.duplicates.count() * BugHeatConstants.DUPLICATE
4149
42 def _getHeatFromAffectedUsers(self):50 def _getHeatFromAffectedUsers(self):
43 """Return the heat generated by the bug's affected users."""51 """Return the heat generated by the bug's affected users."""
44 return self.bug.users_affected.count() * 452 return (
53 self.bug.users_affected.count() * BugHeatConstants.AFFECTED_USER)
4554
46 def _getHeatFromSubscribers(self):55 def _getHeatFromSubscribers(self):
47 """Return the heat generated by the bug's subscribers."""56 """Return the heat generated by the bug's subscribers."""
48 direct_subscribers = self.bug.getDirectSubscribers()57 direct_subscribers = self.bug.getDirectSubscribers()
49 subscribers_from_dupes = self.bug.getSubscribersFromDuplicates()58 subscribers_from_dupes = self.bug.getSubscribersFromDuplicates()
5059
51 return (60 subscriber_count = (
52 (len(direct_subscribers) + len(subscribers_from_dupes)) * 2)61 len(direct_subscribers) + len(subscribers_from_dupes))
62 return subscriber_count * BugHeatConstants.SUBSCRIBER
5363
54 def getBugHeat(self):64 def getBugHeat(self):
55 """Return the total heat for the current bug."""65 """Return the total heat for the current bug."""
56 heat_counts = [66 total_heat = sum([
57 self._getHeatFromAffectedUsers(),67 self._getHeatFromAffectedUsers(),
58 self._getHeatFromDuplicates(),68 self._getHeatFromDuplicates(),
59 self._getHeatFromPrivacy(),69 self._getHeatFromPrivacy(),
60 self._getHeatFromSecurity(),70 self._getHeatFromSecurity(),
61 self._getHeatFromSubscribers(),71 self._getHeatFromSubscribers(),
62 ]72 ])
63
64 total_heat = 0
65 for count in heat_counts:
66 total_heat += count
6773
68 return total_heat74 return total_heat
6975
70
71class BugHeatTunableLoop:
72 """An `ITunableLoop` implementation for bug heat calculations."""
73
74 implements(ITunableLoop)
75
76 total_updated = 0
77
78 def __init__(self, transaction, logger, offset=0):
79 self.transaction = transaction
80 self.logger = logger
81 self.offset = offset
82 self.total_updated = 0
83
84 def isDone(self):
85 """See `ITunableLoop`."""
86 # When the main loop has no more Bugs to process it sets
87 # offset to None. Until then, it always has a numerical
88 # value.
89 return self.offset is None
90
91 def __call__(self, chunk_size):
92 """Retrieve a batch of Bugs and update their heat.
93
94 See `ITunableLoop`.
95 """
96 # XXX 2010-01-08 gmb bug=198767:
97 # We cast chunk_size to an integer to ensure that we're not
98 # trying to slice using floats or anything similarly
99 # foolish. We shouldn't have to do this.
100 chunk_size = int(chunk_size)
101
102 start = self.offset
103 end = self.offset + chunk_size
104
105 self.transaction.begin()
106 # XXX 2010-01-08 gmb bug=505850:
107 # This method call should be taken out and shot as soon as
108 # we have a proper permissions system for scripts.
109 bugs = list(getUtility(IBugSet).dangerousGetAllBugs()[start:end])
110
111 self.offset = None
112 if bugs:
113 starting_id = bugs[0].id
114 self.logger.info("Updating %i Bugs (starting id: %i)" %
115 (len(bugs), starting_id))
116
117 for bug in bugs:
118 # We set the starting point of the next batch to the Bug
119 # id after the one we're looking at now. If there aren't any
120 # bugs this loop will run for 0 iterations and start_id
121 # will remain set to None.
122 start += 1
123 self.offset = start
124 self.logger.debug("Updating heat for bug %s" % bug.id)
125 bug_heat_calculator = BugHeatCalculator(bug)
126 heat = bug_heat_calculator.getBugHeat()
127 bug.setHotness(heat)
128 self.total_updated += 1
129
130 self.transaction.commit()
131
132
133class BugHeatUpdater:
134 """Takes responsibility for updating bug heat."""
135
136 def __init__(self, transaction, logger):
137 self.transaction = transaction
138 self.logger = logger
139
140 def updateBugHeat(self):
141 """Update the heat scores for all bugs."""
142 self.logger.info("Updating heat scores for all bugs")
143
144 loop = BugHeatTunableLoop(self.transaction, self.logger)
145
146 # We use the LoopTuner class to try and get an ideal number of
147 # bugs updated for each iteration of the loop (see the LoopTuner
148 # documentation for more details).
149 loop_tuner = LoopTuner(loop, 2)
150 loop_tuner.run()
151
152 self.logger.info(
153 "Done updating heat for %s bugs" % loop.total_updated)
15476
=== added file 'lib/lp/bugs/scripts/tests/test_bugheat.py'
--- lib/lp/bugs/scripts/tests/test_bugheat.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/scripts/tests/test_bugheat.py 2010-01-12 16:41:23 +0000
@@ -0,0 +1,183 @@
1
2# Copyright 2010 Canonical Ltd. This software is licensed under the
3# GNU Affero General Public License version 3 (see the file LICENSE).
4
5"""Module docstring goes here."""
6
7__metaclass__ = type
8
9import unittest
10
11from canonical.testing import LaunchpadZopelessLayer
12
13from lp.bugs.scripts.bugheat import BugHeatCalculator, BugHeatConstants
14from lp.testing import TestCaseWithFactory
15
16
17class TestBugHeatCalculator(TestCaseWithFactory):
18 """Tests for the BugHeatCalculator class."""
19
20 layer = LaunchpadZopelessLayer
21
22 def setUp(self):
23 super(TestBugHeatCalculator, self).setUp()
24 self.bug = self.factory.makeBug()
25 self.calculator = BugHeatCalculator(self.bug)
26
27 def test__getHeatFromDuplicates(self):
28 # BugHeatCalculator._getHeatFromDuplicates() returns the bug
29 # heat generated by duplicates of a bug.
30 # By default, the bug has no heat from dupes
31 self.assertEqual(0, self.calculator._getHeatFromDuplicates())
32
33 # If adding duplicates, the heat generated by them will be n *
34 # BugHeatConstants.DUPLICATE, where n is the number of
35 # duplicates.
36 for i in range(5):
37 dupe = self.factory.makeBug()
38 dupe.duplicateof = self.bug
39
40 expected_heat = BugHeatConstants.DUPLICATE * 5
41 actual_heat = self.calculator._getHeatFromDuplicates()
42 self.assertEqual(
43 expected_heat, actual_heat,
44 "Heat from duplicates does not match expected heat. "
45 "Expected %s, got %s" % (expected_heat, actual_heat))
46
47 def test__getHeatFromAffectedUsers(self):
48 # BugHeatCalculator._getHeatFromAffectedUsers() returns the bug
49 # heat generated by users affected by the bug.
50 # By default, the heat will be BugHeatConstants.AFFECTED_USER, since
51 # there will be one affected user (the user who filed the bug).
52 self.assertEqual(
53 BugHeatConstants.AFFECTED_USER,
54 self.calculator._getHeatFromAffectedUsers())
55
56 # As the number of affected users increases, the heat generated
57 # will be n * BugHeatConstants.AFFECTED_USER, where n is the number
58 # of affected users.
59 for i in range(5):
60 person = self.factory.makePerson()
61 self.bug.markUserAffected(person)
62
63 expected_heat = BugHeatConstants.AFFECTED_USER * 6
64 actual_heat = self.calculator._getHeatFromAffectedUsers()
65 self.assertEqual(
66 expected_heat, actual_heat,
67 "Heat from affected users does not match expected heat. "
68 "Expected %s, got %s" % (expected_heat, actual_heat))
69
70 def test__getHeatFromSubscribers(self):
71 # BugHeatCalculator._getHeatFromSubscribers() returns the bug
72 # heat generated by users subscribed tothe bug.
73 # By default, the heat will be BugHeatConstants.SUBSCRIBER,
74 # since there will be one direct subscriber (the user who filed
75 # the bug).
76 self.assertEqual(
77 BugHeatConstants.SUBSCRIBER,
78 self.calculator._getHeatFromSubscribers())
79
80 # As the number of subscribers increases, the heat generated
81 # will be n * BugHeatConstants.SUBSCRIBER, where n is the number
82 # of subscribers.
83 for i in range(5):
84 person = self.factory.makePerson()
85 self.bug.subscribe(person, person)
86
87 expected_heat = BugHeatConstants.SUBSCRIBER * 6
88 actual_heat = self.calculator._getHeatFromSubscribers()
89 self.assertEqual(
90 expected_heat, actual_heat,
91 "Heat from subscribers does not match expected heat. "
92 "Expected %s, got %s" % (expected_heat, actual_heat))
93
94 # Subscribers from duplicates are included in the heat returned
95 # by _getHeatFromSubscribers()
96 dupe = self.factory.makeBug()
97 dupe.duplicateof = self.bug
98 expected_heat = BugHeatConstants.SUBSCRIBER * 7
99 actual_heat = self.calculator._getHeatFromSubscribers()
100 self.assertEqual(
101 expected_heat, actual_heat,
102 "Heat from subscribers (including duplicate-subscribers) "
103 "does not match expected heat. Expected %s, got %s" %
104 (expected_heat, actual_heat))
105
106 # Seting the bug to private will increase its heat from
107 # subscribers by 1 * BugHeatConstants.SUBSCRIBER, as the project
108 # owner will now be directly subscribed to it.
109 self.bug.setPrivate(True, self.bug.owner)
110 expected_heat = BugHeatConstants.SUBSCRIBER * 8
111 actual_heat = self.calculator._getHeatFromSubscribers()
112 self.assertEqual(
113 expected_heat, actual_heat,
114 "Heat from subscribers to private bug does not match expected "
115 "heat. Expected %s, got %s" % (expected_heat, actual_heat))
116
117 def test__getHeatFromPrivacy(self):
118 # BugHeatCalculator._getHeatFromPrivacy() returns the heat
119 # generated by the bug's private attribute. If the bug is
120 # public, this will be 0.
121 self.assertEqual(0, self.calculator._getHeatFromPrivacy())
122
123 # However, if the bug is private, _getHeatFromPrivacy() will
124 # return BugHeatConstants.PRIVACY.
125 self.bug.setPrivate(True, self.bug.owner)
126 self.assertEqual(
127 BugHeatConstants.PRIVACY, self.calculator._getHeatFromPrivacy())
128
129 def test__getHeatFromSecurity(self):
130 # BugHeatCalculator._getHeatFromSecurity() returns the heat
131 # generated by the bug's security_related attribute. If the bug
132 # is not security related, _getHeatFromSecurity() will return 0.
133 self.assertEqual(0, self.calculator._getHeatFromPrivacy())
134
135
136 # If, on the other hand, the bug is security_related,
137 # _getHeatFromSecurity() will return BugHeatConstants.SECURITY
138 self.bug.security_related = True
139 self.assertEqual(
140 BugHeatConstants.SECURITY, self.calculator._getHeatFromSecurity())
141
142 def test_getBugHeat(self):
143 # BugHeatCalculator.getBugHeat() returns the total heat for a
144 # given bug as the sum of the results of all _getHeatFrom*()
145 # methods.
146 # By default this will be (BugHeatConstants.AFFECTED_USER +
147 # BugHeatConstants.SUBSCRIBER) since there will be one
148 # subscriber and one affected user only.
149 expected_heat = (
150 BugHeatConstants.AFFECTED_USER + BugHeatConstants.SUBSCRIBER)
151 actual_heat = self.calculator.getBugHeat()
152 self.assertEqual(
153 expected_heat, actual_heat,
154 "Expected bug heat did not match actual bug heat. "
155 "Expected %s, got %s" % (expected_heat, actual_heat))
156
157 # Adding a duplicate and making the bug private and security
158 # related will increase its heat.
159 dupe = self.factory.makeBug()
160 dupe.duplicateof = self.bug
161 self.bug.setPrivate(True, self.bug.owner)
162 self.bug.security_related = True
163
164 expected_heat += (
165 BugHeatConstants.DUPLICATE +
166 BugHeatConstants.PRIVACY +
167 BugHeatConstants.SECURITY
168 )
169
170 # Adding the duplicate and making the bug private means it gets
171 # two new subscribers, the project owner and the duplicate's
172 # direct subscriber.
173 expected_heat += BugHeatConstants.SUBSCRIBER * 2
174 actual_heat = self.calculator.getBugHeat()
175 self.assertEqual(
176 expected_heat, actual_heat,
177 "Expected bug heat did not match actual bug heat. "
178 "Expected %s, got %s" % (expected_heat, actual_heat))
179
180
181def test_suite():
182 return unittest.TestLoader().loadTestsFromName(__name__)
183
Revision history for this message
Henning Eggers (henninge) wrote :

Great work! I like the unit test ... ;-) Thanks for beating this branch into shape and a good thing that Stuart got to it, too. I completely overread the fact that you did not use DBLoopTuner and wouldn't have know anything about garbo.daily. That's team work!

Now go land this! :)

Henning

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'database/replication/Makefile'
--- database/replication/Makefile 2009-12-14 13:00:03 +0000
+++ database/replication/Makefile 2010-01-13 10:10:29 +0000
@@ -100,7 +100,7 @@
100 # up when roles don't exist in this cluster, and we rebuild it later100 # up when roles don't exist in this cluster, and we rebuild it later
101 # with security.py anyway.101 # with security.py anyway.
102 pg_restore --dbname=lpmain_staging_new \102 pg_restore --dbname=lpmain_staging_new \
103 --no-acl --no-owner --exit-on-error ${STAGING_DUMP}103 --no-owner --exit-on-error ${STAGING_DUMP}
104 psql -q -d lpmain_staging_new -f authdb_drop.sql104 psql -q -d lpmain_staging_new -f authdb_drop.sql
105 psql -q -d lpmain_staging_new -f authdb_create.sql \105 psql -q -d lpmain_staging_new -f authdb_create.sql \
106 2>&1 | grep -v _sl || true106 2>&1 | grep -v _sl || true
@@ -187,7 +187,9 @@
187 @echo Running fti.py `date`187 @echo Running fti.py `date`
188 ${SHHH} ../schema/fti.py188 ${SHHH} ../schema/fti.py
189 @echo Running security.py `date`189 @echo Running security.py `date`
190 ./slon_ctl.py stop # security.py can deadlock with slony
190 ${SHHH} ../schema/security.py --cluster -U slony191 ${SHHH} ../schema/security.py --cluster -U slony
192 ./slon_ctl.py --lag="0 seconds" start
191 # Migrate tables to the authdb replication set, creating the set193 # Migrate tables to the authdb replication set, creating the set
192 # and subscribing nodes to it as necessary.194 # and subscribing nodes to it as necessary.
193 ./populate_auth_replication_set.py -U slony195 ./populate_auth_replication_set.py -U slony
194196
=== modified file 'database/replication/helpers.py'
--- database/replication/helpers.py 2009-11-30 11:35:04 +0000
+++ database/replication/helpers.py 2010-01-13 10:10:29 +0000
@@ -71,8 +71,27 @@
71 'public.lp_person',71 'public.lp_person',
72 'public.lp_personlocation',72 'public.lp_personlocation',
73 'public.lp_teamparticipation',73 'public.lp_teamparticipation',
74 # Ubuntu SSO database. These tables where created manually by ISD
75 # and the Launchpad scripts should not mess with them. Eventually
76 # these tables will be in a totally separate database.
77 'public.auth_permission',
78 'public.auth_group',
79 'public.auth_user',
80 'public.auth_message',
81 'public.django_content_type',
82 'public.auth_permission',
83 'public.django_session',
84 'public.django_site',
85 'public.django_admin_log',
86 'public.ssoopenidrpconfig',
87 'public.auth_group_permissions',
88 'public.auth_user_groups',
89 'public.auth_user_user_permissions',
74 ])90 ])
7591
92# Calculate IGNORED_SEQUENCES
93IGNORED_SEQUENCES = set('%s_id_seq' % table for table in IGNORED_TABLES)
94
7695
77def slony_installed(con):96def slony_installed(con):
78 """Return True if the connected database is part of a Launchpad Slony-I97 """Return True if the connected database is part of a Launchpad Slony-I
@@ -447,7 +466,7 @@
447466
448 return (467 return (
449 all_tables - replicated_tables - IGNORED_TABLES,468 all_tables - replicated_tables - IGNORED_TABLES,
450 all_sequences - replicated_sequences)469 all_sequences - replicated_sequences - IGNORED_SEQUENCES)
451470
452471
453class ReplicationConfigError(Exception):472class ReplicationConfigError(Exception):
454473
=== modified file 'database/replication/initialize.py'
--- database/replication/initialize.py 2009-10-17 14:06:03 +0000
+++ database/replication/initialize.py 2010-01-13 10:10:29 +0000
@@ -224,7 +224,8 @@
224 fails += 1224 fails += 1
225 for sequence in all_sequences_in_schema(cur, 'public'):225 for sequence in all_sequences_in_schema(cur, 'public'):
226 times_seen = 0226 times_seen = 0
227 for sequence_set in [authdb_sequences, lpmain_sequences]:227 for sequence_set in [
228 authdb_sequences, lpmain_sequences, helpers.IGNORED_SEQUENCES]:
228 if sequence in sequence_set:229 if sequence in sequence_set:
229 times_seen += 1230 times_seen += 1
230 if times_seen == 0:231 if times_seen == 0:
231232
=== added file 'database/replication/sync.py'
--- database/replication/sync.py 1970-01-01 00:00:00 +0000
+++ database/replication/sync.py 2010-01-13 10:10:29 +0000
@@ -0,0 +1,26 @@
1#!/usr/bin/python2.5
2#
3# Copyright 2010 Canonical Ltd. This software is licensed under the
4# GNU Affero General Public License version 3 (see the file LICENSE).
5
6"""Block until the replication cluster synchronizes."""
7
8__metaclass__ = type
9__all__ = []
10
11import _pythonpath
12
13from optparse import OptionParser
14
15from canonical.launchpad.scripts import logger_options, db_options
16from replication.helpers import sync
17
18if __name__ == '__main__':
19 parser = OptionParser()
20 parser.add_option(
21 "-t", "--timeout", dest="timeout", metavar="SECS", type="int",
22 help="Abort if no sync after SECS seconds.", default=0)
23 logger_options(parser)
24 db_options(parser)
25 options, args = parser.parse_args()
26 sync(options.timeout)
027
=== modified file 'database/sampledata/current-dev.sql'
--- database/sampledata/current-dev.sql 2009-12-14 13:49:03 +0000
+++ database/sampledata/current-dev.sql 2010-01-13 10:10:29 +0000
@@ -1715,8 +1715,8 @@
17151715
1716ALTER TABLE buildqueue DISABLE TRIGGER ALL;1716ALTER TABLE buildqueue DISABLE TRIGGER ALL;
17171717
1718INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration) VALUES (1, 1, 'Dummy sampledata entry, not processing', 1, false, 1, 1, '00:00:00');1718INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration, processor, virtualized) VALUES (1, 1, 'Dummy sampledata entry, not processing', 1, false, 1, 1, '00:00:00', 1, FALSE);
1719INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration) VALUES (2, NULL, NULL, 10, false, 2, 1, '00:01:00');1719INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration, processor, virtualized) VALUES (2, NULL, NULL, 10, false, 2, 1, '00:01:00', 1, FALSE);
17201720
17211721
1722ALTER TABLE buildqueue ENABLE TRIGGER ALL;1722ALTER TABLE buildqueue ENABLE TRIGGER ALL;
17231723
=== modified file 'database/sampledata/current.sql'
--- database/sampledata/current.sql 2009-12-14 13:49:03 +0000
+++ database/sampledata/current.sql 2010-01-13 10:10:29 +0000
@@ -1697,8 +1697,8 @@
16971697
1698ALTER TABLE buildqueue DISABLE TRIGGER ALL;1698ALTER TABLE buildqueue DISABLE TRIGGER ALL;
16991699
1700INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration) VALUES (1, 1, 'Dummy sampledata entry, not processing', 1, false, 1, 1, '00:00:00');1700INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration, processor, virtualized) VALUES (1, 1, 'Dummy sampledata entry, not processing', 1, false, 1, 1, '00:00:00', 1, FALSE);
1701INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration) VALUES (2, NULL, NULL, 10, false, 2, 1, '00:01:00');1701INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration, processor, virtualized) VALUES (2, NULL, NULL, 10, false, 2, 1, '00:01:00', 1, FALSE);
17021702
17031703
1704ALTER TABLE buildqueue ENABLE TRIGGER ALL;1704ALTER TABLE buildqueue ENABLE TRIGGER ALL;
17051705
=== modified file 'database/schema/README'
--- database/schema/README 2006-10-27 01:02:38 +0000
+++ database/schema/README 2010-01-13 10:10:29 +0000
@@ -1,131 +1,1 @@
1= How to make database schema changes =1See https://dev.launchpad.net/PolicyAndProcess/DatabaseSchemaChangesProcess
2
3Important: This documentation is mirrored on https://launchpad.canonical.com/DatabaseSchemaChanges
4
5So, make sure to copy any changes you make here to the wiki page!
6
7== Making schema changes ==
8
9 1. Make an SQL file in `database/schema/pending/` containing the changes you want, excluding any changes to default values.
10 2. Run `make schema` to get a pristine database of sampledata.
11 3. Run the SQL on the database (`psql launchpad_dev -f your-patch.sql`) to ensure it works.
12 4. In `database/schema/`, `make newsampledata`.
13 5. Replace `current.sql` with `newsampledata.sql`.
14 6. Copy your SQL into `database/schema/` with a name like `patch-xx-99-0.sql` (where ''xx'' matches the existing patches), and ending with the line "INSERT INTO Launchpad``Database``Revision VALUES (xx, 99, 0);". Both the name and this last line should be renamed as directed when the patch is approved.
15 At this point `make schema` should work again.
16 7. Make any necessary changes to `lib/canonical/lp/dbschema.py`, `database/schema/fti.py`, and to the relevant `lib/canonical/launchpad/database/` classes.
17 8. Make any changes to the SQL patch to reflect new default values.
18
19== Proposing database schema changes ==
20
21For any tables and fields that you change with an SQL script via Stuart
22(stub on IRC), please make sure you include comments.
23
24The process now looks like this:
25
26 1. If you think the proposed changes may be controversial, or you are just ensure, it is worth discussing the changes on the launchpad mailing list first to avoid wasting your time.
27 2. Work on the patch in a branch as documented above.
28 3. Add the details of your branch to StuartBishop's review queue on PendingReviews.
29 4. Work on it in revision control till StuartBishop is happy. He will give you an official patch number
30 5. Rename your patch to match the official patch number
31 6. Once code is also ready and reviewed, commit as normal.
32
33== Resolving schema conflicts ==
34
35Resolving conflicts in `current.sql` manually is usually more trouble than it's worth. Instead, first resolve any conflicts in `comments.sql`, then: {{{
36
37 cd database/schema/
38 mv {patch-in-question}-0.sql comments.sql pending/
39 cp {parent branch, e.g. rocketfuel}/database/schema/comments.sql ./
40 cp ../sampledata/current.sql.OTHER ../sampledata/current.sql
41 make
42 psql launchpad_dev -f pending/patch-xx-99-0.sql
43 make newsampledata
44 mv ../sampledata/newsampledata.sql ../sampledata/current.sql
45 mv pending/{patch-in-question}-0.sql pending/comments.sql ./
46 make # Just to make sure everything works
47 cd ../..
48 bzr resolve database/sampledata/current.sql
49
50}}}
51
52= Production Database Upgrades =
53
54First get a copy of the Launchpad source built and ready on emperor, readable
55by the postgres user.
56
57Then, before you do anything else, inform #canonical, #launchpad and
58#ubuntu-devel that Launchpad and the Wiki authentication systems will be
59offline for 30 minutes (or longer if there is data migration to do).
60
61Stop PostgreSQL:
62
63 % pg_ctl -m fast stop
64
65Start PostgreSQL without external connections
66
67 % pg_ctl start -o '--tcpip-socket=false' -o '--ssl=false' \
68 -l /var/log/postgresql/postgres.log
69
70As user postgres, run the upgrade.py, fti.py and security.py scripts.
71fti.py can be skipped if you are sure no changes need to be made to the
72full text indexes (ie. fti.py has not been modified and no patches affect
73the tables being indexed). This process should work without issues, as any
74issues (such as DB patches not working on production data) will have been
75picked up from the daily updates to the staging environment. Do not run
76update.py using the --partial option to ensure that changes will be rolled
77back on failure.
78
79 % cd dists/launchpad/database/schema
80 % env LPCONFIG=production \
81 python upgrade.py -d launchpad_prod -U postgres -H ''
82 % env LPCONFIG=production \
83 python fti.py -d launchpad_prod -U postgres -H ''
84 % env LPCONFIG=production \
85 python security.py -d launchpad_prod -U postgres -H ''
86
87Restart PostgreSQL with external connections
88
89 % pg_ctl -m fast stop
90 % pg_ctl start -l /var/log/postgresql/postgres.log
91
92At this point, services should be restarted that don't automatically
93reconnect, such as the Launchpad web application servers and the Librarian.
94
95== Create a new development baseline ==
96
97After a production update, you should occasionally copy the live schema
98back into the development tree. This ensures that any differences that have
99crept in between the development database and reality are fixed.
100The new baseline dump (launchpad-XX-0-0.sql in this directory) can
101be generated on production using the following::
102
103 pg_dump -Fc --schema-only --no-owner --no-acl --schema=public \
104 launchpad_prod > launchpad.dump
105 pg_restore -l launchpad.dump | \
106 grep -v PROCEDURAL | grep -v COMMENT | \
107 grep -v FUNCTION | grep -v VIEW > launchpad.list
108 pg_restore -l launchpad.dump | grep VIEW >> launchpad.list
109 echo "-- Generated `date`" > launchpad.sql
110 echo 'SET client_min_messages=ERROR;' >> launchpad.sql
111 pg_restore --no-owner --no-acl -L launchpad.list launchpad.dump | \
112 grep -v '^--' >> launchpad.sql
113
114Move all the existing patches and the old baseline to the archive directory.
115Add the new baseline using the next revision number (should be in sync
116with the production release version). Create a patch-XX-0-0.sql patch
117to populate the LaunchpadDatabaseRevision table with the correct value
118so the tests pass.
119
120
121= Notes =
122
123There is a Makefile in launchpad/database/schema that will
124create the launchpad_test database (if it doesn't already exist),
125drop all your tables and create the current schema with all patches
126applied.
127
128If you want to check anything into the launchpad/database/schema
129directory, please do not give it a .sql extension or you will might
130confuse the simple Makefile.
131
1322
=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql 2009-12-01 13:45:58 +0000
+++ database/schema/comments.sql 2010-01-13 10:10:29 +0000
@@ -1537,6 +1537,8 @@
1537COMMENT ON COLUMN BuildQueue.job IS 'Foreign key to the `Job` table row with the generic job data.';1537COMMENT ON COLUMN BuildQueue.job IS 'Foreign key to the `Job` table row with the generic job data.';
1538COMMENT ON COLUMN BuildQueue.job_type IS 'Type of job (enumeration value), enables us to find/query the correct table with the data specific to this type of job.';1538COMMENT ON COLUMN BuildQueue.job_type IS 'Type of job (enumeration value), enables us to find/query the correct table with the data specific to this type of job.';
1539COMMENT ON COLUMN BuildQueue.estimated_duration IS 'Estimated job duration, based on previous running times of comparable jobs.';1539COMMENT ON COLUMN BuildQueue.estimated_duration IS 'Estimated job duration, based on previous running times of comparable jobs.';
1540COMMENT ON COLUMN BuildQueue.processor IS 'The processor required by the associated build farm job.';
1541COMMENT ON COLUMN BuildQueue.virtualized IS 'The virtualization setting required by the associated build farm job.';
15401542
1541-- Mirrors1543-- Mirrors
15421544
15431545
=== added file 'database/schema/patch-2207-19-1.sql'
--- database/schema/patch-2207-19-1.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-19-1.sql 2010-01-13 10:10:29 +0000
@@ -0,0 +1,35 @@
1SET client_min_messages=ERROR;
2
3CREATE INDEX bugtask__bugwatch__idx
4ON BugTask(bugwatch) WHERE bugwatch IS NOT NULL;
5
6CREATE INDEX translationimportqueueentry__productseries__idx
7ON TranslationImportQueueEntry(productseries)
8WHERE productseries IS NOT NULL;
9
10CREATE INDEX translationimportqueueentry__sourcepackagename__idx
11ON TranslationImportQueueEntry(sourcepackagename)
12WHERE sourcepackagename IS NOT NULL;
13
14CREATE INDEX translationimportqueueentry__path__idx
15ON TranslationImportQueueEntry(path);
16
17CREATE INDEX translationimportqueueentry__pofile__idx
18ON TranslationImportQueueEntry(pofile)
19WHERE pofile IS NOT NULL;
20
21CREATE INDEX translationimportqueueentry__potemplate__idx
22ON TranslationImportQueueEntry(potemplate)
23WHERE potemplate IS NOT NULL;
24
25CREATE INDEX pofile__from_sourcepackagename__idx
26ON POFile(from_sourcepackagename)
27WHERE from_sourcepackagename IS NOT NULL;
28
29CREATE INDEX bugwatch__lastchecked__idx ON BugWatch(lastchecked);
30CREATE INDEX bugwatch__remotebug__idx ON BugWatch(remotebug);
31CREATE INDEX bugwatch__remote_lp_bug_id__idx ON BUgWatch(remote_lp_bug_id)
32WHERE remote_lp_bug_id IS NOT NULL;
33
34
35INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 19, 1);
036
=== added file 'database/schema/patch-2207-20-0.sql'
--- database/schema/patch-2207-20-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-20-0.sql 2010-01-13 10:10:29 +0000
@@ -0,0 +1,13 @@
1SET client_min_messages=ERROR;
2
3-- Drop the old view.
4DROP VIEW validpersonorteamcache;
5
6-- Create the new view that excludes merged teams.
7CREATE VIEW validpersonorteamcache AS
8 SELECT person.id FROM
9 ((person LEFT JOIN emailaddress ON ((person.id = emailaddress.person))) LEFT JOIN account ON ((emailaddress.account = account.id)))
10 WHERE (((person.teamowner IS NOT NULL) AND (person.merged IS NULL)) OR
11 (person.teamowner IS NULL AND (account.status = 20) AND (emailaddress.status = 4)));
12
13INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 20, 0);
014
=== added file 'database/schema/patch-2207-21-0.sql'
--- database/schema/patch-2207-21-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-21-0.sql 2010-01-13 10:10:29 +0000
@@ -0,0 +1,8 @@
1SET client_min_messages=ERROR;
2
3ALTER TABLE Language ALTER englishname SET NOT NULL;
4
5ALTER TABLE LibraryFileContent ALTER filesize TYPE bigint;
6CLUSTER LibraryFileContent USING libraryfilecontent_pkey; -- repack
7
8INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 21, 0);
09
=== added file 'database/schema/patch-2207-24-0.sql'
--- database/schema/patch-2207-24-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-24-0.sql 2010-01-13 10:10:29 +0000
@@ -0,0 +1,20 @@
1-- Copyright 2009 Canonical Ltd. This software is licensed under the
2-- GNU Affero General Public License version 3 (see the file LICENSE).
3
4SET client_min_messages=ERROR;
5
6-- Another schema patch required for the Soyuz buildd generalisation, see
7-- https://dev.launchpad.net/Soyuz/Specs/BuilddGeneralisation for details.
8-- Bug #505725.
9
10-- Changes needed to the `BuildQueue` table.
11
12-- The 'processor' and the 'virtualized' columns will enable us to formulate
13-- more straightforward queries for finding candidate jobs when builders
14-- become idle.
15ALTER TABLE ONLY buildqueue ADD COLUMN processor integer;
16ALTER TABLE ONLY buildqueue ADD COLUMN virtualized boolean;
17
18CREATE INDEX buildqueue__processor__virtualized__idx ON buildqueue USING btree (processor, virtualized) WHERE (processor IS NOT NULL);
19
20INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 24, 0);
021
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2010-01-06 09:06:56 +0000
+++ database/schema/security.cfg 2010-01-13 10:10:29 +0000
@@ -1793,6 +1793,9 @@
1793# changing DB permissions.1793# changing DB permissions.
1794type=user1794type=user
1795groups=script,read1795groups=script,read
1796public.bug = SELECT, UPDATE
1797public.bugsubscription = SELECT
1798public.bugaffectsperson = SELECT
1796public.bugnotification = SELECT, DELETE1799public.bugnotification = SELECT, DELETE
1797public.bugnotificationrecipientarchive = SELECT1800public.bugnotificationrecipientarchive = SELECT
1798public.codeimportresult = SELECT, DELETE1801public.codeimportresult = SELECT, DELETE
@@ -1840,6 +1843,11 @@
18401843
1841[nagios]1844[nagios]
1842type=user1845type=user
1846public.archive = SELECT
1847public.build = SELECT
1848public.buildqueue = SELECT
1849public.buildpackagejob = SELECT
1850public.job = SELECT
1843public.libraryfilecontent = SELECT1851public.libraryfilecontent = SELECT
1844public.openidrpconfig = SELECT1852public.openidrpconfig = SELECT
1845public.branch = SELECT1853public.branch = SELECT
18461854
=== modified file 'database/schema/security.py'
--- database/schema/security.py 2009-11-18 08:09:58 +0000
+++ database/schema/security.py 2010-01-13 10:10:29 +0000
@@ -28,6 +28,9 @@
28# sensitive information that interactive sessions don't need.28# sensitive information that interactive sessions don't need.
29SECURE_TABLES = [29SECURE_TABLES = [
30 'public.accountpassword',30 'public.accountpassword',
31 'public.oauthnonce',
32 'public.openidnonce',
33 'public.openidconsumernonce',
31 ]34 ]
3235
3336
3437
=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
--- lib/canonical/launchpad/scripts/garbo.py 2009-12-17 02:44:39 +0000
+++ lib/canonical/launchpad/scripts/garbo.py 2010-01-13 10:10:29 +0000
@@ -30,7 +30,9 @@
30from canonical.launchpad.utilities.looptuner import DBLoopTuner30from canonical.launchpad.utilities.looptuner import DBLoopTuner
31from canonical.launchpad.webapp.interfaces import (31from canonical.launchpad.webapp.interfaces import (
32 IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)32 IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)
33from lp.bugs.interfaces.bug import IBugSet
33from lp.bugs.model.bugnotification import BugNotification34from lp.bugs.model.bugnotification import BugNotification
35from lp.bugs.scripts.bugheat import BugHeatCalculator
34from lp.code.interfaces.revision import IRevisionSet36from lp.code.interfaces.revision import IRevisionSet
35from lp.code.model.branchjob import BranchJob37from lp.code.model.branchjob import BranchJob
36from lp.code.model.codeimportresult import CodeImportResult38from lp.code.model.codeimportresult import CodeImportResult
@@ -691,6 +693,66 @@
691 transaction.commit()693 transaction.commit()
692694
693695
696class BugHeatUpdater(TunableLoop):
697 """A `TunableLoop` for bug heat calculations."""
698
699 maximum_chunk_size = 1000
700
701 def __init__(self, log, abort_time=None):
702 super(BugHeatUpdater, self).__init__(log, abort_time)
703 self.transaction = transaction
704 self.offset = 0
705 self.total_updated = 0
706
707 def isDone(self):
708 """See `ITunableLoop`."""
709 # When the main loop has no more Bugs to process it sets
710 # offset to None. Until then, it always has a numerical
711 # value.
712 return self.offset is None
713
714 def __call__(self, chunk_size):
715 """Retrieve a batch of Bugs and update their heat.
716
717 See `ITunableLoop`.
718 """
719 # XXX 2010-01-08 gmb bug=198767:
720 # We cast chunk_size to an integer to ensure that we're not
721 # trying to slice using floats or anything similarly
722 # foolish. We shouldn't have to do this.
723 chunk_size = int(chunk_size)
724
725 start = self.offset
726 end = self.offset + chunk_size
727
728 transaction.begin()
729 # XXX 2010-01-08 gmb bug=505850:
730 # This method call should be taken out and shot as soon as
731 # we have a proper permissions system for scripts.
732 bugs = getUtility(IBugSet).dangerousGetAllBugs()[start:end]
733
734 self.offset = None
735 bug_count = bugs.count()
736 if bug_count > 0:
737 starting_id = bugs.first().id
738 self.log.debug("Updating %i Bugs (starting id: %i)" %
739 (bug_count, starting_id))
740
741 for bug in bugs:
742 # We set the starting point of the next batch to the Bug
743 # id after the one we're looking at now. If there aren't any
744 # bugs this loop will run for 0 iterations and starting_id
745 # will remain set to None.
746 start += 1
747 self.offset = start
748 self.log.debug("Updating heat for bug %s" % bug.id)
749 bug_heat_calculator = BugHeatCalculator(bug)
750 heat = bug_heat_calculator.getBugHeat()
751 bug.setHeat(heat)
752 self.total_updated += 1
753 transaction.commit()
754
755
694class BaseDatabaseGarbageCollector(LaunchpadCronScript):756class BaseDatabaseGarbageCollector(LaunchpadCronScript):
695 """Abstract base class to run a collection of TunableLoops."""757 """Abstract base class to run a collection of TunableLoops."""
696 script_name = None # Script name for locking and database user. Override.758 script_name = None # Script name for locking and database user. Override.
@@ -795,6 +857,7 @@
795 PersonEmailAddressLinkChecker,857 PersonEmailAddressLinkChecker,
796 BugNotificationPruner,858 BugNotificationPruner,
797 BranchJobPruner,859 BranchJobPruner,
860 BugHeatUpdater,
798 ]861 ]
799 experimental_tunable_loops = [862 experimental_tunable_loops = [
800 PersonPruner,863 PersonPruner,
801864
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2009-12-09 11:14:16 +0000
+++ lib/lp/bugs/configure.zcml 2010-01-13 10:10:29 +0000
@@ -572,7 +572,8 @@
572 userCanView572 userCanView
573 personIsDirectSubscriber573 personIsDirectSubscriber
574 personIsAlsoNotifiedSubscriber574 personIsAlsoNotifiedSubscriber
575 personIsSubscribedToDuplicate"/>575 personIsSubscribedToDuplicate
576 heat"/>
576 <require577 <require
577 permission="launchpad.View"578 permission="launchpad.View"
578 attributes="579 attributes="
@@ -668,7 +669,8 @@
668 <require669 <require
669 permission="launchpad.Admin"670 permission="launchpad.Admin"
670 attributes="671 attributes="
671 setCommentVisibility"/>672 setCommentVisibility
673 setHeat"/>
672 </class>674 </class>
673 <adapter675 <adapter
674 for="lp.bugs.interfaces.bug.IBug"676 for="lp.bugs.interfaces.bug.IBug"
675677
=== added file 'lib/lp/bugs/doc/bug-heat.txt'
--- lib/lp/bugs/doc/bug-heat.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/doc/bug-heat.txt 2010-01-13 10:10:29 +0000
@@ -0,0 +1,54 @@
1Calculating bug heat
2====================
3
4Launchpad bugs each have a 'heat' rating. This is an indicator of how
5problematic a given bug is to the community and can be used to determine
6which bugs should be tackled first.
7
8A new bug will have a heat of zero.
9
10 >>> bug_owner = factory.makePerson()
11 >>> bug = factory.makeBug(owner=bug_owner)
12 >>> bug.heat
13 0
14
15The bug's heat can be set by calling its setHeat() method.
16
17 >>> bug.setHeat(42)
18 >>> bug.heat
19 42
20
21
22The BugHeatUpdater class
23---------------------------
24
25In order to calculate bug heat we need to use the BugHeatUpdater
26class, which is designed precisely for that task. It's part of the garbo
27module and runs as part of the garbo-daily cronjob.
28
29 >>> from canonical.launchpad.scripts.garbo import BugHeatUpdater
30 >>> from canonical.launchpad.scripts import FakeLogger
31
32 >>> update_bug_heat = BugHeatUpdater(FakeLogger())
33
34BugHeatUpdater implements ITunableLoop and as such is callable. Calling
35it as a method will update the heat for all the bugs currently held in
36Launchpad.
37
38Before update_bug_heat is called, bug 1 will have no heat.
39
40 >>> from zope.component import getUtility
41 >>> from lp.bugs.interfaces.bug import IBugSet
42 >>> bug_1 = getUtility(IBugSet).get(1)
43
44 >>> bug_1.heat
45 0
46
47 >>> update_bug_heat(chunk_size=1)
48 DEBUG Updating 1 Bugs (starting id: ...)
49 ...
50
51Bug 1's heat will now be greater than 0.
52
53 >>> bug_1.heat > 0
54 True
055
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2009-12-09 20:48:01 +0000
+++ lib/lp/bugs/interfaces/bug.py 2010-01-13 10:10:29 +0000
@@ -296,6 +296,10 @@
296 value_type=Reference(schema=IPerson),296 value_type=Reference(schema=IPerson),
297 readonly=True))297 readonly=True))
298298
299 heat = Int(
300 title=_("The 'heat' of the bug"),
301 required=False, readonly=True)
302
299 # Adding related BugMessages provides a hook for getting at303 # Adding related BugMessages provides a hook for getting at
300 # BugMessage.visible when building bug comments.304 # BugMessage.visible when building bug comments.
301 bug_messages = Attribute('The bug messages related to this object.')305 bug_messages = Attribute('The bug messages related to this object.')
@@ -732,6 +736,9 @@
732 if the user is the owner or an admin.736 if the user is the owner or an admin.
733 """737 """
734738
739 def setHeat(heat):
740 """Set the heat for the bug."""
741
735class InvalidDuplicateValue(Exception):742class InvalidDuplicateValue(Exception):
736 """A bug cannot be set as the duplicate of another."""743 """A bug cannot be set as the duplicate of another."""
737 webservice_error(417)744 webservice_error(417)
@@ -970,6 +977,19 @@
970 Otherwise, return False.977 Otherwise, return False.
971 """978 """
972979
980 def dangerousGetAllBugs():
981 """DO NOT CALL THIS METHOD.
982
983 This method exists solely to allow the bug heat script to grab
984 all the bugs in the database - including private ones - and
985 iterate over them. DO NOT USE IT UNLESS YOU KNOW WHAT YOU'RE
986 DOING. AND IF YOU KNOW WHAT YOU'RE DOING YOU KNOW BETTER THAN TO
987 USE THIS ANYWAY.
988 """
989 # XXX 2010-01-08 gmb bug=505850:
990 # Note, this method should go away when we have a proper
991 # permissions system for scripts.
992
973993
974class InvalidBugTargetType(Exception):994class InvalidBugTargetType(Exception):
975 """Bug target's type is not valid."""995 """Bug target's type is not valid."""
976996
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2009-12-14 15:25:55 +0000
+++ lib/lp/bugs/model/bug.py 2010-01-13 10:10:29 +0000
@@ -255,6 +255,10 @@
255 users_affected_count = IntCol(notNull=True, default=0)255 users_affected_count = IntCol(notNull=True, default=0)
256 users_unaffected_count = IntCol(notNull=True, default=0)256 users_unaffected_count = IntCol(notNull=True, default=0)
257257
258 # This is called 'hotness' in the database but the canonical name in
259 # the code is 'heat', so we use that name here.
260 heat = IntCol(dbName='hotness', notNull=True, default=0)
261
258 @property262 @property
259 def comment_count(self):263 def comment_count(self):
260 """See `IBug`."""264 """See `IBug`."""
@@ -1425,6 +1429,10 @@
14251429
1426 return not subscriptions_from_dupes.is_empty()1430 return not subscriptions_from_dupes.is_empty()
14271431
1432 def setHeat(self, heat):
1433 """See `IBug`."""
1434 self.heat = heat
1435
14281436
1429class BugSet:1437class BugSet:
1430 """See BugSet."""1438 """See BugSet."""
@@ -1665,13 +1673,19 @@
1665 return bugs1673 return bugs
16661674
1667 def getByNumbers(self, bug_numbers):1675 def getByNumbers(self, bug_numbers):
1668 """see `IBugSet`."""1676 """See `IBugSet`."""
1669 if bug_numbers is None or len(bug_numbers) < 1:1677 if bug_numbers is None or len(bug_numbers) < 1:
1670 return EmptyResultSet()1678 return EmptyResultSet()
1671 store = IStore(Bug)1679 store = IStore(Bug)
1672 result_set = store.find(Bug, In(Bug.id, bug_numbers))1680 result_set = store.find(Bug, In(Bug.id, bug_numbers))
1673 return result_set.order_by('id')1681 return result_set.order_by('id')
16741682
1683 def dangerousGetAllBugs(self):
1684 """See `IBugSet`."""
1685 store = IStore(Bug)
1686 result_set = store.find(Bug)
1687 return result_set.order_by('id')
1688
16751689
1676class BugAffectsPerson(SQLBase):1690class BugAffectsPerson(SQLBase):
1677 """A bug is marked as affecting a user."""1691 """A bug is marked as affecting a user."""
16781692
=== added file 'lib/lp/bugs/scripts/bugheat.py'
--- lib/lp/bugs/scripts/bugheat.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/scripts/bugheat.py 2010-01-13 10:10:29 +0000
@@ -0,0 +1,75 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""The innards of the Bug Heat cronscript."""
5
6__metaclass__ = type
7__all__ = []
8
9
10from zope.component import getUtility
11from zope.interface import implements
12
13from canonical.launchpad.interfaces.looptuner import ITunableLoop
14from canonical.launchpad.utilities.looptuner import DBLoopTuner
15
16
17class BugHeatConstants:
18
19 PRIVACY = 150
20 SECURITY = 250
21 DUPLICATE = 6
22 AFFECTED_USER = 4
23 SUBSCRIBER = 2
24
25
26class BugHeatCalculator:
27 """A class to calculate the heat for a bug."""
28
29 def __init__(self, bug):
30 self.bug = bug
31
32 def _getHeatFromPrivacy(self):
33 """Return the heat generated by the bug's `private` attribute."""
34 if self.bug.private:
35 return BugHeatConstants.PRIVACY
36 else:
37 return 0
38
39 def _getHeatFromSecurity(self):
40 """Return the heat generated if the bug is security related."""
41 if self.bug.security_related:
42 return BugHeatConstants.SECURITY
43 else:
44 return 0
45
46 def _getHeatFromDuplicates(self):
47 """Return the heat generated by the bug's duplicates."""
48 return self.bug.duplicates.count() * BugHeatConstants.DUPLICATE
49
50 def _getHeatFromAffectedUsers(self):
51 """Return the heat generated by the bug's affected users."""
52 return (
53 self.bug.users_affected.count() * BugHeatConstants.AFFECTED_USER)
54
55 def _getHeatFromSubscribers(self):
56 """Return the heat generated by the bug's subscribers."""
57 direct_subscribers = self.bug.getDirectSubscribers()
58 subscribers_from_dupes = self.bug.getSubscribersFromDuplicates()
59
60 subscriber_count = (
61 len(direct_subscribers) + len(subscribers_from_dupes))
62 return subscriber_count * BugHeatConstants.SUBSCRIBER
63
64 def getBugHeat(self):
65 """Return the total heat for the current bug."""
66 total_heat = sum([
67 self._getHeatFromAffectedUsers(),
68 self._getHeatFromDuplicates(),
69 self._getHeatFromPrivacy(),
70 self._getHeatFromSecurity(),
71 self._getHeatFromSubscribers(),
72 ])
73
74 return total_heat
75
076
=== added file 'lib/lp/bugs/scripts/tests/test_bugheat.py'
--- lib/lp/bugs/scripts/tests/test_bugheat.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/scripts/tests/test_bugheat.py 2010-01-13 10:10:29 +0000
@@ -0,0 +1,183 @@
1
2# Copyright 2010 Canonical Ltd. This software is licensed under the
3# GNU Affero General Public License version 3 (see the file LICENSE).
4
5"""Module docstring goes here."""
6
7__metaclass__ = type
8
9import unittest
10
11from canonical.testing import LaunchpadZopelessLayer
12
13from lp.bugs.scripts.bugheat import BugHeatCalculator, BugHeatConstants
14from lp.testing import TestCaseWithFactory
15
16
17class TestBugHeatCalculator(TestCaseWithFactory):
18 """Tests for the BugHeatCalculator class."""
19
20 layer = LaunchpadZopelessLayer
21
22 def setUp(self):
23 super(TestBugHeatCalculator, self).setUp()
24 self.bug = self.factory.makeBug()
25 self.calculator = BugHeatCalculator(self.bug)
26
27 def test__getHeatFromDuplicates(self):
28 # BugHeatCalculator._getHeatFromDuplicates() returns the bug
29 # heat generated by duplicates of a bug.
30 # By default, the bug has no heat from dupes
31 self.assertEqual(0, self.calculator._getHeatFromDuplicates())
32
33 # If adding duplicates, the heat generated by them will be n *
34 # BugHeatConstants.DUPLICATE, where n is the number of
35 # duplicates.
36 for i in range(5):
37 dupe = self.factory.makeBug()
38 dupe.duplicateof = self.bug
39
40 expected_heat = BugHeatConstants.DUPLICATE * 5
41 actual_heat = self.calculator._getHeatFromDuplicates()
42 self.assertEqual(
43 expected_heat, actual_heat,
44 "Heat from duplicates does not match expected heat. "
45 "Expected %s, got %s" % (expected_heat, actual_heat))
46
47 def test__getHeatFromAffectedUsers(self):
48 # BugHeatCalculator._getHeatFromAffectedUsers() returns the bug
49 # heat generated by users affected by the bug.
50 # By default, the heat will be BugHeatConstants.AFFECTED_USER, since
51 # there will be one affected user (the user who filed the bug).
52 self.assertEqual(
53 BugHeatConstants.AFFECTED_USER,
54 self.calculator._getHeatFromAffectedUsers())
55
56 # As the number of affected users increases, the heat generated
57 # will be n * BugHeatConstants.AFFECTED_USER, where n is the number
58 # of affected users.
59 for i in range(5):
60 person = self.factory.makePerson()
61 self.bug.markUserAffected(person)
62
63 expected_heat = BugHeatConstants.AFFECTED_USER * 6
64 actual_heat = self.calculator._getHeatFromAffectedUsers()
65 self.assertEqual(
66 expected_heat, actual_heat,
67 "Heat from affected users does not match expected heat. "
68 "Expected %s, got %s" % (expected_heat, actual_heat))
69
70 def test__getHeatFromSubscribers(self):
71 # BugHeatCalculator._getHeatFromSubscribers() returns the bug
72 # heat generated by users subscribed tothe bug.
73 # By default, the heat will be BugHeatConstants.SUBSCRIBER,
74 # since there will be one direct subscriber (the user who filed
75 # the bug).
76 self.assertEqual(
77 BugHeatConstants.SUBSCRIBER,
78 self.calculator._getHeatFromSubscribers())
79
80 # As the number of subscribers increases, the heat generated
81 # will be n * BugHeatConstants.SUBSCRIBER, where n is the number
82 # of subscribers.
83 for i in range(5):
84 person = self.factory.makePerson()
85 self.bug.subscribe(person, person)
86
87 expected_heat = BugHeatConstants.SUBSCRIBER * 6
88 actual_heat = self.calculator._getHeatFromSubscribers()
89 self.assertEqual(
90 expected_heat, actual_heat,
91 "Heat from subscribers does not match expected heat. "
92 "Expected %s, got %s" % (expected_heat, actual_heat))
93
94 # Subscribers from duplicates are included in the heat returned
95 # by _getHeatFromSubscribers()
96 dupe = self.factory.makeBug()
97 dupe.duplicateof = self.bug
98 expected_heat = BugHeatConstants.SUBSCRIBER * 7
99 actual_heat = self.calculator._getHeatFromSubscribers()
100 self.assertEqual(
101 expected_heat, actual_heat,
102 "Heat from subscribers (including duplicate-subscribers) "
103 "does not match expected heat. Expected %s, got %s" %
104 (expected_heat, actual_heat))
105
106 # Seting the bug to private will increase its heat from
107 # subscribers by 1 * BugHeatConstants.SUBSCRIBER, as the project
108 # owner will now be directly subscribed to it.
109 self.bug.setPrivate(True, self.bug.owner)
110 expected_heat = BugHeatConstants.SUBSCRIBER * 8
111 actual_heat = self.calculator._getHeatFromSubscribers()
112 self.assertEqual(
113 expected_heat, actual_heat,
114 "Heat from subscribers to private bug does not match expected "
115 "heat. Expected %s, got %s" % (expected_heat, actual_heat))
116
117 def test__getHeatFromPrivacy(self):
118 # BugHeatCalculator._getHeatFromPrivacy() returns the heat
119 # generated by the bug's private attribute. If the bug is
120 # public, this will be 0.
121 self.assertEqual(0, self.calculator._getHeatFromPrivacy())
122
123 # However, if the bug is private, _getHeatFromPrivacy() will
124 # return BugHeatConstants.PRIVACY.
125 self.bug.setPrivate(True, self.bug.owner)
126 self.assertEqual(
127 BugHeatConstants.PRIVACY, self.calculator._getHeatFromPrivacy())
128
129 def test__getHeatFromSecurity(self):
130 # BugHeatCalculator._getHeatFromSecurity() returns the heat
131 # generated by the bug's security_related attribute. If the bug
132 # is not security related, _getHeatFromSecurity() will return 0.
133 self.assertEqual(0, self.calculator._getHeatFromPrivacy())
134
135
136 # If, on the other hand, the bug is security_related,
137 # _getHeatFromSecurity() will return BugHeatConstants.SECURITY
138 self.bug.security_related = True
139 self.assertEqual(
140 BugHeatConstants.SECURITY, self.calculator._getHeatFromSecurity())
141
142 def test_getBugHeat(self):
143 # BugHeatCalculator.getBugHeat() returns the total heat for a
144 # given bug as the sum of the results of all _getHeatFrom*()
145 # methods.
146 # By default this will be (BugHeatConstants.AFFECTED_USER +
147 # BugHeatConstants.SUBSCRIBER) since there will be one
148 # subscriber and one affected user only.
149 expected_heat = (
150 BugHeatConstants.AFFECTED_USER + BugHeatConstants.SUBSCRIBER)
151 actual_heat = self.calculator.getBugHeat()
152 self.assertEqual(
153 expected_heat, actual_heat,
154 "Expected bug heat did not match actual bug heat. "
155 "Expected %s, got %s" % (expected_heat, actual_heat))
156
157 # Adding a duplicate and making the bug private and security
158 # related will increase its heat.
159 dupe = self.factory.makeBug()
160 dupe.duplicateof = self.bug
161 self.bug.setPrivate(True, self.bug.owner)
162 self.bug.security_related = True
163
164 expected_heat += (
165 BugHeatConstants.DUPLICATE +
166 BugHeatConstants.PRIVACY +
167 BugHeatConstants.SECURITY
168 )
169
170 # Adding the duplicate and making the bug private means it gets
171 # two new subscribers, the project owner and the duplicate's
172 # direct subscriber.
173 expected_heat += BugHeatConstants.SUBSCRIBER * 2
174 actual_heat = self.calculator.getBugHeat()
175 self.assertEqual(
176 expected_heat, actual_heat,
177 "Expected bug heat did not match actual bug heat. "
178 "Expected %s, got %s" % (expected_heat, actual_heat))
179
180
181def test_suite():
182 return unittest.TestLoader().loadTestsFromName(__name__)
183
0184
=== modified file 'lib/lp/bugs/tests/test_doc.py'
--- lib/lp/bugs/tests/test_doc.py 2009-10-02 11:29:22 +0000
+++ lib/lp/bugs/tests/test_doc.py 2010-01-13 10:10:29 +0000
@@ -89,6 +89,12 @@
89 '../doc/cve-update.txt',89 '../doc/cve-update.txt',
90 setUp=cveSetUp, tearDown=tearDown, layer=LaunchpadZopelessLayer90 setUp=cveSetUp, tearDown=tearDown, layer=LaunchpadZopelessLayer
91 ),91 ),
92 'bug-heat.txt': LayeredDocFileSuite(
93 '../doc/bug-heat.txt',
94 setUp=setUp,
95 tearDown=tearDown,
96 layer=LaunchpadZopelessLayer
97 ),
92 'bugnotificationrecipients.txt-uploader': LayeredDocFileSuite(98 'bugnotificationrecipients.txt-uploader': LayeredDocFileSuite(
93 '../doc/bugnotificationrecipients.txt',99 '../doc/bugnotificationrecipients.txt',
94 setUp=uploaderBugsSetUp,100 setUp=uploaderBugsSetUp,
95101
=== modified file 'lib/lp/code/browser/codereviewvote.py'
--- lib/lp/code/browser/codereviewvote.py 2009-12-10 20:46:32 +0000
+++ lib/lp/code/browser/codereviewvote.py 2010-01-13 10:10:29 +0000
@@ -1,20 +1,18 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4
5"""Views, navigation and actions for CodeReviewVotes."""4"""Views, navigation and actions for CodeReviewVotes."""
65
7
8__metaclass__ = type6__metaclass__ = type
97
108
11from zope.interface import Interface9from zope.interface import Interface
12from zope.security.proxy import removeSecurityProxy
1310
14from canonical.launchpad import _11from canonical.launchpad import _
15from canonical.launchpad.fields import PublicPersonChoice12from canonical.launchpad.fields import PublicPersonChoice
16from canonical.launchpad.webapp import (13from canonical.launchpad.webapp import (
17 action, canonical_url, LaunchpadFormView)14 action, canonical_url, LaunchpadFormView)
15from lp.code.errors import ReviewNotPending, UserHasExistingReview
1816
1917
20class ReassignSchema(Interface):18class ReassignSchema(Interface):
@@ -35,8 +33,14 @@
35 @action('Reassign', name='reassign')33 @action('Reassign', name='reassign')
36 def reassign_action(self, action, data):34 def reassign_action(self, action, data):
37 """Use the form data to change the review request reviewer."""35 """Use the form data to change the review request reviewer."""
38 # XXX TimPenhey 2009-12-11 bug=49520136 self.context.reassignReview(data['reviewer'])
39 # This should check for existing reviews by the reviewer, and have
40 # the logic moved into the model code.
41 removeSecurityProxy(self.context).reviewer = data['reviewer']
42 self.next_url = canonical_url(self.context.branch_merge_proposal)37 self.next_url = canonical_url(self.context.branch_merge_proposal)
38
39 def validate(self, data):
40 """Make sure that the reassignment can happen."""
41 reviewer = data.get('reviewer')
42 if reviewer is not None:
43 try:
44 self.context.validateReasignReview(reviewer)
45 except (ReviewNotPending, UserHasExistingReview), e:
46 self.addError(str(e))
4347
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2009-12-07 06:51:42 +0000
+++ lib/lp/code/errors.py 2010-01-13 10:10:29 +0000
@@ -11,6 +11,7 @@
11 'ClaimReviewFailed',11 'ClaimReviewFailed',
12 'InvalidBranchMergeProposal',12 'InvalidBranchMergeProposal',
13 'ReviewNotPending',13 'ReviewNotPending',
14 'UserHasExistingReview',
14 'UserNotBranchReviewer',15 'UserNotBranchReviewer',
15 'WrongBranchMergeProposal',16 'WrongBranchMergeProposal',
16]17]
@@ -43,6 +44,10 @@
43 """The requested review is not in a pending state."""44 """The requested review is not in a pending state."""
4445
4546
47class UserHasExistingReview(Exception):
48 """The user has an existing review."""
49
50
46class UserNotBranchReviewer(Exception):51class UserNotBranchReviewer(Exception):
47 """The user who attempted to review the merge proposal isn't a reviewer.52 """The user who attempted to review the merge proposal isn't a reviewer.
4853
4954
=== modified file 'lib/lp/code/interfaces/codereviewvote.py'
--- lib/lp/code/interfaces/codereviewvote.py 2009-12-07 06:51:42 +0000
+++ lib/lp/code/interfaces/codereviewvote.py 2010-01-13 10:10:29 +0000
@@ -17,10 +17,11 @@
17 IBranchMergeProposal)17 IBranchMergeProposal)
18from lp.code.interfaces.codereviewcomment import (18from lp.code.interfaces.codereviewcomment import (
19 ICodeReviewComment)19 ICodeReviewComment)
20from lp.registry.interfaces.person import IPerson
20from lazr.restful.fields import Reference21from lazr.restful.fields import Reference
21from lazr.restful.declarations import (22from lazr.restful.declarations import (
22 call_with, export_as_webservice_entry, export_destructor_operation,23 call_with, export_as_webservice_entry, export_destructor_operation,
23 export_write_operation, exported, REQUEST_USER)24 export_write_operation, exported, operation_parameters, REQUEST_USER)
2425
2526
26class ICodeReviewVoteReferencePublic(Interface):27class ICodeReviewVoteReferencePublic(Interface):
@@ -70,6 +71,15 @@
70class ICodeReviewVoteReferenceEdit(Interface):71class ICodeReviewVoteReferenceEdit(Interface):
71 """Method that require edit permissions."""72 """Method that require edit permissions."""
7273
74 def validateClaimReview(claimant):
75 """Implements the validation for claiming a review.
76
77 :raises ClaimReviewFailed: If the claimant already has a
78 personal review, if the reviewer is not a team, if the
79 claimant is not in the reviewer team, or if the review is
80 not pending.
81 """
82
73 @call_with(claimant=REQUEST_USER)83 @call_with(claimant=REQUEST_USER)
74 @export_write_operation()84 @export_write_operation()
75 def claimReview(claimant):85 def claimReview(claimant):
@@ -86,6 +96,30 @@
86 not pending.96 not pending.
87 """97 """
8898
99 def validateReasignReview(reviewer):
100 """Implements the validation for reassignment.
101
102 :raises ReviewNotPending: If the review is not pending.
103 :raises ReassignReviewFailed: If the reviewer is an individual and
104 already has a personal review.
105 """
106
107 @operation_parameters(
108 reviewer=Reference(
109 title=_("The person or team to assign to do the review."),
110 schema=IPerson))
111 @export_write_operation()
112 def reassignReview(reviewer):
113 """Reassign a pending review to someone else.
114
115 Pending reviews can be reassigned to someone else.
116
117 :param reviewer: The person to assign the pending review to.
118 :raises ReviewNotPending: If the review is not pending.
119 :raises ReassignReviewFailed: If the reviewer is an individual and
120 already has a personal review.
121 """
122
89 @export_destructor_operation()123 @export_destructor_operation()
90 def delete():124 def delete():
91 """Delete the pending review.125 """Delete the pending review.
92126
=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py 2010-01-07 06:37:14 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py 2010-01-13 10:10:29 +0000
@@ -832,7 +832,7 @@
832 target branch.832 target branch.
833 """833 """
834 db_target_branch, target_tree = self.create_branch_and_tree(834 db_target_branch, target_tree = self.create_branch_and_tree(
835 format=format)835 tree_location='.', format=format)
836 target_tree.branch.set_public_branch(db_target_branch.bzr_identity)836 target_tree.branch.set_public_branch(db_target_branch.bzr_identity)
837 target_tree.commit('rev1')837 target_tree.commit('rev1')
838 # Make sure that the created branch has been mirrored.838 # Make sure that the created branch has been mirrored.
839839
=== modified file 'lib/lp/code/model/codereviewvote.py'
--- lib/lp/code/model/codereviewvote.py 2009-12-07 06:51:42 +0000
+++ lib/lp/code/model/codereviewvote.py 2010-01-13 10:10:29 +0000
@@ -15,7 +15,8 @@
15from canonical.database.constants import DEFAULT15from canonical.database.constants import DEFAULT
16from canonical.database.datetimecol import UtcDateTimeCol16from canonical.database.datetimecol import UtcDateTimeCol
17from canonical.database.sqlbase import SQLBase17from canonical.database.sqlbase import SQLBase
18from lp.code.errors import ClaimReviewFailed, ReviewNotPending18from lp.code.errors import (
19 ClaimReviewFailed, ReviewNotPending, UserHasExistingReview)
19from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference20from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
2021
2122
@@ -44,10 +45,25 @@
44 # Reviews are pending if there is no associated comment.45 # Reviews are pending if there is no associated comment.
45 return self.comment is None46 return self.comment is None
4647
47 def claimReview(self, claimant):48 def _validatePending(self):
49 """Raise if the review is not pending."""
50 if not self.is_pending:
51 raise ReviewNotPending('The review is not pending.')
52
53 def _validateNoReviewForUser(self, user):
54 """Make sure there isn't an existing review for the user."""
55 bmp = self.branch_merge_proposal
56 existing_review = bmp.getUsersVoteReference(user)
57 if existing_review is not None:
58 if existing_review.is_pending:
59 error_str = '%s has already been asked to review this'
60 else:
61 error_str = '%s has already reviewed this'
62 raise UserHasExistingReview(error_str % user.unique_displayname)
63
64 def validateClaimReview(self, claimant):
48 """See `ICodeReviewVote`"""65 """See `ICodeReviewVote`"""
49 if not self.is_pending:66 self._validatePending()
50 raise ClaimReviewFailed('The review is not pending.')
51 if not self.reviewer.is_team:67 if not self.reviewer.is_team:
52 raise ClaimReviewFailed('Cannot claim non-team reviews.')68 raise ClaimReviewFailed('Cannot claim non-team reviews.')
53 if not claimant.inTeam(self.reviewer):69 if not claimant.inTeam(self.reviewer):
@@ -55,17 +71,24 @@
55 '%s is not a member of %s' %71 '%s is not a member of %s' %
56 (claimant.unique_displayname,72 (claimant.unique_displayname,
57 self.reviewer.unique_displayname))73 self.reviewer.unique_displayname))
58 claimant_review = (74 self._validateNoReviewForUser(claimant)
59 self.branch_merge_proposal.getUsersVoteReference(claimant))75
60 if claimant_review is not None:76 def claimReview(self, claimant):
61 if claimant_review.is_pending:77 """See `ICodeReviewVote`"""
62 error_str = '%s has an existing pending review'78 self.validateClaimReview(claimant)
63 else:
64 error_str = '%s has an existing personal review'
65 raise ClaimReviewFailed(
66 error_str % claimant.unique_displayname)
67 self.reviewer = claimant79 self.reviewer = claimant
6880
81 def validateReasignReview(self, reviewer):
82 """See `ICodeReviewVote`"""
83 self._validatePending()
84 if not reviewer.is_team:
85 self._validateNoReviewForUser(reviewer)
86
87 def reassignReview(self, reviewer):
88 """See `ICodeReviewVote`"""
89 self.validateReasignReview(reviewer)
90 self.reviewer = reviewer
91
69 def delete(self):92 def delete(self):
70 """See `ICodeReviewVote`"""93 """See `ICodeReviewVote`"""
71 if not self.is_pending:94 if not self.is_pending:
7295
=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
--- lib/lp/code/model/tests/test_branchjob.py 2009-11-21 00:07:19 +0000
+++ lib/lp/code/model/tests/test_branchjob.py 2010-01-13 10:10:29 +0000
@@ -8,6 +8,7 @@
8import datetime8import datetime
9import os9import os
10import shutil10import shutil
11import tempfile
11from unittest import TestLoader12from unittest import TestLoader
1213
13from bzrlib import errors as bzr_errors14from bzrlib import errors as bzr_errors
@@ -111,11 +112,16 @@
111 def test_run_diff_content(self):112 def test_run_diff_content(self):
112 """Ensure that run generates expected diff."""113 """Ensure that run generates expected diff."""
113 self.useBzrBranches()114 self.useBzrBranches()
114 branch, tree = self.create_branch_and_tree()115
115 open('file', 'wb').write('foo\n')116 tree_location = tempfile.mkdtemp()
117 self.addCleanup(lambda: shutil.rmtree(tree_location))
118
119 branch, tree = self.create_branch_and_tree(tree_location=tree_location)
120 tree_file = os.path.join(tree_location, 'file')
121 open(tree_file, 'wb').write('foo\n')
116 tree.add('file')122 tree.add('file')
117 tree.commit('First commit')123 tree.commit('First commit')
118 open('file', 'wb').write('bar\n')124 open(tree_file, 'wb').write('bar\n')
119 tree.commit('Next commit')125 tree.commit('Next commit')
120 job = BranchDiffJob.create(branch, '1', '2')126 job = BranchDiffJob.create(branch, '1', '2')
121 static_diff = job.run()127 static_diff = job.run()
122128
=== modified file 'lib/lp/code/model/tests/test_codereviewvote.py'
--- lib/lp/code/model/tests/test_codereviewvote.py 2009-12-07 06:51:42 +0000
+++ lib/lp/code/model/tests/test_codereviewvote.py 2010-01-13 10:10:29 +0000
@@ -9,7 +9,8 @@
9from canonical.testing import DatabaseFunctionalLayer9from canonical.testing import DatabaseFunctionalLayer
1010
11from lp.code.enums import CodeReviewVote11from lp.code.enums import CodeReviewVote
12from lp.code.errors import ClaimReviewFailed, ReviewNotPending12from lp.code.errors import (
13 ClaimReviewFailed, ReviewNotPending, UserHasExistingReview)
13from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference14from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
14from lp.testing import login_person, TestCaseWithFactory15from lp.testing import login_person, TestCaseWithFactory
1516
@@ -43,7 +44,7 @@
43 TestCaseWithFactory.setUp(self)44 TestCaseWithFactory.setUp(self)
44 # Setup the proposal, claimant and team reviewer.45 # Setup the proposal, claimant and team reviewer.
45 self.bmp = self.factory.makeBranchMergeProposal()46 self.bmp = self.factory.makeBranchMergeProposal()
46 self.claimant = self.factory.makePerson()47 self.claimant = self.factory.makePerson(name='eric')
47 self.review_team = self.factory.makeTeam()48 self.review_team = self.factory.makeTeam()
4849
49 def _addPendingReview(self):50 def _addPendingReview(self):
@@ -71,8 +72,10 @@
71 vote=CodeReviewVote.APPROVE)72 vote=CodeReviewVote.APPROVE)
72 review = self._addPendingReview()73 review = self._addPendingReview()
73 self._addClaimantToReviewTeam()74 self._addClaimantToReviewTeam()
74 self.assertRaises(75 self.assertRaisesWithContent(
75 ClaimReviewFailed, review.claimReview, self.claimant)76 UserHasExistingReview,
77 'Eric (eric) has already reviewed this',
78 review.claimReview, self.claimant)
7679
77 def test_personal_pending_review(self):80 def test_personal_pending_review(self):
78 # If the claimant has a pending review already, then they can't claim81 # If the claimant has a pending review already, then they can't claim
@@ -83,8 +86,10 @@
83 self.bmp.nominateReviewer(86 self.bmp.nominateReviewer(
84 reviewer=self.claimant, registrant=self.bmp.registrant)87 reviewer=self.claimant, registrant=self.bmp.registrant)
85 login_person(self.claimant)88 login_person(self.claimant)
86 self.assertRaises(89 self.assertRaisesWithContent(
87 ClaimReviewFailed, review.claimReview, self.claimant)90 UserHasExistingReview,
91 'Eric (eric) has already been asked to review this',
92 review.claimReview, self.claimant)
8893
89 def test_personal_not_in_review_team(self):94 def test_personal_not_in_review_team(self):
90 # If the claimant is not in the review team, an error is raised.95 # If the claimant is not in the review team, an error is raised.
@@ -183,5 +188,75 @@
183 self.assertRaises(ReviewNotPending, review.delete)188 self.assertRaises(ReviewNotPending, review.delete)
184189
185190
191class TestCodeReviewVoteReferenceReassignReview(TestCaseWithFactory):
192 """Tests for CodeReviewVoteReference.reassignReview."""
193
194 layer = DatabaseFunctionalLayer
195
196 def makeMergeProposalWithReview(self, completed=False):
197 """Return a new merge proposal with a review."""
198 bmp = self.factory.makeBranchMergeProposal()
199 reviewer = self.factory.makePerson()
200 if completed:
201 login_person(reviewer)
202 bmp.createComment(
203 reviewer, 'Message subject', 'Message content',
204 vote=CodeReviewVote.APPROVE)
205 [review] = list(bmp.votes)
206 else:
207 login_person(bmp.registrant)
208 review = bmp.nominateReviewer(
209 reviewer=reviewer, registrant=bmp.registrant)
210 return bmp, review
211
212 def test_reassign_pending(self):
213 # A pending review can be reassigned to someone else.
214 bmp, review = self.makeMergeProposalWithReview()
215 new_reviewer = self.factory.makePerson()
216 review.reassignReview(new_reviewer)
217 self.assertEqual(new_reviewer, review.reviewer)
218
219 def test_reassign_completed_review(self):
220 # A completed review cannot be reassigned
221 bmp, review = self.makeMergeProposalWithReview(completed=True)
222 self.assertRaises(
223 ReviewNotPending, review.reassignReview, bmp.registrant)
224
225 def test_reassign_to_user_existing_pending(self):
226 # If a user has an existing pending review, they cannot have another
227 # pending review assigned to them.
228 bmp, review = self.makeMergeProposalWithReview()
229 reviewer = self.factory.makePerson(name='eric')
230 user_review = bmp.nominateReviewer(
231 reviewer=reviewer, registrant=bmp.registrant)
232 self.assertRaisesWithContent(
233 UserHasExistingReview,
234 'Eric (eric) has already been asked to review this',
235 review.reassignReview, reviewer)
236
237 def test_reassign_to_user_existing_completed(self):
238 # If a user has an existing completed review, they cannot have another
239 # pending review assigned to them.
240 bmp, review = self.makeMergeProposalWithReview()
241 reviewer = self.factory.makePerson(name='eric')
242 bmp.createComment(
243 reviewer, 'Message subject', 'Message content',
244 vote=CodeReviewVote.APPROVE)
245 self.assertRaisesWithContent(
246 UserHasExistingReview,
247 'Eric (eric) has already reviewed this',
248 review.reassignReview, reviewer)
249
250 def test_reassign_to_team_existing(self):
251 # If a team has an existing review, they can have another pending
252 # review assigned to them.
253 bmp, review = self.makeMergeProposalWithReview()
254 reviewer_team = self.factory.makeTeam()
255 team_review = bmp.nominateReviewer(
256 reviewer=reviewer_team, registrant=bmp.registrant)
257 review.reassignReview(reviewer_team)
258 self.assertEqual(reviewer_team, review.reviewer)
259
260
186def test_suite():261def test_suite():
187 return TestLoader().loadTestsFromName(__name__)262 return TestLoader().loadTestsFromName(__name__)
188263
=== renamed file 'lib/lp/code/stories/branches/xx-branch-merge-proposals.txt' => 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt'
--- lib/lp/code/stories/branches/xx-branch-merge-proposals.txt 2010-01-07 21:02:00 +0000
+++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-01-13 10:10:29 +0000
@@ -233,15 +233,23 @@
233The claimant can reassign the review to someone else.233The claimant can reassign the review to someone else.
234234
235 >>> foobar_browser.getLink('Reassign').click()235 >>> foobar_browser.getLink('Reassign').click()
236 >>> foobar_browser.getControl('Reviewer').value = 'no-priv'
237 >>> foobar_browser.getControl('Reassign').click()
238
239If the person already has a review, the user gets an error...
240
241 >>> print_feedback_messages(foobar_browser.contents)
242 There is 1 error.
243 No Privileges Person (no-priv) has already reviewed this
244
245... if not, the review is reassigned.
246
236 >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team'247 >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team'
237 >>> foobar_browser.getControl('Reassign').click()248 >>> foobar_browser.getControl('Reassign').click()
238 >>> foobar_browser.open(klingon_proposal)
239 >>> pending = find_tag_by_id(
240 ... foobar_browser.contents, 'code-review-votes')
241249
242The review is now reassigned to the HWDB team.250The review is now reassigned to the HWDB team.
243251
244 >>> print extract_text(pending)252 >>> print_tag_with_id(foobar_browser.contents, 'code-review-votes')
245 Reviewer Review Type Date Requested Status...253 Reviewer Review Type Date Requested Status...
246 HWDB Team claimable ... ago Pending ...254 HWDB Team claimable ... ago Pending ...
247255
@@ -294,8 +302,7 @@
294 >>> sample_browser.getControl('Merged Revision Number').value = '42'302 >>> sample_browser.getControl('Merged Revision Number').value = '42'
295 >>> sample_browser.getControl('Mark as Merged').click()303 >>> sample_browser.getControl('Mark as Merged').click()
296304
297 >>> for message in get_feedback_messages(sample_browser.contents):305 >>> print_feedback_messages(sample_browser.contents)
298 ... print extract_text(message)
299 The proposal's merged revision has been updated.306 The proposal's merged revision has been updated.
300 >>> print_summary(sample_browser)307 >>> print_summary(sample_browser)
301 Status: Merged308 Status: Merged
@@ -436,8 +443,7 @@
436setting it gives an appropriate error.443setting it gives an appropriate error.
437444
438 >>> nopriv_browser.getControl('Propose Merge').click()445 >>> nopriv_browser.getControl('Propose Merge').click()
439 >>> for message in get_feedback_messages(nopriv_browser.contents):446 >>> print_feedback_messages(nopriv_browser.contents)
440 ... print extract_text(message)
441 There is 1 error.447 There is 1 error.
442 Required input is missing.448 Required input is missing.
443449
@@ -447,8 +453,7 @@
447 ... name='field.target_branch.target_branch').value = (453 ... name='field.target_branch.target_branch').value = (
448 ... 'fooix')454 ... 'fooix')
449 >>> nopriv_browser.getControl('Propose Merge').click()455 >>> nopriv_browser.getControl('Propose Merge').click()
450 >>> for message in get_feedback_messages(nopriv_browser.contents):456 >>> print_feedback_messages(nopriv_browser.contents)
451 ... print extract_text(message)
452 There is 1 error.457 There is 1 error.
453 Invalid value458 Invalid value
454459
455460
=== modified file 'lib/lp/soyuz/interfaces/buildqueue.py'
--- lib/lp/soyuz/interfaces/buildqueue.py 2010-01-07 04:07:09 +0000
+++ lib/lp/soyuz/interfaces/buildqueue.py 2010-01-13 10:10:29 +0000
@@ -13,7 +13,7 @@
13 ]13 ]
1414
15from zope.interface import Interface, Attribute15from zope.interface import Interface, Attribute
16from zope.schema import Choice, Datetime, Field, Timedelta16from zope.schema import Bool, Choice, Datetime, Field, Int, Text, Timedelta
1717
18from lazr.restful.fields import Reference18from lazr.restful.fields import Reference
1919
@@ -21,6 +21,8 @@
21from lp.buildmaster.interfaces.buildfarmjob import (21from lp.buildmaster.interfaces.buildfarmjob import (
22 IBuildFarmJob, BuildFarmJobType)22 IBuildFarmJob, BuildFarmJobType)
23from lp.services.job.interfaces.job import IJob23from lp.services.job.interfaces.job import IJob
24from lp.soyuz.interfaces.builder import IBuilder
25from lp.soyuz.interfaces.processor import IProcessor
2426
2527
26class IBuildQueue(Interface):28class IBuildQueue(Interface):
@@ -38,10 +40,21 @@
38 """40 """
3941
40 id = Attribute("Job identifier")42 id = Attribute("Job identifier")
41 builder = Attribute("The IBuilder instance processing this job")43 builder = Reference(
42 logtail = Attribute("The current tail of the log of the build")44 IBuilder, title=_("Builder"), required=True, readonly=True,
43 lastscore = Attribute("Last score to be computed for this job")45 description=_("The IBuilder instance processing this job"))
44 manual = Attribute("Whether or not the job was manually scored")46 logtail = Text(
47 description=_("The current tail of the log of the job"))
48 lastscore = Int(description=_("This job's score."))
49 manual = Bool(
50 description=_("Whether or not the job was manually scored."))
51 processor = Reference(
52 IProcessor, title=_("Processor"), required=False, readonly=True,
53 description=_("The processor required by this build farm job."))
54 virtualized = Bool(
55 required=False,
56 description=_(
57 "The virtualization setting required by this build farm job."))
4558
46 job = Reference(59 job = Reference(
47 IJob, title=_("Job"), required=True, readonly=True,60 IJob, title=_("Job"), required=True, readonly=True,
4861
=== modified file 'lib/lp/soyuz/model/build.py'
--- lib/lp/soyuz/model/build.py 2010-01-11 23:48:25 +0000
+++ lib/lp/soyuz/model/build.py 2010-01-13 10:10:29 +0000
@@ -682,7 +682,8 @@
682 queue_entry = BuildQueue(682 queue_entry = BuildQueue(
683 estimated_duration=duration_estimate,683 estimated_duration=duration_estimate,
684 job_type=BuildFarmJobType.PACKAGEBUILD,684 job_type=BuildFarmJobType.PACKAGEBUILD,
685 job=job.id)685 job=job.id, processor=self.processor,
686 virtualized=self.is_virtualized)
686 store.add(queue_entry)687 store.add(queue_entry)
687 return queue_entry688 return queue_entry
688689
689690
=== modified file 'lib/lp/soyuz/model/buildqueue.py'
--- lib/lp/soyuz/model/buildqueue.py 2010-01-11 23:48:25 +0000
+++ lib/lp/soyuz/model/buildqueue.py 2010-01-13 10:10:29 +0000
@@ -52,6 +52,9 @@
52 lastscore = IntCol(dbName='lastscore', default=0)52 lastscore = IntCol(dbName='lastscore', default=0)
53 manual = BoolCol(dbName='manual', default=False)53 manual = BoolCol(dbName='manual', default=False)
54 estimated_duration = IntervalCol()54 estimated_duration = IntervalCol()
55 processor = ForeignKey(
56 dbName='processor', foreignKey='Processor', notNull=True)
57 virtualized = BoolCol(dbName='virtualized')
5558
56 @property59 @property
57 def required_build_behavior(self):60 def required_build_behavior(self):
5861
=== modified file 'lib/lp/soyuz/tests/test_buildqueue.py'
--- lib/lp/soyuz/tests/test_buildqueue.py 2010-01-11 23:43:59 +0000
+++ lib/lp/soyuz/tests/test_buildqueue.py 2010-01-13 10:10:29 +0000
@@ -740,3 +740,50 @@
740 self.assertEqual(740 self.assertEqual(
741 bq.specific_job_classes[BuildFarmJobType.BRANCHBUILD],741 bq.specific_job_classes[BuildFarmJobType.BRANCHBUILD],
742 FakeBranchBuild)742 FakeBranchBuild)
743
744
745class TestPlatformData(TestCaseWithFactory):
746 """Tests covering the processor/virtualized properties."""
747
748 layer = LaunchpadZopelessLayer
749
750 def setUp(self):
751 """Set up a native x86 build for the test archive."""
752 super(TestPlatformData, self).setUp()
753
754 self.publisher = SoyuzTestPublisher()
755 self.publisher.prepareBreezyAutotest()
756
757 # First mark all builds in the sample data as already built.
758 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
759 sample_data = store.find(Build)
760 for build in sample_data:
761 build.buildstate = BuildStatus.FULLYBUILT
762 store.flush()
763
764 # We test builds that target a primary archive.
765 self.non_ppa = self.factory.makeArchive(
766 name="primary", purpose=ArchivePurpose.PRIMARY)
767 self.non_ppa.require_virtualized = False
768
769 self.builds = []
770 self.builds.extend(
771 self.publisher.getPubSource(
772 sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
773 archive=self.non_ppa).createMissingBuilds())
774
775 def test_JobPlatformSettings(self):
776 """The `BuildQueue` instance shares the processor/virtualized
777 properties with the associated `Build`."""
778 build, bq = find_job(self, 'gedit')
779
780 # Make sure the 'processor' properties are the same.
781 self.assertEqual(
782 bq.processor, build.processor,
783 "The 'processor' property deviates.")
784
785 # Make sure the 'virtualized' properties are the same.
786 self.assertEqual(
787 bq.virtualized, build.is_virtualized,
788 "The 'virtualized' property deviates.")
789
743790
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2009-12-22 23:50:27 +0000
+++ lib/lp/testing/__init__.py 2010-01-13 10:10:29 +0000
@@ -537,7 +537,7 @@
537 return BzrDir.create_branch_convenience(537 return BzrDir.create_branch_convenience(
538 branch_url, format=format)538 branch_url, format=format)
539539
540 def create_branch_and_tree(self, tree_location='.', product=None,540 def create_branch_and_tree(self, tree_location=None, product=None,
541 hosted=False, db_branch=None, format=None,541 hosted=False, db_branch=None, format=None,
542 **kwargs):542 **kwargs):
543 """Create a database branch, bzr branch and bzr checkout.543 """Create a database branch, bzr branch and bzr checkout.
@@ -562,6 +562,9 @@
562 if self.real_bzr_server:562 if self.real_bzr_server:
563 transaction.commit()563 transaction.commit()
564 bzr_branch = self.createBranchAtURL(branch_url, format=format)564 bzr_branch = self.createBranchAtURL(branch_url, format=format)
565 if tree_location is None:
566 tree_location = tempfile.mkdtemp()
567 self.addCleanup(lambda: shutil.rmtree(tree_location))
565 return db_branch, bzr_branch.create_checkout(568 return db_branch, bzr_branch.create_checkout(
566 tree_location, lightweight=True)569 tree_location, lightweight=True)
567570
568571
=== modified file 'scripts/librarian-report.py'
--- scripts/librarian-report.py 2009-12-22 02:25:12 +0000
+++ scripts/librarian-report.py 2010-01-13 10:10:29 +0000
@@ -71,7 +71,7 @@
71 cur.execute("""71 cur.execute("""
72 SELECT72 SELECT
73 COALESCE(SUM(filesize), 0),73 COALESCE(SUM(filesize), 0),
74 pg_size_pretty(COALESCE(SUM(filesize), 0)),74 pg_size_pretty(CAST(COALESCE(SUM(filesize), 0) AS bigint)),
75 COUNT(*)75 COUNT(*)
76 FROM (76 FROM (
77 SELECT DISTINCT ON (LFC.id) LFC.id, LFC.filesize77 SELECT DISTINCT ON (LFC.id) LFC.id, LFC.filesize