Merge lp:~gmb/launchpad/bug-heat-infrastructure-bug-503745 into lp:launchpad
- bug-heat-infrastructure-bug-503745
- 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 | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Henning Eggers (community) | code | Approve | |
|
Review via email:
|
|||
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
| Graham Binns (gmb) wrote : | # |
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/
> + hotness"/>
> + setHotness"/>
See above.
> === added file 'lib/lp/
> +
> +A new bug will have a hotness of zero.
> +
> + >>> from lp.testing.factory import LaunchpadObject
> +
> + >>> factory = LaunchpadObject
You can delete those two lines, "factory" is already defined in doc tests.
> + >>> bug_owner = factory.
> + >>> bug = factory.
> + >>> 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,
> +_getHeatFromDu
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/
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_
> + 6
>>> heat = bug_heat_
>>> print heat == BugHeatConstant
True
> +
> +With 5 duplicates, _getHeatFromDup
With 5 duplicates, the value will be 5 times as high.
> +
> + >>> for i in range(4):
> + ... dupe = factory.makeBug()
> + ... dupe.duplicateof = bug
> + >>> transaction.
> +
> + >>> bug_heat_
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 = LaunchpadObject
>
> 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,
> > +_getHeatFromDu
>
> 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/
>
> 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_
> > + 6
>
> >>> heat = bug_heat_
> >>> print heat == BugHeatConstant
> True
>
> > +
> > +With 5 duplicates, _getHeatFromDup
>
> With 5 duplicates, the value will be 5 times as high.
>
> > +
> > + >>> for i in range(4):
> > + ... dupe = factory.makeBug()
> > + ... dupe.duplicateof = bug
> > + >>> transaction.
> > +
> > + >>> bug_heat_
> > + 30
>
> >>> heat = bug_heat_
> >>> print heat == 5 * BugHeatConstant
> 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.
> > +BugHeatCalcula
> > +------
> > +
> > +BugHeatCalcula
| 1 | === modified file 'lib/canonical/launchpad/scripts/garbo.py' |
| 2 | --- lib/canonical/launchpad/scripts/garbo.py 2009-12-17 02:44:39 +0000 |
| 3 | +++ lib/canonical/launchpad/scripts/garbo.py 2010-01-12 16:51:27 +0000 |
| 4 | @@ -30,7 +30,9 @@ |
| 5 | from canonical.launchpad.utilities.looptuner import DBLoopTuner |
| 6 | from canonical.launchpad.webapp.interfaces import ( |
| 7 | IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR) |
| 8 | +from lp.bugs.interfaces.bug import IBugSet |
| 9 | from lp.bugs.model.bugnotification import BugNotification |
| 10 | +from lp.bugs.scripts.bugheat import BugHeatCalculator |
| 11 | from lp.code.interfaces.revision import IRevisionSet |
| 12 | from lp.code.model.branchjob import BranchJob |
| 13 | from lp.code.model.codeimportresult import CodeImportResult |
| 14 | @@ -691,6 +693,66 @@ |
| 15 | transaction.commit() |
| 16 | |
| 17 | |
| 18 | +class BugHeatUpdater(TunableLoop): |
| 19 | + """A `TunableLoop` for bug heat calculations.""" |
| 20 | + |
| 21 | + maximum_chunk_size = 1000 |
| 22 | + |
| 23 | + def __init__(self, log, abort_time=None): |
| 24 | + super(BugHeatUpdater, self).__init__(log, abort_time) |
| 25 | + self.transaction = transaction |
| 26 | + self.offset = 0 |
| 27 | + self.total_updated = 0 |
| 28 | + |
| 29 | + def isDone(self): |
| 30 | + """See `ITunableLoop`.""" |
| 31 | + # When the main loop has no more Bugs to process it sets |
| 32 | + # offset to None. Until then, it always has a numerical |
| 33 | + # value. |
| 34 | + return self.offset is None |
| 35 | + |
| 36 | + def __call__(self, chunk_size): |
| 37 | + """Retrieve a batch of Bugs and update their heat. |
| 38 | + |
| 39 | + See `ITunableLoop`. |
| 40 | + """ |
| 41 | + # XXX 2010-01-08 gmb bug=198767: |
| 42 | + # We cast chunk_size to an integer to ensure that we're not |
| 43 | + # trying to slice using floats or anything similarly |
| 44 | + # foolish. We shouldn't have to do this. |
| 45 | + chunk_size = int(chunk_size) |
| 46 | + |
| 47 | + start = self.offset |
| 48 | + end = self.offset + chunk_size |
| 49 | + |
| 50 | + transaction.begin() |
| 51 | + # XXX 2010-01-08 gmb bug=505850: |
| 52 | + # This method call should be taken out and shot as soon as |
| 53 | + # we have a proper permissions system for scripts. |
| 54 | + bugs = getUtility(IBugSet).dangerousGetAllBugs()[start:end] |
| 55 | + |
| 56 | + self.offset = None |
| 57 | + bug_count = bugs.count() |
| 58 | + if bug_count > 0: |
| 59 | + starting_id = bugs.first().id |
| 60 | + self.log.debug("Updating %i Bugs (starting id: %i)" % |
| 61 | + (bug_count, starting_id)) |
| 62 | + |
| 63 | + for bug in bugs: |
| 64 | + # We set the starting point of the next batch to the Bug |
| 65 | + # id after the one we're looking at now. If there aren't any |
| 66 | + # bugs this loop will run for 0 iterations and starting_id |
| 67 | + # will remain set to None. |
| 68 | + start += 1 |
| 69 | + self.offset = start |
| 70 | + self.log.debug("Updating heat for bug %s" % bug.id) |
| 71 | + bug_heat_calculator = BugHeatCalculator(bug) |
| 72 | + heat = bug_heat_calculator.getBugHeat() |
| 73 | + bug.setHeat(heat) |
| 74 | + self.total_updated += 1 |
| 75 | + transaction.commit() |
| 76 | + |
| 77 | + |
| 78 | class BaseDatabaseGarbageCollector(LaunchpadCronScript): |
| 79 | """Abstract base class to run a collection of TunableLoops.""" |
| 80 | script_name = None # Script name for locking and database user. Override. |
| 81 | @@ -795,6 +857,7 @@ |
| 82 | PersonEmailAddressLinkChecker, |
| 83 | BugNotificationPruner, |
| 84 | BranchJobPruner, |
| 85 | + BugHeatUpdater, |
| 86 | ] |
| 87 | experimental_tunable_loops = [ |
| 88 | PersonPruner, |
| 89 | |
| 90 | === modified file 'lib/lp/bugs/configure.zcml' |
| 91 | --- lib/lp/bugs/configure.zcml 2010-01-08 14:48:25 +0000 |
| 92 | +++ lib/lp/bugs/configure.zcml 2010-01-11 12:00:48 +0000 |
| 93 | @@ -573,7 +573,7 @@ |
| 94 | personIsDirectSubscriber |
| 95 | personIsAlsoNotifiedSubscriber |
| 96 | personIsSubscribedToDuplicate |
| 97 | - hotness"/> |
| 98 | + heat"/> |
| 99 | <require |
| 100 | permission="launchpad.View" |
| 101 | attributes=" |
| 102 | @@ -670,7 +670,7 @@ |
| 103 | permission="launchpad.Admin" |
| 104 | attributes=" |
| 105 | setCommentVisibility |
| 106 | - setHotness"/> |
| 107 | + setHeat"/> |
| 108 | </class> |
| 109 | <adapter |
| 110 | for="lp.bugs.interfaces.bug.IBug" |
| 111 | |
| 112 | === modified file 'lib/lp/bugs/doc/bug-heat.txt' |
| 113 | --- lib/lp/bugs/doc/bug-heat.txt 2010-01-11 11:16:48 +0000 |
| 114 | +++ lib/lp/bugs/doc/bug-heat.txt 2010-01-12 16:41:23 +0000 |
| 115 | @@ -1,217 +1,54 @@ |
| 116 | Calculating bug heat |
| 117 | ==================== |
| 118 | |
| 119 | -Launchpad bugs each have a 'hotness' rating. This is an indicator of how |
| 120 | +Launchpad bugs each have a 'heat' rating. This is an indicator of how |
| 121 | problematic a given bug is to the community and can be used to determine |
| 122 | which bugs should be tackled first. |
| 123 | |
| 124 | -A new bug will have a hotness of zero. |
| 125 | - |
| 126 | - >>> from lp.testing.factory import LaunchpadObjectFactory |
| 127 | - |
| 128 | - >>> factory = LaunchpadObjectFactory() |
| 129 | +A new bug will have a heat of zero. |
| 130 | + |
| 131 | >>> bug_owner = factory.makePerson() |
| 132 | >>> bug = factory.makeBug(owner=bug_owner) |
| 133 | - >>> bug.hotness |
| 134 | + >>> bug.heat |
| 135 | 0 |
| 136 | |
| 137 | -The bug's hotness can be set by calling its setHotness() method. |
| 138 | +The bug's heat can be set by calling its setHeat() method. |
| 139 | |
| 140 | - >>> bug.setHotness(42) |
| 141 | - >>> bug.hotness |
| 142 | + >>> bug.setHeat(42) |
| 143 | + >>> bug.heat |
| 144 | 42 |
| 145 | |
| 146 | -We'll reset it to zero for the purposes of testing. |
| 147 | - |
| 148 | - >>> bug.setHotness(0) |
| 149 | - |
| 150 | - |
| 151 | -The BugHeatCalculator class |
| 152 | ---------------------------- |
| 153 | - |
| 154 | -To calculate the heat for a particular bug, we use the BugHeatCalculator |
| 155 | -class. This class contains all the methods necessary to get the heat for |
| 156 | -a particular bug. |
| 157 | - |
| 158 | - >>> from lp.bugs.scripts.bugheat import BugHeatCalculator |
| 159 | - >>> bug_heat_calculator = BugHeatCalculator(bug) |
| 160 | - |
| 161 | -BugHeatCalculator has a number of private methods, each of which is |
| 162 | -called in turn to get the heat generated by one of the bug's attributes. |
| 163 | - |
| 164 | - |
| 165 | -BugHeatCalculator._getHeatFromDuplicates(): |
| 166 | ------------------------------------------- |
| 167 | - |
| 168 | -Each time a bug is duplicated its heat increases. The heat generated by |
| 169 | -duplicates is calculated using _getHeatFromDuplicates(). |
| 170 | -_getHeatFromDuplicates() will return 0 for a bug with no duplicates. |
| 171 | - |
| 172 | - >>> bug_heat_calculator._getHeatFromDuplicates() |
| 173 | - 0 |
| 174 | - |
| 175 | -The heat generated by duplicates is n*6, where n is the number of |
| 176 | -duplicates that the bug has. With one duplicate, |
| 177 | -_getHeatFromDuplicates() will return 6. |
| 178 | - |
| 179 | - >>> dupe = factory.makeBug() |
| 180 | - >>> dupe.duplicateof = bug |
| 181 | - |
| 182 | - >>> bug_heat_calculator._getHeatFromDuplicates() |
| 183 | - 6 |
| 184 | - |
| 185 | -With 5 duplicates, _getHeatFromDuplicates() will return 30. |
| 186 | - |
| 187 | - >>> for i in range(4): |
| 188 | - ... dupe = factory.makeBug() |
| 189 | - ... dupe.duplicateof = bug |
| 190 | - >>> transaction.commit() |
| 191 | - |
| 192 | - >>> bug_heat_calculator._getHeatFromDuplicates() |
| 193 | - 30 |
| 194 | - |
| 195 | - |
| 196 | -BugHeatCalculator._getHeatFromAffectedUsers() |
| 197 | ---------------------------------------------- |
| 198 | - |
| 199 | -The number of users affected by a bug also generates heat. The heat |
| 200 | -generated is returned by BugHeatCalculator._getHeatFromAffectedUsers() |
| 201 | -as n*4, where n is the number of affected users. |
| 202 | - |
| 203 | -The heat returned by _getHeatFromAffectedUsers() for our bug will be 4, |
| 204 | -since there is one affected user - the person who filed the bug. |
| 205 | - |
| 206 | - >>> bug_heat_calculator._getHeatFromAffectedUsers() |
| 207 | - 4 |
| 208 | - |
| 209 | -Adding some more affected users will increase the bug's heat. |
| 210 | - |
| 211 | - >>> for i in range(4): |
| 212 | - ... user = factory.makePerson() |
| 213 | - ... bug.markUserAffected(user) |
| 214 | - |
| 215 | - >>> bug_heat_calculator._getHeatFromAffectedUsers() |
| 216 | - 20 |
| 217 | - |
| 218 | - |
| 219 | -BugHeatCalculator._getHeatFromSubscribers() |
| 220 | -------------------------------------------- |
| 221 | - |
| 222 | -The number of subscribers a bug has generates heat, too. This heat is |
| 223 | -calculated by BugHeatCalculator._getHeatFromSubscribers() and is |
| 224 | -returned as n*2, where n is the number of subscribers the bug has. |
| 225 | - |
| 226 | -The number of subscribers includes those subscribed to duplicates of the |
| 227 | -bug. In the case of our bug, therefore, the initial subscriber-generated |
| 228 | -heat will be 12 because there are six subscribers: the bug owner and |
| 229 | -five subscribers-from-duplicates. |
| 230 | - |
| 231 | -Note that indirect subscribers (assignees, structural subscribers, etc.) |
| 232 | -aren't included when generating heat. |
| 233 | - |
| 234 | - >>> bug_heat_calculator._getHeatFromSubscribers() |
| 235 | - 12 |
| 236 | - |
| 237 | -Adding some subscribers will increase the bug's heat. |
| 238 | - |
| 239 | - >>> for i in range(3): |
| 240 | - ... user = factory.makePerson() |
| 241 | - ... bug.subscribe(user, user) |
| 242 | - <BugSubscription... |
| 243 | - <BugSubscription... |
| 244 | - <BugSubscription... |
| 245 | - |
| 246 | - >>> bug_heat_calculator._getHeatFromSubscribers() |
| 247 | - 18 |
| 248 | - |
| 249 | - |
| 250 | -BugHeatCalculator._getHeatFromPrivacy() |
| 251 | ---------------------------------------- |
| 252 | - |
| 253 | -BugHeatCalculator._getHeatFromPrivacy() returns the heat generated by |
| 254 | -the bug's private attribute. If the bug is public, this will be 0. |
| 255 | - |
| 256 | - >>> bug_heat_calculator._getHeatFromPrivacy() |
| 257 | - 0 |
| 258 | - |
| 259 | -However, if the bug is private, _getHeatFromPrivacy() will return 150. |
| 260 | - |
| 261 | - >>> bug.setPrivate(True, bug_owner) |
| 262 | - True |
| 263 | - |
| 264 | - >>> bug_heat_calculator._getHeatFromPrivacy() |
| 265 | - 150 |
| 266 | - |
| 267 | -Note that since the bug is now private its heat from subscribers has |
| 268 | -changed from 18 to 20. This is because the project owner, who previously |
| 269 | -had an indirect subscription to the bug, now as a direct subscription as |
| 270 | -a result of the behaviour of Bug.setPrivate() |
| 271 | - |
| 272 | - >>> bug_heat_calculator._getHeatFromSubscribers() |
| 273 | - 20 |
| 274 | - |
| 275 | - |
| 276 | -BugHeatCalculator._getHeatFromSecurity() |
| 277 | ----------------------------------------- |
| 278 | - |
| 279 | -BugHeatCalculator._getHeatFromSecurity() returns the heat generated by |
| 280 | -the bug's security_related attribute. If the bug is not security |
| 281 | -related, _getHeatFromSecurity() will return 0. |
| 282 | - |
| 283 | - >>> bug_heat_calculator._getHeatFromSecurity() |
| 284 | - 0 |
| 285 | - |
| 286 | -If, on the other hand, the bug is security_related, |
| 287 | -_getHeatFromSecurity() will return 250. |
| 288 | - |
| 289 | - >>> bug.security_related = True |
| 290 | - >>> bug_heat_calculator._getHeatFromSecurity() |
| 291 | - 250 |
| 292 | - |
| 293 | - |
| 294 | -BugHeatCalculator.getBugHeat() |
| 295 | ------------------------------- |
| 296 | - |
| 297 | -BugHeatCalculator.getBugHeat() returns the total heat for a given bug as |
| 298 | -the sum of the results of all _getHeatFrom*() methods. |
| 299 | - |
| 300 | -For our bug, the total heat will be 470: |
| 301 | - |
| 302 | - * From duplicates 30 |
| 303 | - * From subscribers 20 |
| 304 | - * From privacy 150 |
| 305 | - * From security 250 |
| 306 | - * From affected users 20 |
| 307 | - |
| 308 | - >>> bug_heat_calculator.getBugHeat() |
| 309 | - 470 |
| 310 | - |
| 311 | |
| 312 | The BugHeatUpdater class |
| 313 | --------------------------- |
| 314 | |
| 315 | In order to calculate bug heat we need to use the BugHeatUpdater |
| 316 | -class, which is designed precisely for that task. |
| 317 | +class, which is designed precisely for that task. It's part of the garbo |
| 318 | +module and runs as part of the garbo-daily cronjob. |
| 319 | |
| 320 | - >>> from lp.bugs.scripts.bugheat import BugHeatUpdater |
| 321 | + >>> from canonical.launchpad.scripts.garbo import BugHeatUpdater |
| 322 | >>> from canonical.launchpad.scripts import FakeLogger |
| 323 | |
| 324 | - >>> bug_heat_updater = BugHeatUpdater(transaction, logger=FakeLogger()) |
| 325 | - |
| 326 | -BugHeatUpdater's updateBugHeat() method will update the heat for all the |
| 327 | -bugs currently held in Launchpad. We'll commit our transaction now so |
| 328 | -that all the updates we've made are in the database. |
| 329 | - |
| 330 | - >>> transaction.commit() |
| 331 | - |
| 332 | - >>> bug_heat_updater.updateBugHeat() |
| 333 | - INFO Updating heat scores for all bugs |
| 334 | - INFO Updating 1 Bugs (starting id: ...) |
| 335 | + >>> update_bug_heat = BugHeatUpdater(FakeLogger()) |
| 336 | + |
| 337 | +BugHeatUpdater implements ITunableLoop and as such is callable. Calling |
| 338 | +it as a method will update the heat for all the bugs currently held in |
| 339 | +Launchpad. |
| 340 | + |
| 341 | +Before update_bug_heat is called, bug 1 will have no heat. |
| 342 | + |
| 343 | + >>> from zope.component import getUtility |
| 344 | + >>> from lp.bugs.interfaces.bug import IBugSet |
| 345 | + >>> bug_1 = getUtility(IBugSet).get(1) |
| 346 | + |
| 347 | + >>> bug_1.heat |
| 348 | + 0 |
| 349 | + |
| 350 | + >>> update_bug_heat(chunk_size=1) |
| 351 | + DEBUG Updating 1 Bugs (starting id: ...) |
| 352 | ... |
| 353 | - INFO Done updating heat for ... bugs |
| 354 | - |
| 355 | -The hotness of the bug that we created at the start of this document |
| 356 | -should now be 470. |
| 357 | - |
| 358 | - >>> bug.hotness |
| 359 | - 470 |
| 360 | + |
| 361 | +Bug 1's heat will now be greater than 0. |
| 362 | + |
| 363 | + >>> bug_1.heat > 0 |
| 364 | + True |
| 365 | |
| 366 | === modified file 'lib/lp/bugs/interfaces/bug.py' |
| 367 | --- lib/lp/bugs/interfaces/bug.py 2010-01-11 11:16:48 +0000 |
| 368 | +++ lib/lp/bugs/interfaces/bug.py 2010-01-11 12:00:48 +0000 |
| 369 | @@ -296,8 +296,8 @@ |
| 370 | value_type=Reference(schema=IPerson), |
| 371 | readonly=True)) |
| 372 | |
| 373 | - hotness = Int( |
| 374 | - title=_("The 'hotness' of the bug"), |
| 375 | + heat = Int( |
| 376 | + title=_("The 'heat' of the bug"), |
| 377 | required=False, readonly=True) |
| 378 | |
| 379 | # Adding related BugMessages provides a hook for getting at |
| 380 | |
| 381 | === modified file 'lib/lp/bugs/model/bug.py' |
| 382 | --- lib/lp/bugs/model/bug.py 2010-01-08 14:48:25 +0000 |
| 383 | +++ lib/lp/bugs/model/bug.py 2010-01-11 12:00:48 +0000 |
| 384 | @@ -254,7 +254,10 @@ |
| 385 | message_count = IntCol(notNull=True, default=0) |
| 386 | users_affected_count = IntCol(notNull=True, default=0) |
| 387 | users_unaffected_count = IntCol(notNull=True, default=0) |
| 388 | - hotness = IntCol(notNull=True, default=0) |
| 389 | + |
| 390 | + # This is called 'hotness' in the database but the canonical name in |
| 391 | + # the code is 'heat', so we use that name here. |
| 392 | + heat = IntCol(dbName='hotness', notNull=True, default=0) |
| 393 | |
| 394 | @property |
| 395 | def comment_count(self): |
| 396 | @@ -1426,9 +1429,9 @@ |
| 397 | |
| 398 | return not subscriptions_from_dupes.is_empty() |
| 399 | |
| 400 | - def setHotness(self, hotness): |
| 401 | + def setHeat(self, heat): |
| 402 | """See `IBug`.""" |
| 403 | - self.hotness = hotness |
| 404 | + self.heat = heat |
| 405 | |
| 406 | |
| 407 | class BugSet: |
| 408 | |
| 409 | === modified file 'lib/lp/bugs/scripts/bugheat.py' |
| 410 | --- lib/lp/bugs/scripts/bugheat.py 2010-01-11 11:24:40 +0000 |
| 411 | +++ lib/lp/bugs/scripts/bugheat.py 2010-01-12 16:41:23 +0000 |
| 412 | @@ -11,9 +11,17 @@ |
| 413 | from zope.interface import implements |
| 414 | |
| 415 | from canonical.launchpad.interfaces.looptuner import ITunableLoop |
| 416 | -from canonical.launchpad.utilities.looptuner import LoopTuner |
| 417 | - |
| 418 | -from lp.bugs.interfaces.bug import IBugSet |
| 419 | +from canonical.launchpad.utilities.looptuner import DBLoopTuner |
| 420 | + |
| 421 | + |
| 422 | +class BugHeatConstants: |
| 423 | + |
| 424 | + PRIVACY = 150 |
| 425 | + SECURITY = 250 |
| 426 | + DUPLICATE = 6 |
| 427 | + AFFECTED_USER = 4 |
| 428 | + SUBSCRIBER = 2 |
| 429 | + |
| 430 | |
| 431 | class BugHeatCalculator: |
| 432 | """A class to calculate the heat for a bug.""" |
| 433 | @@ -24,130 +32,44 @@ |
| 434 | def _getHeatFromPrivacy(self): |
| 435 | """Return the heat generated by the bug's `private` attribute.""" |
| 436 | if self.bug.private: |
| 437 | - return 150 |
| 438 | + return BugHeatConstants.PRIVACY |
| 439 | else: |
| 440 | return 0 |
| 441 | |
| 442 | def _getHeatFromSecurity(self): |
| 443 | """Return the heat generated if the bug is security related.""" |
| 444 | if self.bug.security_related: |
| 445 | - return 250 |
| 446 | + return BugHeatConstants.SECURITY |
| 447 | else: |
| 448 | return 0 |
| 449 | |
| 450 | def _getHeatFromDuplicates(self): |
| 451 | """Return the heat generated by the bug's duplicates.""" |
| 452 | - return self.bug.duplicates.count() * 6 |
| 453 | + return self.bug.duplicates.count() * BugHeatConstants.DUPLICATE |
| 454 | |
| 455 | def _getHeatFromAffectedUsers(self): |
| 456 | """Return the heat generated by the bug's affected users.""" |
| 457 | - return self.bug.users_affected.count() * 4 |
| 458 | + return ( |
| 459 | + self.bug.users_affected.count() * BugHeatConstants.AFFECTED_USER) |
| 460 | |
| 461 | def _getHeatFromSubscribers(self): |
| 462 | """Return the heat generated by the bug's subscribers.""" |
| 463 | direct_subscribers = self.bug.getDirectSubscribers() |
| 464 | subscribers_from_dupes = self.bug.getSubscribersFromDuplicates() |
| 465 | |
| 466 | - return ( |
| 467 | - (len(direct_subscribers) + len(subscribers_from_dupes)) * 2) |
| 468 | + subscriber_count = ( |
| 469 | + len(direct_subscribers) + len(subscribers_from_dupes)) |
| 470 | + return subscriber_count * BugHeatConstants.SUBSCRIBER |
| 471 | |
| 472 | def getBugHeat(self): |
| 473 | """Return the total heat for the current bug.""" |
| 474 | - heat_counts = [ |
| 475 | + total_heat = sum([ |
| 476 | self._getHeatFromAffectedUsers(), |
| 477 | self._getHeatFromDuplicates(), |
| 478 | self._getHeatFromPrivacy(), |
| 479 | self._getHeatFromSecurity(), |
| 480 | self._getHeatFromSubscribers(), |
| 481 | - ] |
| 482 | - |
| 483 | - total_heat = 0 |
| 484 | - for count in heat_counts: |
| 485 | - total_heat += count |
| 486 | + ]) |
| 487 | |
| 488 | return total_heat |
| 489 | |
| 490 | - |
| 491 | -class BugHeatTunableLoop: |
| 492 | - """An `ITunableLoop` implementation for bug heat calculations.""" |
| 493 | - |
| 494 | - implements(ITunableLoop) |
| 495 | - |
| 496 | - total_updated = 0 |
| 497 | - |
| 498 | - def __init__(self, transaction, logger, offset=0): |
| 499 | - self.transaction = transaction |
| 500 | - self.logger = logger |
| 501 | - self.offset = offset |
| 502 | - self.total_updated = 0 |
| 503 | - |
| 504 | - def isDone(self): |
| 505 | - """See `ITunableLoop`.""" |
| 506 | - # When the main loop has no more Bugs to process it sets |
| 507 | - # offset to None. Until then, it always has a numerical |
| 508 | - # value. |
| 509 | - return self.offset is None |
| 510 | - |
| 511 | - def __call__(self, chunk_size): |
| 512 | - """Retrieve a batch of Bugs and update their heat. |
| 513 | - |
| 514 | - See `ITunableLoop`. |
| 515 | - """ |
| 516 | - # XXX 2010-01-08 gmb bug=198767: |
| 517 | - # We cast chunk_size to an integer to ensure that we're not |
| 518 | - # trying to slice using floats or anything similarly |
| 519 | - # foolish. We shouldn't have to do this. |
| 520 | - chunk_size = int(chunk_size) |
| 521 | - |
| 522 | - start = self.offset |
| 523 | - end = self.offset + chunk_size |
| 524 | - |
| 525 | - self.transaction.begin() |
| 526 | - # XXX 2010-01-08 gmb bug=505850: |
| 527 | - # This method call should be taken out and shot as soon as |
| 528 | - # we have a proper permissions system for scripts. |
| 529 | - bugs = list(getUtility(IBugSet).dangerousGetAllBugs()[start:end]) |
| 530 | - |
| 531 | - self.offset = None |
| 532 | - if bugs: |
| 533 | - starting_id = bugs[0].id |
| 534 | - self.logger.info("Updating %i Bugs (starting id: %i)" % |
| 535 | - (len(bugs), starting_id)) |
| 536 | - |
| 537 | - for bug in bugs: |
| 538 | - # We set the starting point of the next batch to the Bug |
| 539 | - # id after the one we're looking at now. If there aren't any |
| 540 | - # bugs this loop will run for 0 iterations and start_id |
| 541 | - # will remain set to None. |
| 542 | - start += 1 |
| 543 | - self.offset = start |
| 544 | - self.logger.debug("Updating heat for bug %s" % bug.id) |
| 545 | - bug_heat_calculator = BugHeatCalculator(bug) |
| 546 | - heat = bug_heat_calculator.getBugHeat() |
| 547 | - bug.setHotness(heat) |
| 548 | - self.total_updated += 1 |
| 549 | - |
| 550 | - self.transaction.commit() |
| 551 | - |
| 552 | - |
| 553 | -class BugHeatUpdater: |
| 554 | - """Takes responsibility for updating bug heat.""" |
| 555 | - |
| 556 | - def __init__(self, transaction, logger): |
| 557 | - self.transaction = transaction |
| 558 | - self.logger = logger |
| 559 | - |
| 560 | - def updateBugHeat(self): |
| 561 | - """Update the heat scores for all bugs.""" |
| 562 | - self.logger.info("Updating heat scores for all bugs") |
| 563 | - |
| 564 | - loop = BugHeatTunableLoop(self.transaction, self.logger) |
| 565 | - |
| 566 | - # We use the LoopTuner class to try and get an ideal number of |
| 567 | - # bugs updated for each iteration of the loop (see the LoopTuner |
| 568 | - # documentation for more details). |
| 569 | - loop_tuner = LoopTuner(loop, 2) |
| 570 | - loop_tuner.run() |
| 571 | - |
| 572 | - self.logger.info( |
| 573 | - "Done updating heat for %s bugs" % loop.total_updated) |
| 574 | |
| 575 | === added file 'lib/lp/bugs/scripts/tests/test_bugheat.py' |
| 576 | --- lib/lp/bugs/scripts/tests/test_bugheat.py 1970-01-01 00:00:00 +0000 |
| 577 | +++ lib/lp/bugs/scripts/tests/test_bugheat.py 2010-01-12 16:41:23 +0000 |
| 578 | @@ -0,0 +1,183 @@ |
| 579 | + |
| 580 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
| 581 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
| 582 | + |
| 583 | +"""Module docstring goes here.""" |
| 584 | + |
| 585 | +__metaclass__ = type |
| 586 | + |
| 587 | +import unittest |
| 588 | + |
| 589 | +from canonical.testing import LaunchpadZopelessLayer |
| 590 | + |
| 591 | +from lp.bugs.scripts.bugheat import BugHeatCalculator, BugHeatConstants |
| 592 | +from lp.testing import TestCaseWithFactory |
| 593 | + |
| 594 | + |
| 595 | +class TestBugHeatCalculator(TestCaseWithFactory): |
| 596 | + """Tests for the BugHeatCalculator class.""" |
| 597 | + |
| 598 | + layer = LaunchpadZopelessLayer |
| 599 | + |
| 600 | + def setUp(self): |
| 601 | + super(TestBugHeatCalculator, self).setUp() |
| 602 | + self.bug = self.factory.makeBug() |
| 603 | + self.calculator = BugHeatCalculator(self.bug) |
| 604 | + |
| 605 | + def test__getHeatFromDuplicates(self): |
| 606 | + # BugHeatCalculator._getHeatFromDuplicates() returns the bug |
| 607 | + # heat generated by duplicates of a bug. |
| 608 | + # By default, the bug has no heat from dupes |
| 609 | + self.assertEqual(0, self.calculator._getHeatFromDuplicates()) |
| 610 | + |
| 611 | + # If adding duplicates, the heat generated by them will be n * |
| 612 | + # BugHeatConstants.DUPLICATE, where n is the number of |
| 613 | + # duplicates. |
| 614 | + for i in range(5): |
| 615 | + dupe = self.factory.makeBug() |
| 616 | + dupe.duplicateof = self.bug |
| 617 | + |
| 618 | + expected_heat = BugHeatConstants.DUPLICATE * 5 |
| 619 | + actual_heat = self.calculator._getHeatFromDuplicates() |
| 620 | + self.assertEqual( |
| 621 | + expected_heat, actual_heat, |
| 622 | + "Heat from duplicates does not match expected heat. " |
| 623 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 624 | + |
| 625 | + def test__getHeatFromAffectedUsers(self): |
| 626 | + # BugHeatCalculator._getHeatFromAffectedUsers() returns the bug |
| 627 | + # heat generated by users affected by the bug. |
| 628 | + # By default, the heat will be BugHeatConstants.AFFECTED_USER, since |
| 629 | + # there will be one affected user (the user who filed the bug). |
| 630 | + self.assertEqual( |
| 631 | + BugHeatConstants.AFFECTED_USER, |
| 632 | + self.calculator._getHeatFromAffectedUsers()) |
| 633 | + |
| 634 | + # As the number of affected users increases, the heat generated |
| 635 | + # will be n * BugHeatConstants.AFFECTED_USER, where n is the number |
| 636 | + # of affected users. |
| 637 | + for i in range(5): |
| 638 | + person = self.factory.makePerson() |
| 639 | + self.bug.markUserAffected(person) |
| 640 | + |
| 641 | + expected_heat = BugHeatConstants.AFFECTED_USER * 6 |
| 642 | + actual_heat = self.calculator._getHeatFromAffectedUsers() |
| 643 | + self.assertEqual( |
| 644 | + expected_heat, actual_heat, |
| 645 | + "Heat from affected users does not match expected heat. " |
| 646 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 647 | + |
| 648 | + def test__getHeatFromSubscribers(self): |
| 649 | + # BugHeatCalculator._getHeatFromSubscribers() returns the bug |
| 650 | + # heat generated by users subscribed tothe bug. |
| 651 | + # By default, the heat will be BugHeatConstants.SUBSCRIBER, |
| 652 | + # since there will be one direct subscriber (the user who filed |
| 653 | + # the bug). |
| 654 | + self.assertEqual( |
| 655 | + BugHeatConstants.SUBSCRIBER, |
| 656 | + self.calculator._getHeatFromSubscribers()) |
| 657 | + |
| 658 | + # As the number of subscribers increases, the heat generated |
| 659 | + # will be n * BugHeatConstants.SUBSCRIBER, where n is the number |
| 660 | + # of subscribers. |
| 661 | + for i in range(5): |
| 662 | + person = self.factory.makePerson() |
| 663 | + self.bug.subscribe(person, person) |
| 664 | + |
| 665 | + expected_heat = BugHeatConstants.SUBSCRIBER * 6 |
| 666 | + actual_heat = self.calculator._getHeatFromSubscribers() |
| 667 | + self.assertEqual( |
| 668 | + expected_heat, actual_heat, |
| 669 | + "Heat from subscribers does not match expected heat. " |
| 670 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 671 | + |
| 672 | + # Subscribers from duplicates are included in the heat returned |
| 673 | + # by _getHeatFromSubscribers() |
| 674 | + dupe = self.factory.makeBug() |
| 675 | + dupe.duplicateof = self.bug |
| 676 | + expected_heat = BugHeatConstants.SUBSCRIBER * 7 |
| 677 | + actual_heat = self.calculator._getHeatFromSubscribers() |
| 678 | + self.assertEqual( |
| 679 | + expected_heat, actual_heat, |
| 680 | + "Heat from subscribers (including duplicate-subscribers) " |
| 681 | + "does not match expected heat. Expected %s, got %s" % |
| 682 | + (expected_heat, actual_heat)) |
| 683 | + |
| 684 | + # Seting the bug to private will increase its heat from |
| 685 | + # subscribers by 1 * BugHeatConstants.SUBSCRIBER, as the project |
| 686 | + # owner will now be directly subscribed to it. |
| 687 | + self.bug.setPrivate(True, self.bug.owner) |
| 688 | + expected_heat = BugHeatConstants.SUBSCRIBER * 8 |
| 689 | + actual_heat = self.calculator._getHeatFromSubscribers() |
| 690 | + self.assertEqual( |
| 691 | + expected_heat, actual_heat, |
| 692 | + "Heat from subscribers to private bug does not match expected " |
| 693 | + "heat. Expected %s, got %s" % (expected_heat, actual_heat)) |
| 694 | + |
| 695 | + def test__getHeatFromPrivacy(self): |
| 696 | + # BugHeatCalculator._getHeatFromPrivacy() returns the heat |
| 697 | + # generated by the bug's private attribute. If the bug is |
| 698 | + # public, this will be 0. |
| 699 | + self.assertEqual(0, self.calculator._getHeatFromPrivacy()) |
| 700 | + |
| 701 | + # However, if the bug is private, _getHeatFromPrivacy() will |
| 702 | + # return BugHeatConstants.PRIVACY. |
| 703 | + self.bug.setPrivate(True, self.bug.owner) |
| 704 | + self.assertEqual( |
| 705 | + BugHeatConstants.PRIVACY, self.calculator._getHeatFromPrivacy()) |
| 706 | + |
| 707 | + def test__getHeatFromSecurity(self): |
| 708 | + # BugHeatCalculator._getHeatFromSecurity() returns the heat |
| 709 | + # generated by the bug's security_related attribute. If the bug |
| 710 | + # is not security related, _getHeatFromSecurity() will return 0. |
| 711 | + self.assertEqual(0, self.calculator._getHeatFromPrivacy()) |
| 712 | + |
| 713 | + |
| 714 | + # If, on the other hand, the bug is security_related, |
| 715 | + # _getHeatFromSecurity() will return BugHeatConstants.SECURITY |
| 716 | + self.bug.security_related = True |
| 717 | + self.assertEqual( |
| 718 | + BugHeatConstants.SECURITY, self.calculator._getHeatFromSecurity()) |
| 719 | + |
| 720 | + def test_getBugHeat(self): |
| 721 | + # BugHeatCalculator.getBugHeat() returns the total heat for a |
| 722 | + # given bug as the sum of the results of all _getHeatFrom*() |
| 723 | + # methods. |
| 724 | + # By default this will be (BugHeatConstants.AFFECTED_USER + |
| 725 | + # BugHeatConstants.SUBSCRIBER) since there will be one |
| 726 | + # subscriber and one affected user only. |
| 727 | + expected_heat = ( |
| 728 | + BugHeatConstants.AFFECTED_USER + BugHeatConstants.SUBSCRIBER) |
| 729 | + actual_heat = self.calculator.getBugHeat() |
| 730 | + self.assertEqual( |
| 731 | + expected_heat, actual_heat, |
| 732 | + "Expected bug heat did not match actual bug heat. " |
| 733 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 734 | + |
| 735 | + # Adding a duplicate and making the bug private and security |
| 736 | + # related will increase its heat. |
| 737 | + dupe = self.factory.makeBug() |
| 738 | + dupe.duplicateof = self.bug |
| 739 | + self.bug.setPrivate(True, self.bug.owner) |
| 740 | + self.bug.security_related = True |
| 741 | + |
| 742 | + expected_heat += ( |
| 743 | + BugHeatConstants.DUPLICATE + |
| 744 | + BugHeatConstants.PRIVACY + |
| 745 | + BugHeatConstants.SECURITY |
| 746 | + ) |
| 747 | + |
| 748 | + # Adding the duplicate and making the bug private means it gets |
| 749 | + # two new subscribers, the project owner and the duplicate's |
| 750 | + # direct subscriber. |
| 751 | + expected_heat += BugHeatConstants.SUBSCRIBER * 2 |
| 752 | + actual_heat = self.calculator.getBugHeat() |
| 753 | + self.assertEqual( |
| 754 | + expected_heat, actual_heat, |
| 755 | + "Expected bug heat did not match actual bug heat. " |
| 756 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 757 | + |
| 758 | + |
| 759 | +def test_suite(): |
| 760 | + return unittest.TestLoader().loadTestsFromName(__name__) |
| 761 | + |
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
| 1 | === modified file 'database/replication/Makefile' |
| 2 | --- database/replication/Makefile 2009-12-14 13:00:03 +0000 |
| 3 | +++ database/replication/Makefile 2010-01-13 10:10:29 +0000 |
| 4 | @@ -100,7 +100,7 @@ |
| 5 | # up when roles don't exist in this cluster, and we rebuild it later |
| 6 | # with security.py anyway. |
| 7 | pg_restore --dbname=lpmain_staging_new \ |
| 8 | - --no-acl --no-owner --exit-on-error ${STAGING_DUMP} |
| 9 | + --no-owner --exit-on-error ${STAGING_DUMP} |
| 10 | psql -q -d lpmain_staging_new -f authdb_drop.sql |
| 11 | psql -q -d lpmain_staging_new -f authdb_create.sql \ |
| 12 | 2>&1 | grep -v _sl || true |
| 13 | @@ -187,7 +187,9 @@ |
| 14 | @echo Running fti.py `date` |
| 15 | ${SHHH} ../schema/fti.py |
| 16 | @echo Running security.py `date` |
| 17 | + ./slon_ctl.py stop # security.py can deadlock with slony |
| 18 | ${SHHH} ../schema/security.py --cluster -U slony |
| 19 | + ./slon_ctl.py --lag="0 seconds" start |
| 20 | # Migrate tables to the authdb replication set, creating the set |
| 21 | # and subscribing nodes to it as necessary. |
| 22 | ./populate_auth_replication_set.py -U slony |
| 23 | |
| 24 | === modified file 'database/replication/helpers.py' |
| 25 | --- database/replication/helpers.py 2009-11-30 11:35:04 +0000 |
| 26 | +++ database/replication/helpers.py 2010-01-13 10:10:29 +0000 |
| 27 | @@ -71,8 +71,27 @@ |
| 28 | 'public.lp_person', |
| 29 | 'public.lp_personlocation', |
| 30 | 'public.lp_teamparticipation', |
| 31 | + # Ubuntu SSO database. These tables where created manually by ISD |
| 32 | + # and the Launchpad scripts should not mess with them. Eventually |
| 33 | + # these tables will be in a totally separate database. |
| 34 | + 'public.auth_permission', |
| 35 | + 'public.auth_group', |
| 36 | + 'public.auth_user', |
| 37 | + 'public.auth_message', |
| 38 | + 'public.django_content_type', |
| 39 | + 'public.auth_permission', |
| 40 | + 'public.django_session', |
| 41 | + 'public.django_site', |
| 42 | + 'public.django_admin_log', |
| 43 | + 'public.ssoopenidrpconfig', |
| 44 | + 'public.auth_group_permissions', |
| 45 | + 'public.auth_user_groups', |
| 46 | + 'public.auth_user_user_permissions', |
| 47 | ]) |
| 48 | |
| 49 | +# Calculate IGNORED_SEQUENCES |
| 50 | +IGNORED_SEQUENCES = set('%s_id_seq' % table for table in IGNORED_TABLES) |
| 51 | + |
| 52 | |
| 53 | def slony_installed(con): |
| 54 | """Return True if the connected database is part of a Launchpad Slony-I |
| 55 | @@ -447,7 +466,7 @@ |
| 56 | |
| 57 | return ( |
| 58 | all_tables - replicated_tables - IGNORED_TABLES, |
| 59 | - all_sequences - replicated_sequences) |
| 60 | + all_sequences - replicated_sequences - IGNORED_SEQUENCES) |
| 61 | |
| 62 | |
| 63 | class ReplicationConfigError(Exception): |
| 64 | |
| 65 | === modified file 'database/replication/initialize.py' |
| 66 | --- database/replication/initialize.py 2009-10-17 14:06:03 +0000 |
| 67 | +++ database/replication/initialize.py 2010-01-13 10:10:29 +0000 |
| 68 | @@ -224,7 +224,8 @@ |
| 69 | fails += 1 |
| 70 | for sequence in all_sequences_in_schema(cur, 'public'): |
| 71 | times_seen = 0 |
| 72 | - for sequence_set in [authdb_sequences, lpmain_sequences]: |
| 73 | + for sequence_set in [ |
| 74 | + authdb_sequences, lpmain_sequences, helpers.IGNORED_SEQUENCES]: |
| 75 | if sequence in sequence_set: |
| 76 | times_seen += 1 |
| 77 | if times_seen == 0: |
| 78 | |
| 79 | === added file 'database/replication/sync.py' |
| 80 | --- database/replication/sync.py 1970-01-01 00:00:00 +0000 |
| 81 | +++ database/replication/sync.py 2010-01-13 10:10:29 +0000 |
| 82 | @@ -0,0 +1,26 @@ |
| 83 | +#!/usr/bin/python2.5 |
| 84 | +# |
| 85 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
| 86 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
| 87 | + |
| 88 | +"""Block until the replication cluster synchronizes.""" |
| 89 | + |
| 90 | +__metaclass__ = type |
| 91 | +__all__ = [] |
| 92 | + |
| 93 | +import _pythonpath |
| 94 | + |
| 95 | +from optparse import OptionParser |
| 96 | + |
| 97 | +from canonical.launchpad.scripts import logger_options, db_options |
| 98 | +from replication.helpers import sync |
| 99 | + |
| 100 | +if __name__ == '__main__': |
| 101 | + parser = OptionParser() |
| 102 | + parser.add_option( |
| 103 | + "-t", "--timeout", dest="timeout", metavar="SECS", type="int", |
| 104 | + help="Abort if no sync after SECS seconds.", default=0) |
| 105 | + logger_options(parser) |
| 106 | + db_options(parser) |
| 107 | + options, args = parser.parse_args() |
| 108 | + sync(options.timeout) |
| 109 | |
| 110 | === modified file 'database/sampledata/current-dev.sql' |
| 111 | --- database/sampledata/current-dev.sql 2009-12-14 13:49:03 +0000 |
| 112 | +++ database/sampledata/current-dev.sql 2010-01-13 10:10:29 +0000 |
| 113 | @@ -1715,8 +1715,8 @@ |
| 114 | |
| 115 | ALTER TABLE buildqueue DISABLE TRIGGER ALL; |
| 116 | |
| 117 | -INSERT 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'); |
| 118 | -INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration) VALUES (2, NULL, NULL, 10, false, 2, 1, '00:01:00'); |
| 119 | +INSERT 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); |
| 120 | +INSERT 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); |
| 121 | |
| 122 | |
| 123 | ALTER TABLE buildqueue ENABLE TRIGGER ALL; |
| 124 | |
| 125 | === modified file 'database/sampledata/current.sql' |
| 126 | --- database/sampledata/current.sql 2009-12-14 13:49:03 +0000 |
| 127 | +++ database/sampledata/current.sql 2010-01-13 10:10:29 +0000 |
| 128 | @@ -1697,8 +1697,8 @@ |
| 129 | |
| 130 | ALTER TABLE buildqueue DISABLE TRIGGER ALL; |
| 131 | |
| 132 | -INSERT 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'); |
| 133 | -INSERT INTO buildqueue (id, builder, logtail, lastscore, manual, job, job_type, estimated_duration) VALUES (2, NULL, NULL, 10, false, 2, 1, '00:01:00'); |
| 134 | +INSERT 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); |
| 135 | +INSERT 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); |
| 136 | |
| 137 | |
| 138 | ALTER TABLE buildqueue ENABLE TRIGGER ALL; |
| 139 | |
| 140 | === modified file 'database/schema/README' |
| 141 | --- database/schema/README 2006-10-27 01:02:38 +0000 |
| 142 | +++ database/schema/README 2010-01-13 10:10:29 +0000 |
| 143 | @@ -1,131 +1,1 @@ |
| 144 | -= How to make database schema changes = |
| 145 | - |
| 146 | -Important: This documentation is mirrored on https://launchpad.canonical.com/DatabaseSchemaChanges |
| 147 | - |
| 148 | -So, make sure to copy any changes you make here to the wiki page! |
| 149 | - |
| 150 | -== Making schema changes == |
| 151 | - |
| 152 | - 1. Make an SQL file in `database/schema/pending/` containing the changes you want, excluding any changes to default values. |
| 153 | - 2. Run `make schema` to get a pristine database of sampledata. |
| 154 | - 3. Run the SQL on the database (`psql launchpad_dev -f your-patch.sql`) to ensure it works. |
| 155 | - 4. In `database/schema/`, `make newsampledata`. |
| 156 | - 5. Replace `current.sql` with `newsampledata.sql`. |
| 157 | - 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. |
| 158 | - At this point `make schema` should work again. |
| 159 | - 7. Make any necessary changes to `lib/canonical/lp/dbschema.py`, `database/schema/fti.py`, and to the relevant `lib/canonical/launchpad/database/` classes. |
| 160 | - 8. Make any changes to the SQL patch to reflect new default values. |
| 161 | - |
| 162 | -== Proposing database schema changes == |
| 163 | - |
| 164 | -For any tables and fields that you change with an SQL script via Stuart |
| 165 | -(stub on IRC), please make sure you include comments. |
| 166 | - |
| 167 | -The process now looks like this: |
| 168 | - |
| 169 | - 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. |
| 170 | - 2. Work on the patch in a branch as documented above. |
| 171 | - 3. Add the details of your branch to StuartBishop's review queue on PendingReviews. |
| 172 | - 4. Work on it in revision control till StuartBishop is happy. He will give you an official patch number |
| 173 | - 5. Rename your patch to match the official patch number |
| 174 | - 6. Once code is also ready and reviewed, commit as normal. |
| 175 | - |
| 176 | -== Resolving schema conflicts == |
| 177 | - |
| 178 | -Resolving conflicts in `current.sql` manually is usually more trouble than it's worth. Instead, first resolve any conflicts in `comments.sql`, then: {{{ |
| 179 | - |
| 180 | - cd database/schema/ |
| 181 | - mv {patch-in-question}-0.sql comments.sql pending/ |
| 182 | - cp {parent branch, e.g. rocketfuel}/database/schema/comments.sql ./ |
| 183 | - cp ../sampledata/current.sql.OTHER ../sampledata/current.sql |
| 184 | - make |
| 185 | - psql launchpad_dev -f pending/patch-xx-99-0.sql |
| 186 | - make newsampledata |
| 187 | - mv ../sampledata/newsampledata.sql ../sampledata/current.sql |
| 188 | - mv pending/{patch-in-question}-0.sql pending/comments.sql ./ |
| 189 | - make # Just to make sure everything works |
| 190 | - cd ../.. |
| 191 | - bzr resolve database/sampledata/current.sql |
| 192 | - |
| 193 | -}}} |
| 194 | - |
| 195 | -= Production Database Upgrades = |
| 196 | - |
| 197 | -First get a copy of the Launchpad source built and ready on emperor, readable |
| 198 | -by the postgres user. |
| 199 | - |
| 200 | -Then, before you do anything else, inform #canonical, #launchpad and |
| 201 | -#ubuntu-devel that Launchpad and the Wiki authentication systems will be |
| 202 | -offline for 30 minutes (or longer if there is data migration to do). |
| 203 | - |
| 204 | -Stop PostgreSQL: |
| 205 | - |
| 206 | - % pg_ctl -m fast stop |
| 207 | - |
| 208 | -Start PostgreSQL without external connections |
| 209 | - |
| 210 | - % pg_ctl start -o '--tcpip-socket=false' -o '--ssl=false' \ |
| 211 | - -l /var/log/postgresql/postgres.log |
| 212 | - |
| 213 | -As user postgres, run the upgrade.py, fti.py and security.py scripts. |
| 214 | -fti.py can be skipped if you are sure no changes need to be made to the |
| 215 | -full text indexes (ie. fti.py has not been modified and no patches affect |
| 216 | -the tables being indexed). This process should work without issues, as any |
| 217 | -issues (such as DB patches not working on production data) will have been |
| 218 | -picked up from the daily updates to the staging environment. Do not run |
| 219 | -update.py using the --partial option to ensure that changes will be rolled |
| 220 | -back on failure. |
| 221 | - |
| 222 | - % cd dists/launchpad/database/schema |
| 223 | - % env LPCONFIG=production \ |
| 224 | - python upgrade.py -d launchpad_prod -U postgres -H '' |
| 225 | - % env LPCONFIG=production \ |
| 226 | - python fti.py -d launchpad_prod -U postgres -H '' |
| 227 | - % env LPCONFIG=production \ |
| 228 | - python security.py -d launchpad_prod -U postgres -H '' |
| 229 | - |
| 230 | -Restart PostgreSQL with external connections |
| 231 | - |
| 232 | - % pg_ctl -m fast stop |
| 233 | - % pg_ctl start -l /var/log/postgresql/postgres.log |
| 234 | - |
| 235 | -At this point, services should be restarted that don't automatically |
| 236 | -reconnect, such as the Launchpad web application servers and the Librarian. |
| 237 | - |
| 238 | -== Create a new development baseline == |
| 239 | - |
| 240 | -After a production update, you should occasionally copy the live schema |
| 241 | -back into the development tree. This ensures that any differences that have |
| 242 | -crept in between the development database and reality are fixed. |
| 243 | -The new baseline dump (launchpad-XX-0-0.sql in this directory) can |
| 244 | -be generated on production using the following:: |
| 245 | - |
| 246 | - pg_dump -Fc --schema-only --no-owner --no-acl --schema=public \ |
| 247 | - launchpad_prod > launchpad.dump |
| 248 | - pg_restore -l launchpad.dump | \ |
| 249 | - grep -v PROCEDURAL | grep -v COMMENT | \ |
| 250 | - grep -v FUNCTION | grep -v VIEW > launchpad.list |
| 251 | - pg_restore -l launchpad.dump | grep VIEW >> launchpad.list |
| 252 | - echo "-- Generated `date`" > launchpad.sql |
| 253 | - echo 'SET client_min_messages=ERROR;' >> launchpad.sql |
| 254 | - pg_restore --no-owner --no-acl -L launchpad.list launchpad.dump | \ |
| 255 | - grep -v '^--' >> launchpad.sql |
| 256 | - |
| 257 | -Move all the existing patches and the old baseline to the archive directory. |
| 258 | -Add the new baseline using the next revision number (should be in sync |
| 259 | -with the production release version). Create a patch-XX-0-0.sql patch |
| 260 | -to populate the LaunchpadDatabaseRevision table with the correct value |
| 261 | -so the tests pass. |
| 262 | - |
| 263 | - |
| 264 | -= Notes = |
| 265 | - |
| 266 | -There is a Makefile in launchpad/database/schema that will |
| 267 | -create the launchpad_test database (if it doesn't already exist), |
| 268 | -drop all your tables and create the current schema with all patches |
| 269 | -applied. |
| 270 | - |
| 271 | -If you want to check anything into the launchpad/database/schema |
| 272 | -directory, please do not give it a .sql extension or you will might |
| 273 | -confuse the simple Makefile. |
| 274 | - |
| 275 | +See https://dev.launchpad.net/PolicyAndProcess/DatabaseSchemaChangesProcess |
| 276 | |
| 277 | === modified file 'database/schema/comments.sql' |
| 278 | --- database/schema/comments.sql 2009-12-01 13:45:58 +0000 |
| 279 | +++ database/schema/comments.sql 2010-01-13 10:10:29 +0000 |
| 280 | @@ -1537,6 +1537,8 @@ |
| 281 | COMMENT ON COLUMN BuildQueue.job IS 'Foreign key to the `Job` table row with the generic job data.'; |
| 282 | COMMENT 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.'; |
| 283 | COMMENT ON COLUMN BuildQueue.estimated_duration IS 'Estimated job duration, based on previous running times of comparable jobs.'; |
| 284 | +COMMENT ON COLUMN BuildQueue.processor IS 'The processor required by the associated build farm job.'; |
| 285 | +COMMENT ON COLUMN BuildQueue.virtualized IS 'The virtualization setting required by the associated build farm job.'; |
| 286 | |
| 287 | -- Mirrors |
| 288 | |
| 289 | |
| 290 | === added file 'database/schema/patch-2207-19-1.sql' |
| 291 | --- database/schema/patch-2207-19-1.sql 1970-01-01 00:00:00 +0000 |
| 292 | +++ database/schema/patch-2207-19-1.sql 2010-01-13 10:10:29 +0000 |
| 293 | @@ -0,0 +1,35 @@ |
| 294 | +SET client_min_messages=ERROR; |
| 295 | + |
| 296 | +CREATE INDEX bugtask__bugwatch__idx |
| 297 | +ON BugTask(bugwatch) WHERE bugwatch IS NOT NULL; |
| 298 | + |
| 299 | +CREATE INDEX translationimportqueueentry__productseries__idx |
| 300 | +ON TranslationImportQueueEntry(productseries) |
| 301 | +WHERE productseries IS NOT NULL; |
| 302 | + |
| 303 | +CREATE INDEX translationimportqueueentry__sourcepackagename__idx |
| 304 | +ON TranslationImportQueueEntry(sourcepackagename) |
| 305 | +WHERE sourcepackagename IS NOT NULL; |
| 306 | + |
| 307 | +CREATE INDEX translationimportqueueentry__path__idx |
| 308 | +ON TranslationImportQueueEntry(path); |
| 309 | + |
| 310 | +CREATE INDEX translationimportqueueentry__pofile__idx |
| 311 | +ON TranslationImportQueueEntry(pofile) |
| 312 | +WHERE pofile IS NOT NULL; |
| 313 | + |
| 314 | +CREATE INDEX translationimportqueueentry__potemplate__idx |
| 315 | +ON TranslationImportQueueEntry(potemplate) |
| 316 | +WHERE potemplate IS NOT NULL; |
| 317 | + |
| 318 | +CREATE INDEX pofile__from_sourcepackagename__idx |
| 319 | +ON POFile(from_sourcepackagename) |
| 320 | +WHERE from_sourcepackagename IS NOT NULL; |
| 321 | + |
| 322 | +CREATE INDEX bugwatch__lastchecked__idx ON BugWatch(lastchecked); |
| 323 | +CREATE INDEX bugwatch__remotebug__idx ON BugWatch(remotebug); |
| 324 | +CREATE INDEX bugwatch__remote_lp_bug_id__idx ON BUgWatch(remote_lp_bug_id) |
| 325 | +WHERE remote_lp_bug_id IS NOT NULL; |
| 326 | + |
| 327 | + |
| 328 | +INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 19, 1); |
| 329 | |
| 330 | === added file 'database/schema/patch-2207-20-0.sql' |
| 331 | --- database/schema/patch-2207-20-0.sql 1970-01-01 00:00:00 +0000 |
| 332 | +++ database/schema/patch-2207-20-0.sql 2010-01-13 10:10:29 +0000 |
| 333 | @@ -0,0 +1,13 @@ |
| 334 | +SET client_min_messages=ERROR; |
| 335 | + |
| 336 | +-- Drop the old view. |
| 337 | +DROP VIEW validpersonorteamcache; |
| 338 | + |
| 339 | +-- Create the new view that excludes merged teams. |
| 340 | +CREATE VIEW validpersonorteamcache AS |
| 341 | + SELECT person.id FROM |
| 342 | + ((person LEFT JOIN emailaddress ON ((person.id = emailaddress.person))) LEFT JOIN account ON ((emailaddress.account = account.id))) |
| 343 | + WHERE (((person.teamowner IS NOT NULL) AND (person.merged IS NULL)) OR |
| 344 | + (person.teamowner IS NULL AND (account.status = 20) AND (emailaddress.status = 4))); |
| 345 | + |
| 346 | +INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 20, 0); |
| 347 | |
| 348 | === added file 'database/schema/patch-2207-21-0.sql' |
| 349 | --- database/schema/patch-2207-21-0.sql 1970-01-01 00:00:00 +0000 |
| 350 | +++ database/schema/patch-2207-21-0.sql 2010-01-13 10:10:29 +0000 |
| 351 | @@ -0,0 +1,8 @@ |
| 352 | +SET client_min_messages=ERROR; |
| 353 | + |
| 354 | +ALTER TABLE Language ALTER englishname SET NOT NULL; |
| 355 | + |
| 356 | +ALTER TABLE LibraryFileContent ALTER filesize TYPE bigint; |
| 357 | +CLUSTER LibraryFileContent USING libraryfilecontent_pkey; -- repack |
| 358 | + |
| 359 | +INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 21, 0); |
| 360 | |
| 361 | === added file 'database/schema/patch-2207-24-0.sql' |
| 362 | --- database/schema/patch-2207-24-0.sql 1970-01-01 00:00:00 +0000 |
| 363 | +++ database/schema/patch-2207-24-0.sql 2010-01-13 10:10:29 +0000 |
| 364 | @@ -0,0 +1,20 @@ |
| 365 | +-- Copyright 2009 Canonical Ltd. This software is licensed under the |
| 366 | +-- GNU Affero General Public License version 3 (see the file LICENSE). |
| 367 | + |
| 368 | +SET client_min_messages=ERROR; |
| 369 | + |
| 370 | +-- Another schema patch required for the Soyuz buildd generalisation, see |
| 371 | +-- https://dev.launchpad.net/Soyuz/Specs/BuilddGeneralisation for details. |
| 372 | +-- Bug #505725. |
| 373 | + |
| 374 | +-- Changes needed to the `BuildQueue` table. |
| 375 | + |
| 376 | +-- The 'processor' and the 'virtualized' columns will enable us to formulate |
| 377 | +-- more straightforward queries for finding candidate jobs when builders |
| 378 | +-- become idle. |
| 379 | +ALTER TABLE ONLY buildqueue ADD COLUMN processor integer; |
| 380 | +ALTER TABLE ONLY buildqueue ADD COLUMN virtualized boolean; |
| 381 | + |
| 382 | +CREATE INDEX buildqueue__processor__virtualized__idx ON buildqueue USING btree (processor, virtualized) WHERE (processor IS NOT NULL); |
| 383 | + |
| 384 | +INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 24, 0); |
| 385 | |
| 386 | === modified file 'database/schema/security.cfg' |
| 387 | --- database/schema/security.cfg 2010-01-06 09:06:56 +0000 |
| 388 | +++ database/schema/security.cfg 2010-01-13 10:10:29 +0000 |
| 389 | @@ -1793,6 +1793,9 @@ |
| 390 | # changing DB permissions. |
| 391 | type=user |
| 392 | groups=script,read |
| 393 | +public.bug = SELECT, UPDATE |
| 394 | +public.bugsubscription = SELECT |
| 395 | +public.bugaffectsperson = SELECT |
| 396 | public.bugnotification = SELECT, DELETE |
| 397 | public.bugnotificationrecipientarchive = SELECT |
| 398 | public.codeimportresult = SELECT, DELETE |
| 399 | @@ -1840,6 +1843,11 @@ |
| 400 | |
| 401 | [nagios] |
| 402 | type=user |
| 403 | +public.archive = SELECT |
| 404 | +public.build = SELECT |
| 405 | +public.buildqueue = SELECT |
| 406 | +public.buildpackagejob = SELECT |
| 407 | +public.job = SELECT |
| 408 | public.libraryfilecontent = SELECT |
| 409 | public.openidrpconfig = SELECT |
| 410 | public.branch = SELECT |
| 411 | |
| 412 | === modified file 'database/schema/security.py' |
| 413 | --- database/schema/security.py 2009-11-18 08:09:58 +0000 |
| 414 | +++ database/schema/security.py 2010-01-13 10:10:29 +0000 |
| 415 | @@ -28,6 +28,9 @@ |
| 416 | # sensitive information that interactive sessions don't need. |
| 417 | SECURE_TABLES = [ |
| 418 | 'public.accountpassword', |
| 419 | + 'public.oauthnonce', |
| 420 | + 'public.openidnonce', |
| 421 | + 'public.openidconsumernonce', |
| 422 | ] |
| 423 | |
| 424 | |
| 425 | |
| 426 | === modified file 'lib/canonical/launchpad/scripts/garbo.py' |
| 427 | --- lib/canonical/launchpad/scripts/garbo.py 2009-12-17 02:44:39 +0000 |
| 428 | +++ lib/canonical/launchpad/scripts/garbo.py 2010-01-13 10:10:29 +0000 |
| 429 | @@ -30,7 +30,9 @@ |
| 430 | from canonical.launchpad.utilities.looptuner import DBLoopTuner |
| 431 | from canonical.launchpad.webapp.interfaces import ( |
| 432 | IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR) |
| 433 | +from lp.bugs.interfaces.bug import IBugSet |
| 434 | from lp.bugs.model.bugnotification import BugNotification |
| 435 | +from lp.bugs.scripts.bugheat import BugHeatCalculator |
| 436 | from lp.code.interfaces.revision import IRevisionSet |
| 437 | from lp.code.model.branchjob import BranchJob |
| 438 | from lp.code.model.codeimportresult import CodeImportResult |
| 439 | @@ -691,6 +693,66 @@ |
| 440 | transaction.commit() |
| 441 | |
| 442 | |
| 443 | +class BugHeatUpdater(TunableLoop): |
| 444 | + """A `TunableLoop` for bug heat calculations.""" |
| 445 | + |
| 446 | + maximum_chunk_size = 1000 |
| 447 | + |
| 448 | + def __init__(self, log, abort_time=None): |
| 449 | + super(BugHeatUpdater, self).__init__(log, abort_time) |
| 450 | + self.transaction = transaction |
| 451 | + self.offset = 0 |
| 452 | + self.total_updated = 0 |
| 453 | + |
| 454 | + def isDone(self): |
| 455 | + """See `ITunableLoop`.""" |
| 456 | + # When the main loop has no more Bugs to process it sets |
| 457 | + # offset to None. Until then, it always has a numerical |
| 458 | + # value. |
| 459 | + return self.offset is None |
| 460 | + |
| 461 | + def __call__(self, chunk_size): |
| 462 | + """Retrieve a batch of Bugs and update their heat. |
| 463 | + |
| 464 | + See `ITunableLoop`. |
| 465 | + """ |
| 466 | + # XXX 2010-01-08 gmb bug=198767: |
| 467 | + # We cast chunk_size to an integer to ensure that we're not |
| 468 | + # trying to slice using floats or anything similarly |
| 469 | + # foolish. We shouldn't have to do this. |
| 470 | + chunk_size = int(chunk_size) |
| 471 | + |
| 472 | + start = self.offset |
| 473 | + end = self.offset + chunk_size |
| 474 | + |
| 475 | + transaction.begin() |
| 476 | + # XXX 2010-01-08 gmb bug=505850: |
| 477 | + # This method call should be taken out and shot as soon as |
| 478 | + # we have a proper permissions system for scripts. |
| 479 | + bugs = getUtility(IBugSet).dangerousGetAllBugs()[start:end] |
| 480 | + |
| 481 | + self.offset = None |
| 482 | + bug_count = bugs.count() |
| 483 | + if bug_count > 0: |
| 484 | + starting_id = bugs.first().id |
| 485 | + self.log.debug("Updating %i Bugs (starting id: %i)" % |
| 486 | + (bug_count, starting_id)) |
| 487 | + |
| 488 | + for bug in bugs: |
| 489 | + # We set the starting point of the next batch to the Bug |
| 490 | + # id after the one we're looking at now. If there aren't any |
| 491 | + # bugs this loop will run for 0 iterations and starting_id |
| 492 | + # will remain set to None. |
| 493 | + start += 1 |
| 494 | + self.offset = start |
| 495 | + self.log.debug("Updating heat for bug %s" % bug.id) |
| 496 | + bug_heat_calculator = BugHeatCalculator(bug) |
| 497 | + heat = bug_heat_calculator.getBugHeat() |
| 498 | + bug.setHeat(heat) |
| 499 | + self.total_updated += 1 |
| 500 | + transaction.commit() |
| 501 | + |
| 502 | + |
| 503 | class BaseDatabaseGarbageCollector(LaunchpadCronScript): |
| 504 | """Abstract base class to run a collection of TunableLoops.""" |
| 505 | script_name = None # Script name for locking and database user. Override. |
| 506 | @@ -795,6 +857,7 @@ |
| 507 | PersonEmailAddressLinkChecker, |
| 508 | BugNotificationPruner, |
| 509 | BranchJobPruner, |
| 510 | + BugHeatUpdater, |
| 511 | ] |
| 512 | experimental_tunable_loops = [ |
| 513 | PersonPruner, |
| 514 | |
| 515 | === modified file 'lib/lp/bugs/configure.zcml' |
| 516 | --- lib/lp/bugs/configure.zcml 2009-12-09 11:14:16 +0000 |
| 517 | +++ lib/lp/bugs/configure.zcml 2010-01-13 10:10:29 +0000 |
| 518 | @@ -572,7 +572,8 @@ |
| 519 | userCanView |
| 520 | personIsDirectSubscriber |
| 521 | personIsAlsoNotifiedSubscriber |
| 522 | - personIsSubscribedToDuplicate"/> |
| 523 | + personIsSubscribedToDuplicate |
| 524 | + heat"/> |
| 525 | <require |
| 526 | permission="launchpad.View" |
| 527 | attributes=" |
| 528 | @@ -668,7 +669,8 @@ |
| 529 | <require |
| 530 | permission="launchpad.Admin" |
| 531 | attributes=" |
| 532 | - setCommentVisibility"/> |
| 533 | + setCommentVisibility |
| 534 | + setHeat"/> |
| 535 | </class> |
| 536 | <adapter |
| 537 | for="lp.bugs.interfaces.bug.IBug" |
| 538 | |
| 539 | === added file 'lib/lp/bugs/doc/bug-heat.txt' |
| 540 | --- lib/lp/bugs/doc/bug-heat.txt 1970-01-01 00:00:00 +0000 |
| 541 | +++ lib/lp/bugs/doc/bug-heat.txt 2010-01-13 10:10:29 +0000 |
| 542 | @@ -0,0 +1,54 @@ |
| 543 | +Calculating bug heat |
| 544 | +==================== |
| 545 | + |
| 546 | +Launchpad bugs each have a 'heat' rating. This is an indicator of how |
| 547 | +problematic a given bug is to the community and can be used to determine |
| 548 | +which bugs should be tackled first. |
| 549 | + |
| 550 | +A new bug will have a heat of zero. |
| 551 | + |
| 552 | + >>> bug_owner = factory.makePerson() |
| 553 | + >>> bug = factory.makeBug(owner=bug_owner) |
| 554 | + >>> bug.heat |
| 555 | + 0 |
| 556 | + |
| 557 | +The bug's heat can be set by calling its setHeat() method. |
| 558 | + |
| 559 | + >>> bug.setHeat(42) |
| 560 | + >>> bug.heat |
| 561 | + 42 |
| 562 | + |
| 563 | + |
| 564 | +The BugHeatUpdater class |
| 565 | +--------------------------- |
| 566 | + |
| 567 | +In order to calculate bug heat we need to use the BugHeatUpdater |
| 568 | +class, which is designed precisely for that task. It's part of the garbo |
| 569 | +module and runs as part of the garbo-daily cronjob. |
| 570 | + |
| 571 | + >>> from canonical.launchpad.scripts.garbo import BugHeatUpdater |
| 572 | + >>> from canonical.launchpad.scripts import FakeLogger |
| 573 | + |
| 574 | + >>> update_bug_heat = BugHeatUpdater(FakeLogger()) |
| 575 | + |
| 576 | +BugHeatUpdater implements ITunableLoop and as such is callable. Calling |
| 577 | +it as a method will update the heat for all the bugs currently held in |
| 578 | +Launchpad. |
| 579 | + |
| 580 | +Before update_bug_heat is called, bug 1 will have no heat. |
| 581 | + |
| 582 | + >>> from zope.component import getUtility |
| 583 | + >>> from lp.bugs.interfaces.bug import IBugSet |
| 584 | + >>> bug_1 = getUtility(IBugSet).get(1) |
| 585 | + |
| 586 | + >>> bug_1.heat |
| 587 | + 0 |
| 588 | + |
| 589 | + >>> update_bug_heat(chunk_size=1) |
| 590 | + DEBUG Updating 1 Bugs (starting id: ...) |
| 591 | + ... |
| 592 | + |
| 593 | +Bug 1's heat will now be greater than 0. |
| 594 | + |
| 595 | + >>> bug_1.heat > 0 |
| 596 | + True |
| 597 | |
| 598 | === modified file 'lib/lp/bugs/interfaces/bug.py' |
| 599 | --- lib/lp/bugs/interfaces/bug.py 2009-12-09 20:48:01 +0000 |
| 600 | +++ lib/lp/bugs/interfaces/bug.py 2010-01-13 10:10:29 +0000 |
| 601 | @@ -296,6 +296,10 @@ |
| 602 | value_type=Reference(schema=IPerson), |
| 603 | readonly=True)) |
| 604 | |
| 605 | + heat = Int( |
| 606 | + title=_("The 'heat' of the bug"), |
| 607 | + required=False, readonly=True) |
| 608 | + |
| 609 | # Adding related BugMessages provides a hook for getting at |
| 610 | # BugMessage.visible when building bug comments. |
| 611 | bug_messages = Attribute('The bug messages related to this object.') |
| 612 | @@ -732,6 +736,9 @@ |
| 613 | if the user is the owner or an admin. |
| 614 | """ |
| 615 | |
| 616 | + def setHeat(heat): |
| 617 | + """Set the heat for the bug.""" |
| 618 | + |
| 619 | class InvalidDuplicateValue(Exception): |
| 620 | """A bug cannot be set as the duplicate of another.""" |
| 621 | webservice_error(417) |
| 622 | @@ -970,6 +977,19 @@ |
| 623 | Otherwise, return False. |
| 624 | """ |
| 625 | |
| 626 | + def dangerousGetAllBugs(): |
| 627 | + """DO NOT CALL THIS METHOD. |
| 628 | + |
| 629 | + This method exists solely to allow the bug heat script to grab |
| 630 | + all the bugs in the database - including private ones - and |
| 631 | + iterate over them. DO NOT USE IT UNLESS YOU KNOW WHAT YOU'RE |
| 632 | + DOING. AND IF YOU KNOW WHAT YOU'RE DOING YOU KNOW BETTER THAN TO |
| 633 | + USE THIS ANYWAY. |
| 634 | + """ |
| 635 | + # XXX 2010-01-08 gmb bug=505850: |
| 636 | + # Note, this method should go away when we have a proper |
| 637 | + # permissions system for scripts. |
| 638 | + |
| 639 | |
| 640 | class InvalidBugTargetType(Exception): |
| 641 | """Bug target's type is not valid.""" |
| 642 | |
| 643 | === modified file 'lib/lp/bugs/model/bug.py' |
| 644 | --- lib/lp/bugs/model/bug.py 2009-12-14 15:25:55 +0000 |
| 645 | +++ lib/lp/bugs/model/bug.py 2010-01-13 10:10:29 +0000 |
| 646 | @@ -255,6 +255,10 @@ |
| 647 | users_affected_count = IntCol(notNull=True, default=0) |
| 648 | users_unaffected_count = IntCol(notNull=True, default=0) |
| 649 | |
| 650 | + # This is called 'hotness' in the database but the canonical name in |
| 651 | + # the code is 'heat', so we use that name here. |
| 652 | + heat = IntCol(dbName='hotness', notNull=True, default=0) |
| 653 | + |
| 654 | @property |
| 655 | def comment_count(self): |
| 656 | """See `IBug`.""" |
| 657 | @@ -1425,6 +1429,10 @@ |
| 658 | |
| 659 | return not subscriptions_from_dupes.is_empty() |
| 660 | |
| 661 | + def setHeat(self, heat): |
| 662 | + """See `IBug`.""" |
| 663 | + self.heat = heat |
| 664 | + |
| 665 | |
| 666 | class BugSet: |
| 667 | """See BugSet.""" |
| 668 | @@ -1665,13 +1673,19 @@ |
| 669 | return bugs |
| 670 | |
| 671 | def getByNumbers(self, bug_numbers): |
| 672 | - """see `IBugSet`.""" |
| 673 | + """See `IBugSet`.""" |
| 674 | if bug_numbers is None or len(bug_numbers) < 1: |
| 675 | return EmptyResultSet() |
| 676 | store = IStore(Bug) |
| 677 | result_set = store.find(Bug, In(Bug.id, bug_numbers)) |
| 678 | return result_set.order_by('id') |
| 679 | |
| 680 | + def dangerousGetAllBugs(self): |
| 681 | + """See `IBugSet`.""" |
| 682 | + store = IStore(Bug) |
| 683 | + result_set = store.find(Bug) |
| 684 | + return result_set.order_by('id') |
| 685 | + |
| 686 | |
| 687 | class BugAffectsPerson(SQLBase): |
| 688 | """A bug is marked as affecting a user.""" |
| 689 | |
| 690 | === added file 'lib/lp/bugs/scripts/bugheat.py' |
| 691 | --- lib/lp/bugs/scripts/bugheat.py 1970-01-01 00:00:00 +0000 |
| 692 | +++ lib/lp/bugs/scripts/bugheat.py 2010-01-13 10:10:29 +0000 |
| 693 | @@ -0,0 +1,75 @@ |
| 694 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
| 695 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
| 696 | + |
| 697 | +"""The innards of the Bug Heat cronscript.""" |
| 698 | + |
| 699 | +__metaclass__ = type |
| 700 | +__all__ = [] |
| 701 | + |
| 702 | + |
| 703 | +from zope.component import getUtility |
| 704 | +from zope.interface import implements |
| 705 | + |
| 706 | +from canonical.launchpad.interfaces.looptuner import ITunableLoop |
| 707 | +from canonical.launchpad.utilities.looptuner import DBLoopTuner |
| 708 | + |
| 709 | + |
| 710 | +class BugHeatConstants: |
| 711 | + |
| 712 | + PRIVACY = 150 |
| 713 | + SECURITY = 250 |
| 714 | + DUPLICATE = 6 |
| 715 | + AFFECTED_USER = 4 |
| 716 | + SUBSCRIBER = 2 |
| 717 | + |
| 718 | + |
| 719 | +class BugHeatCalculator: |
| 720 | + """A class to calculate the heat for a bug.""" |
| 721 | + |
| 722 | + def __init__(self, bug): |
| 723 | + self.bug = bug |
| 724 | + |
| 725 | + def _getHeatFromPrivacy(self): |
| 726 | + """Return the heat generated by the bug's `private` attribute.""" |
| 727 | + if self.bug.private: |
| 728 | + return BugHeatConstants.PRIVACY |
| 729 | + else: |
| 730 | + return 0 |
| 731 | + |
| 732 | + def _getHeatFromSecurity(self): |
| 733 | + """Return the heat generated if the bug is security related.""" |
| 734 | + if self.bug.security_related: |
| 735 | + return BugHeatConstants.SECURITY |
| 736 | + else: |
| 737 | + return 0 |
| 738 | + |
| 739 | + def _getHeatFromDuplicates(self): |
| 740 | + """Return the heat generated by the bug's duplicates.""" |
| 741 | + return self.bug.duplicates.count() * BugHeatConstants.DUPLICATE |
| 742 | + |
| 743 | + def _getHeatFromAffectedUsers(self): |
| 744 | + """Return the heat generated by the bug's affected users.""" |
| 745 | + return ( |
| 746 | + self.bug.users_affected.count() * BugHeatConstants.AFFECTED_USER) |
| 747 | + |
| 748 | + def _getHeatFromSubscribers(self): |
| 749 | + """Return the heat generated by the bug's subscribers.""" |
| 750 | + direct_subscribers = self.bug.getDirectSubscribers() |
| 751 | + subscribers_from_dupes = self.bug.getSubscribersFromDuplicates() |
| 752 | + |
| 753 | + subscriber_count = ( |
| 754 | + len(direct_subscribers) + len(subscribers_from_dupes)) |
| 755 | + return subscriber_count * BugHeatConstants.SUBSCRIBER |
| 756 | + |
| 757 | + def getBugHeat(self): |
| 758 | + """Return the total heat for the current bug.""" |
| 759 | + total_heat = sum([ |
| 760 | + self._getHeatFromAffectedUsers(), |
| 761 | + self._getHeatFromDuplicates(), |
| 762 | + self._getHeatFromPrivacy(), |
| 763 | + self._getHeatFromSecurity(), |
| 764 | + self._getHeatFromSubscribers(), |
| 765 | + ]) |
| 766 | + |
| 767 | + return total_heat |
| 768 | + |
| 769 | |
| 770 | === added file 'lib/lp/bugs/scripts/tests/test_bugheat.py' |
| 771 | --- lib/lp/bugs/scripts/tests/test_bugheat.py 1970-01-01 00:00:00 +0000 |
| 772 | +++ lib/lp/bugs/scripts/tests/test_bugheat.py 2010-01-13 10:10:29 +0000 |
| 773 | @@ -0,0 +1,183 @@ |
| 774 | + |
| 775 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
| 776 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
| 777 | + |
| 778 | +"""Module docstring goes here.""" |
| 779 | + |
| 780 | +__metaclass__ = type |
| 781 | + |
| 782 | +import unittest |
| 783 | + |
| 784 | +from canonical.testing import LaunchpadZopelessLayer |
| 785 | + |
| 786 | +from lp.bugs.scripts.bugheat import BugHeatCalculator, BugHeatConstants |
| 787 | +from lp.testing import TestCaseWithFactory |
| 788 | + |
| 789 | + |
| 790 | +class TestBugHeatCalculator(TestCaseWithFactory): |
| 791 | + """Tests for the BugHeatCalculator class.""" |
| 792 | + |
| 793 | + layer = LaunchpadZopelessLayer |
| 794 | + |
| 795 | + def setUp(self): |
| 796 | + super(TestBugHeatCalculator, self).setUp() |
| 797 | + self.bug = self.factory.makeBug() |
| 798 | + self.calculator = BugHeatCalculator(self.bug) |
| 799 | + |
| 800 | + def test__getHeatFromDuplicates(self): |
| 801 | + # BugHeatCalculator._getHeatFromDuplicates() returns the bug |
| 802 | + # heat generated by duplicates of a bug. |
| 803 | + # By default, the bug has no heat from dupes |
| 804 | + self.assertEqual(0, self.calculator._getHeatFromDuplicates()) |
| 805 | + |
| 806 | + # If adding duplicates, the heat generated by them will be n * |
| 807 | + # BugHeatConstants.DUPLICATE, where n is the number of |
| 808 | + # duplicates. |
| 809 | + for i in range(5): |
| 810 | + dupe = self.factory.makeBug() |
| 811 | + dupe.duplicateof = self.bug |
| 812 | + |
| 813 | + expected_heat = BugHeatConstants.DUPLICATE * 5 |
| 814 | + actual_heat = self.calculator._getHeatFromDuplicates() |
| 815 | + self.assertEqual( |
| 816 | + expected_heat, actual_heat, |
| 817 | + "Heat from duplicates does not match expected heat. " |
| 818 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 819 | + |
| 820 | + def test__getHeatFromAffectedUsers(self): |
| 821 | + # BugHeatCalculator._getHeatFromAffectedUsers() returns the bug |
| 822 | + # heat generated by users affected by the bug. |
| 823 | + # By default, the heat will be BugHeatConstants.AFFECTED_USER, since |
| 824 | + # there will be one affected user (the user who filed the bug). |
| 825 | + self.assertEqual( |
| 826 | + BugHeatConstants.AFFECTED_USER, |
| 827 | + self.calculator._getHeatFromAffectedUsers()) |
| 828 | + |
| 829 | + # As the number of affected users increases, the heat generated |
| 830 | + # will be n * BugHeatConstants.AFFECTED_USER, where n is the number |
| 831 | + # of affected users. |
| 832 | + for i in range(5): |
| 833 | + person = self.factory.makePerson() |
| 834 | + self.bug.markUserAffected(person) |
| 835 | + |
| 836 | + expected_heat = BugHeatConstants.AFFECTED_USER * 6 |
| 837 | + actual_heat = self.calculator._getHeatFromAffectedUsers() |
| 838 | + self.assertEqual( |
| 839 | + expected_heat, actual_heat, |
| 840 | + "Heat from affected users does not match expected heat. " |
| 841 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 842 | + |
| 843 | + def test__getHeatFromSubscribers(self): |
| 844 | + # BugHeatCalculator._getHeatFromSubscribers() returns the bug |
| 845 | + # heat generated by users subscribed tothe bug. |
| 846 | + # By default, the heat will be BugHeatConstants.SUBSCRIBER, |
| 847 | + # since there will be one direct subscriber (the user who filed |
| 848 | + # the bug). |
| 849 | + self.assertEqual( |
| 850 | + BugHeatConstants.SUBSCRIBER, |
| 851 | + self.calculator._getHeatFromSubscribers()) |
| 852 | + |
| 853 | + # As the number of subscribers increases, the heat generated |
| 854 | + # will be n * BugHeatConstants.SUBSCRIBER, where n is the number |
| 855 | + # of subscribers. |
| 856 | + for i in range(5): |
| 857 | + person = self.factory.makePerson() |
| 858 | + self.bug.subscribe(person, person) |
| 859 | + |
| 860 | + expected_heat = BugHeatConstants.SUBSCRIBER * 6 |
| 861 | + actual_heat = self.calculator._getHeatFromSubscribers() |
| 862 | + self.assertEqual( |
| 863 | + expected_heat, actual_heat, |
| 864 | + "Heat from subscribers does not match expected heat. " |
| 865 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 866 | + |
| 867 | + # Subscribers from duplicates are included in the heat returned |
| 868 | + # by _getHeatFromSubscribers() |
| 869 | + dupe = self.factory.makeBug() |
| 870 | + dupe.duplicateof = self.bug |
| 871 | + expected_heat = BugHeatConstants.SUBSCRIBER * 7 |
| 872 | + actual_heat = self.calculator._getHeatFromSubscribers() |
| 873 | + self.assertEqual( |
| 874 | + expected_heat, actual_heat, |
| 875 | + "Heat from subscribers (including duplicate-subscribers) " |
| 876 | + "does not match expected heat. Expected %s, got %s" % |
| 877 | + (expected_heat, actual_heat)) |
| 878 | + |
| 879 | + # Seting the bug to private will increase its heat from |
| 880 | + # subscribers by 1 * BugHeatConstants.SUBSCRIBER, as the project |
| 881 | + # owner will now be directly subscribed to it. |
| 882 | + self.bug.setPrivate(True, self.bug.owner) |
| 883 | + expected_heat = BugHeatConstants.SUBSCRIBER * 8 |
| 884 | + actual_heat = self.calculator._getHeatFromSubscribers() |
| 885 | + self.assertEqual( |
| 886 | + expected_heat, actual_heat, |
| 887 | + "Heat from subscribers to private bug does not match expected " |
| 888 | + "heat. Expected %s, got %s" % (expected_heat, actual_heat)) |
| 889 | + |
| 890 | + def test__getHeatFromPrivacy(self): |
| 891 | + # BugHeatCalculator._getHeatFromPrivacy() returns the heat |
| 892 | + # generated by the bug's private attribute. If the bug is |
| 893 | + # public, this will be 0. |
| 894 | + self.assertEqual(0, self.calculator._getHeatFromPrivacy()) |
| 895 | + |
| 896 | + # However, if the bug is private, _getHeatFromPrivacy() will |
| 897 | + # return BugHeatConstants.PRIVACY. |
| 898 | + self.bug.setPrivate(True, self.bug.owner) |
| 899 | + self.assertEqual( |
| 900 | + BugHeatConstants.PRIVACY, self.calculator._getHeatFromPrivacy()) |
| 901 | + |
| 902 | + def test__getHeatFromSecurity(self): |
| 903 | + # BugHeatCalculator._getHeatFromSecurity() returns the heat |
| 904 | + # generated by the bug's security_related attribute. If the bug |
| 905 | + # is not security related, _getHeatFromSecurity() will return 0. |
| 906 | + self.assertEqual(0, self.calculator._getHeatFromPrivacy()) |
| 907 | + |
| 908 | + |
| 909 | + # If, on the other hand, the bug is security_related, |
| 910 | + # _getHeatFromSecurity() will return BugHeatConstants.SECURITY |
| 911 | + self.bug.security_related = True |
| 912 | + self.assertEqual( |
| 913 | + BugHeatConstants.SECURITY, self.calculator._getHeatFromSecurity()) |
| 914 | + |
| 915 | + def test_getBugHeat(self): |
| 916 | + # BugHeatCalculator.getBugHeat() returns the total heat for a |
| 917 | + # given bug as the sum of the results of all _getHeatFrom*() |
| 918 | + # methods. |
| 919 | + # By default this will be (BugHeatConstants.AFFECTED_USER + |
| 920 | + # BugHeatConstants.SUBSCRIBER) since there will be one |
| 921 | + # subscriber and one affected user only. |
| 922 | + expected_heat = ( |
| 923 | + BugHeatConstants.AFFECTED_USER + BugHeatConstants.SUBSCRIBER) |
| 924 | + actual_heat = self.calculator.getBugHeat() |
| 925 | + self.assertEqual( |
| 926 | + expected_heat, actual_heat, |
| 927 | + "Expected bug heat did not match actual bug heat. " |
| 928 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 929 | + |
| 930 | + # Adding a duplicate and making the bug private and security |
| 931 | + # related will increase its heat. |
| 932 | + dupe = self.factory.makeBug() |
| 933 | + dupe.duplicateof = self.bug |
| 934 | + self.bug.setPrivate(True, self.bug.owner) |
| 935 | + self.bug.security_related = True |
| 936 | + |
| 937 | + expected_heat += ( |
| 938 | + BugHeatConstants.DUPLICATE + |
| 939 | + BugHeatConstants.PRIVACY + |
| 940 | + BugHeatConstants.SECURITY |
| 941 | + ) |
| 942 | + |
| 943 | + # Adding the duplicate and making the bug private means it gets |
| 944 | + # two new subscribers, the project owner and the duplicate's |
| 945 | + # direct subscriber. |
| 946 | + expected_heat += BugHeatConstants.SUBSCRIBER * 2 |
| 947 | + actual_heat = self.calculator.getBugHeat() |
| 948 | + self.assertEqual( |
| 949 | + expected_heat, actual_heat, |
| 950 | + "Expected bug heat did not match actual bug heat. " |
| 951 | + "Expected %s, got %s" % (expected_heat, actual_heat)) |
| 952 | + |
| 953 | + |
| 954 | +def test_suite(): |
| 955 | + return unittest.TestLoader().loadTestsFromName(__name__) |
| 956 | + |
| 957 | |
| 958 | === modified file 'lib/lp/bugs/tests/test_doc.py' |
| 959 | --- lib/lp/bugs/tests/test_doc.py 2009-10-02 11:29:22 +0000 |
| 960 | +++ lib/lp/bugs/tests/test_doc.py 2010-01-13 10:10:29 +0000 |
| 961 | @@ -89,6 +89,12 @@ |
| 962 | '../doc/cve-update.txt', |
| 963 | setUp=cveSetUp, tearDown=tearDown, layer=LaunchpadZopelessLayer |
| 964 | ), |
| 965 | + 'bug-heat.txt': LayeredDocFileSuite( |
| 966 | + '../doc/bug-heat.txt', |
| 967 | + setUp=setUp, |
| 968 | + tearDown=tearDown, |
| 969 | + layer=LaunchpadZopelessLayer |
| 970 | + ), |
| 971 | 'bugnotificationrecipients.txt-uploader': LayeredDocFileSuite( |
| 972 | '../doc/bugnotificationrecipients.txt', |
| 973 | setUp=uploaderBugsSetUp, |
| 974 | |
| 975 | === modified file 'lib/lp/code/browser/codereviewvote.py' |
| 976 | --- lib/lp/code/browser/codereviewvote.py 2009-12-10 20:46:32 +0000 |
| 977 | +++ lib/lp/code/browser/codereviewvote.py 2010-01-13 10:10:29 +0000 |
| 978 | @@ -1,20 +1,18 @@ |
| 979 | # Copyright 2009 Canonical Ltd. This software is licensed under the |
| 980 | # GNU Affero General Public License version 3 (see the file LICENSE). |
| 981 | |
| 982 | - |
| 983 | """Views, navigation and actions for CodeReviewVotes.""" |
| 984 | |
| 985 | - |
| 986 | __metaclass__ = type |
| 987 | |
| 988 | |
| 989 | from zope.interface import Interface |
| 990 | -from zope.security.proxy import removeSecurityProxy |
| 991 | |
| 992 | from canonical.launchpad import _ |
| 993 | from canonical.launchpad.fields import PublicPersonChoice |
| 994 | from canonical.launchpad.webapp import ( |
| 995 | action, canonical_url, LaunchpadFormView) |
| 996 | +from lp.code.errors import ReviewNotPending, UserHasExistingReview |
| 997 | |
| 998 | |
| 999 | class ReassignSchema(Interface): |
| 1000 | @@ -35,8 +33,14 @@ |
| 1001 | @action('Reassign', name='reassign') |
| 1002 | def reassign_action(self, action, data): |
| 1003 | """Use the form data to change the review request reviewer.""" |
| 1004 | - # XXX TimPenhey 2009-12-11 bug=495201 |
| 1005 | - # This should check for existing reviews by the reviewer, and have |
| 1006 | - # the logic moved into the model code. |
| 1007 | - removeSecurityProxy(self.context).reviewer = data['reviewer'] |
| 1008 | + self.context.reassignReview(data['reviewer']) |
| 1009 | self.next_url = canonical_url(self.context.branch_merge_proposal) |
| 1010 | + |
| 1011 | + def validate(self, data): |
| 1012 | + """Make sure that the reassignment can happen.""" |
| 1013 | + reviewer = data.get('reviewer') |
| 1014 | + if reviewer is not None: |
| 1015 | + try: |
| 1016 | + self.context.validateReasignReview(reviewer) |
| 1017 | + except (ReviewNotPending, UserHasExistingReview), e: |
| 1018 | + self.addError(str(e)) |
| 1019 | |
| 1020 | === modified file 'lib/lp/code/errors.py' |
| 1021 | --- lib/lp/code/errors.py 2009-12-07 06:51:42 +0000 |
| 1022 | +++ lib/lp/code/errors.py 2010-01-13 10:10:29 +0000 |
| 1023 | @@ -11,6 +11,7 @@ |
| 1024 | 'ClaimReviewFailed', |
| 1025 | 'InvalidBranchMergeProposal', |
| 1026 | 'ReviewNotPending', |
| 1027 | + 'UserHasExistingReview', |
| 1028 | 'UserNotBranchReviewer', |
| 1029 | 'WrongBranchMergeProposal', |
| 1030 | ] |
| 1031 | @@ -43,6 +44,10 @@ |
| 1032 | """The requested review is not in a pending state.""" |
| 1033 | |
| 1034 | |
| 1035 | +class UserHasExistingReview(Exception): |
| 1036 | + """The user has an existing review.""" |
| 1037 | + |
| 1038 | + |
| 1039 | class UserNotBranchReviewer(Exception): |
| 1040 | """The user who attempted to review the merge proposal isn't a reviewer. |
| 1041 | |
| 1042 | |
| 1043 | === modified file 'lib/lp/code/interfaces/codereviewvote.py' |
| 1044 | --- lib/lp/code/interfaces/codereviewvote.py 2009-12-07 06:51:42 +0000 |
| 1045 | +++ lib/lp/code/interfaces/codereviewvote.py 2010-01-13 10:10:29 +0000 |
| 1046 | @@ -17,10 +17,11 @@ |
| 1047 | IBranchMergeProposal) |
| 1048 | from lp.code.interfaces.codereviewcomment import ( |
| 1049 | ICodeReviewComment) |
| 1050 | +from lp.registry.interfaces.person import IPerson |
| 1051 | from lazr.restful.fields import Reference |
| 1052 | from lazr.restful.declarations import ( |
| 1053 | call_with, export_as_webservice_entry, export_destructor_operation, |
| 1054 | - export_write_operation, exported, REQUEST_USER) |
| 1055 | + export_write_operation, exported, operation_parameters, REQUEST_USER) |
| 1056 | |
| 1057 | |
| 1058 | class ICodeReviewVoteReferencePublic(Interface): |
| 1059 | @@ -70,6 +71,15 @@ |
| 1060 | class ICodeReviewVoteReferenceEdit(Interface): |
| 1061 | """Method that require edit permissions.""" |
| 1062 | |
| 1063 | + def validateClaimReview(claimant): |
| 1064 | + """Implements the validation for claiming a review. |
| 1065 | + |
| 1066 | + :raises ClaimReviewFailed: If the claimant already has a |
| 1067 | + personal review, if the reviewer is not a team, if the |
| 1068 | + claimant is not in the reviewer team, or if the review is |
| 1069 | + not pending. |
| 1070 | + """ |
| 1071 | + |
| 1072 | @call_with(claimant=REQUEST_USER) |
| 1073 | @export_write_operation() |
| 1074 | def claimReview(claimant): |
| 1075 | @@ -86,6 +96,30 @@ |
| 1076 | not pending. |
| 1077 | """ |
| 1078 | |
| 1079 | + def validateReasignReview(reviewer): |
| 1080 | + """Implements the validation for reassignment. |
| 1081 | + |
| 1082 | + :raises ReviewNotPending: If the review is not pending. |
| 1083 | + :raises ReassignReviewFailed: If the reviewer is an individual and |
| 1084 | + already has a personal review. |
| 1085 | + """ |
| 1086 | + |
| 1087 | + @operation_parameters( |
| 1088 | + reviewer=Reference( |
| 1089 | + title=_("The person or team to assign to do the review."), |
| 1090 | + schema=IPerson)) |
| 1091 | + @export_write_operation() |
| 1092 | + def reassignReview(reviewer): |
| 1093 | + """Reassign a pending review to someone else. |
| 1094 | + |
| 1095 | + Pending reviews can be reassigned to someone else. |
| 1096 | + |
| 1097 | + :param reviewer: The person to assign the pending review to. |
| 1098 | + :raises ReviewNotPending: If the review is not pending. |
| 1099 | + :raises ReassignReviewFailed: If the reviewer is an individual and |
| 1100 | + already has a personal review. |
| 1101 | + """ |
| 1102 | + |
| 1103 | @export_destructor_operation() |
| 1104 | def delete(): |
| 1105 | """Delete the pending review. |
| 1106 | |
| 1107 | === modified file 'lib/lp/code/mail/tests/test_codehandler.py' |
| 1108 | --- lib/lp/code/mail/tests/test_codehandler.py 2010-01-07 06:37:14 +0000 |
| 1109 | +++ lib/lp/code/mail/tests/test_codehandler.py 2010-01-13 10:10:29 +0000 |
| 1110 | @@ -832,7 +832,7 @@ |
| 1111 | target branch. |
| 1112 | """ |
| 1113 | db_target_branch, target_tree = self.create_branch_and_tree( |
| 1114 | - format=format) |
| 1115 | + tree_location='.', format=format) |
| 1116 | target_tree.branch.set_public_branch(db_target_branch.bzr_identity) |
| 1117 | target_tree.commit('rev1') |
| 1118 | # Make sure that the created branch has been mirrored. |
| 1119 | |
| 1120 | === modified file 'lib/lp/code/model/codereviewvote.py' |
| 1121 | --- lib/lp/code/model/codereviewvote.py 2009-12-07 06:51:42 +0000 |
| 1122 | +++ lib/lp/code/model/codereviewvote.py 2010-01-13 10:10:29 +0000 |
| 1123 | @@ -15,7 +15,8 @@ |
| 1124 | from canonical.database.constants import DEFAULT |
| 1125 | from canonical.database.datetimecol import UtcDateTimeCol |
| 1126 | from canonical.database.sqlbase import SQLBase |
| 1127 | -from lp.code.errors import ClaimReviewFailed, ReviewNotPending |
| 1128 | +from lp.code.errors import ( |
| 1129 | + ClaimReviewFailed, ReviewNotPending, UserHasExistingReview) |
| 1130 | from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference |
| 1131 | |
| 1132 | |
| 1133 | @@ -44,10 +45,25 @@ |
| 1134 | # Reviews are pending if there is no associated comment. |
| 1135 | return self.comment is None |
| 1136 | |
| 1137 | - def claimReview(self, claimant): |
| 1138 | + def _validatePending(self): |
| 1139 | + """Raise if the review is not pending.""" |
| 1140 | + if not self.is_pending: |
| 1141 | + raise ReviewNotPending('The review is not pending.') |
| 1142 | + |
| 1143 | + def _validateNoReviewForUser(self, user): |
| 1144 | + """Make sure there isn't an existing review for the user.""" |
| 1145 | + bmp = self.branch_merge_proposal |
| 1146 | + existing_review = bmp.getUsersVoteReference(user) |
| 1147 | + if existing_review is not None: |
| 1148 | + if existing_review.is_pending: |
| 1149 | + error_str = '%s has already been asked to review this' |
| 1150 | + else: |
| 1151 | + error_str = '%s has already reviewed this' |
| 1152 | + raise UserHasExistingReview(error_str % user.unique_displayname) |
| 1153 | + |
| 1154 | + def validateClaimReview(self, claimant): |
| 1155 | """See `ICodeReviewVote`""" |
| 1156 | - if not self.is_pending: |
| 1157 | - raise ClaimReviewFailed('The review is not pending.') |
| 1158 | + self._validatePending() |
| 1159 | if not self.reviewer.is_team: |
| 1160 | raise ClaimReviewFailed('Cannot claim non-team reviews.') |
| 1161 | if not claimant.inTeam(self.reviewer): |
| 1162 | @@ -55,17 +71,24 @@ |
| 1163 | '%s is not a member of %s' % |
| 1164 | (claimant.unique_displayname, |
| 1165 | self.reviewer.unique_displayname)) |
| 1166 | - claimant_review = ( |
| 1167 | - self.branch_merge_proposal.getUsersVoteReference(claimant)) |
| 1168 | - if claimant_review is not None: |
| 1169 | - if claimant_review.is_pending: |
| 1170 | - error_str = '%s has an existing pending review' |
| 1171 | - else: |
| 1172 | - error_str = '%s has an existing personal review' |
| 1173 | - raise ClaimReviewFailed( |
| 1174 | - error_str % claimant.unique_displayname) |
| 1175 | + self._validateNoReviewForUser(claimant) |
| 1176 | + |
| 1177 | + def claimReview(self, claimant): |
| 1178 | + """See `ICodeReviewVote`""" |
| 1179 | + self.validateClaimReview(claimant) |
| 1180 | self.reviewer = claimant |
| 1181 | |
| 1182 | + def validateReasignReview(self, reviewer): |
| 1183 | + """See `ICodeReviewVote`""" |
| 1184 | + self._validatePending() |
| 1185 | + if not reviewer.is_team: |
| 1186 | + self._validateNoReviewForUser(reviewer) |
| 1187 | + |
| 1188 | + def reassignReview(self, reviewer): |
| 1189 | + """See `ICodeReviewVote`""" |
| 1190 | + self.validateReasignReview(reviewer) |
| 1191 | + self.reviewer = reviewer |
| 1192 | + |
| 1193 | def delete(self): |
| 1194 | """See `ICodeReviewVote`""" |
| 1195 | if not self.is_pending: |
| 1196 | |
| 1197 | === modified file 'lib/lp/code/model/tests/test_branchjob.py' |
| 1198 | --- lib/lp/code/model/tests/test_branchjob.py 2009-11-21 00:07:19 +0000 |
| 1199 | +++ lib/lp/code/model/tests/test_branchjob.py 2010-01-13 10:10:29 +0000 |
| 1200 | @@ -8,6 +8,7 @@ |
| 1201 | import datetime |
| 1202 | import os |
| 1203 | import shutil |
| 1204 | +import tempfile |
| 1205 | from unittest import TestLoader |
| 1206 | |
| 1207 | from bzrlib import errors as bzr_errors |
| 1208 | @@ -111,11 +112,16 @@ |
| 1209 | def test_run_diff_content(self): |
| 1210 | """Ensure that run generates expected diff.""" |
| 1211 | self.useBzrBranches() |
| 1212 | - branch, tree = self.create_branch_and_tree() |
| 1213 | - open('file', 'wb').write('foo\n') |
| 1214 | + |
| 1215 | + tree_location = tempfile.mkdtemp() |
| 1216 | + self.addCleanup(lambda: shutil.rmtree(tree_location)) |
| 1217 | + |
| 1218 | + branch, tree = self.create_branch_and_tree(tree_location=tree_location) |
| 1219 | + tree_file = os.path.join(tree_location, 'file') |
| 1220 | + open(tree_file, 'wb').write('foo\n') |
| 1221 | tree.add('file') |
| 1222 | tree.commit('First commit') |
| 1223 | - open('file', 'wb').write('bar\n') |
| 1224 | + open(tree_file, 'wb').write('bar\n') |
| 1225 | tree.commit('Next commit') |
| 1226 | job = BranchDiffJob.create(branch, '1', '2') |
| 1227 | static_diff = job.run() |
| 1228 | |
| 1229 | === modified file 'lib/lp/code/model/tests/test_codereviewvote.py' |
| 1230 | --- lib/lp/code/model/tests/test_codereviewvote.py 2009-12-07 06:51:42 +0000 |
| 1231 | +++ lib/lp/code/model/tests/test_codereviewvote.py 2010-01-13 10:10:29 +0000 |
| 1232 | @@ -9,7 +9,8 @@ |
| 1233 | from canonical.testing import DatabaseFunctionalLayer |
| 1234 | |
| 1235 | from lp.code.enums import CodeReviewVote |
| 1236 | -from lp.code.errors import ClaimReviewFailed, ReviewNotPending |
| 1237 | +from lp.code.errors import ( |
| 1238 | + ClaimReviewFailed, ReviewNotPending, UserHasExistingReview) |
| 1239 | from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference |
| 1240 | from lp.testing import login_person, TestCaseWithFactory |
| 1241 | |
| 1242 | @@ -43,7 +44,7 @@ |
| 1243 | TestCaseWithFactory.setUp(self) |
| 1244 | # Setup the proposal, claimant and team reviewer. |
| 1245 | self.bmp = self.factory.makeBranchMergeProposal() |
| 1246 | - self.claimant = self.factory.makePerson() |
| 1247 | + self.claimant = self.factory.makePerson(name='eric') |
| 1248 | self.review_team = self.factory.makeTeam() |
| 1249 | |
| 1250 | def _addPendingReview(self): |
| 1251 | @@ -71,8 +72,10 @@ |
| 1252 | vote=CodeReviewVote.APPROVE) |
| 1253 | review = self._addPendingReview() |
| 1254 | self._addClaimantToReviewTeam() |
| 1255 | - self.assertRaises( |
| 1256 | - ClaimReviewFailed, review.claimReview, self.claimant) |
| 1257 | + self.assertRaisesWithContent( |
| 1258 | + UserHasExistingReview, |
| 1259 | + 'Eric (eric) has already reviewed this', |
| 1260 | + review.claimReview, self.claimant) |
| 1261 | |
| 1262 | def test_personal_pending_review(self): |
| 1263 | # If the claimant has a pending review already, then they can't claim |
| 1264 | @@ -83,8 +86,10 @@ |
| 1265 | self.bmp.nominateReviewer( |
| 1266 | reviewer=self.claimant, registrant=self.bmp.registrant) |
| 1267 | login_person(self.claimant) |
| 1268 | - self.assertRaises( |
| 1269 | - ClaimReviewFailed, review.claimReview, self.claimant) |
| 1270 | + self.assertRaisesWithContent( |
| 1271 | + UserHasExistingReview, |
| 1272 | + 'Eric (eric) has already been asked to review this', |
| 1273 | + review.claimReview, self.claimant) |
| 1274 | |
| 1275 | def test_personal_not_in_review_team(self): |
| 1276 | # If the claimant is not in the review team, an error is raised. |
| 1277 | @@ -183,5 +188,75 @@ |
| 1278 | self.assertRaises(ReviewNotPending, review.delete) |
| 1279 | |
| 1280 | |
| 1281 | +class TestCodeReviewVoteReferenceReassignReview(TestCaseWithFactory): |
| 1282 | + """Tests for CodeReviewVoteReference.reassignReview.""" |
| 1283 | + |
| 1284 | + layer = DatabaseFunctionalLayer |
| 1285 | + |
| 1286 | + def makeMergeProposalWithReview(self, completed=False): |
| 1287 | + """Return a new merge proposal with a review.""" |
| 1288 | + bmp = self.factory.makeBranchMergeProposal() |
| 1289 | + reviewer = self.factory.makePerson() |
| 1290 | + if completed: |
| 1291 | + login_person(reviewer) |
| 1292 | + bmp.createComment( |
| 1293 | + reviewer, 'Message subject', 'Message content', |
| 1294 | + vote=CodeReviewVote.APPROVE) |
| 1295 | + [review] = list(bmp.votes) |
| 1296 | + else: |
| 1297 | + login_person(bmp.registrant) |
| 1298 | + review = bmp.nominateReviewer( |
| 1299 | + reviewer=reviewer, registrant=bmp.registrant) |
| 1300 | + return bmp, review |
| 1301 | + |
| 1302 | + def test_reassign_pending(self): |
| 1303 | + # A pending review can be reassigned to someone else. |
| 1304 | + bmp, review = self.makeMergeProposalWithReview() |
| 1305 | + new_reviewer = self.factory.makePerson() |
| 1306 | + review.reassignReview(new_reviewer) |
| 1307 | + self.assertEqual(new_reviewer, review.reviewer) |
| 1308 | + |
| 1309 | + def test_reassign_completed_review(self): |
| 1310 | + # A completed review cannot be reassigned |
| 1311 | + bmp, review = self.makeMergeProposalWithReview(completed=True) |
| 1312 | + self.assertRaises( |
| 1313 | + ReviewNotPending, review.reassignReview, bmp.registrant) |
| 1314 | + |
| 1315 | + def test_reassign_to_user_existing_pending(self): |
| 1316 | + # If a user has an existing pending review, they cannot have another |
| 1317 | + # pending review assigned to them. |
| 1318 | + bmp, review = self.makeMergeProposalWithReview() |
| 1319 | + reviewer = self.factory.makePerson(name='eric') |
| 1320 | + user_review = bmp.nominateReviewer( |
| 1321 | + reviewer=reviewer, registrant=bmp.registrant) |
| 1322 | + self.assertRaisesWithContent( |
| 1323 | + UserHasExistingReview, |
| 1324 | + 'Eric (eric) has already been asked to review this', |
| 1325 | + review.reassignReview, reviewer) |
| 1326 | + |
| 1327 | + def test_reassign_to_user_existing_completed(self): |
| 1328 | + # If a user has an existing completed review, they cannot have another |
| 1329 | + # pending review assigned to them. |
| 1330 | + bmp, review = self.makeMergeProposalWithReview() |
| 1331 | + reviewer = self.factory.makePerson(name='eric') |
| 1332 | + bmp.createComment( |
| 1333 | + reviewer, 'Message subject', 'Message content', |
| 1334 | + vote=CodeReviewVote.APPROVE) |
| 1335 | + self.assertRaisesWithContent( |
| 1336 | + UserHasExistingReview, |
| 1337 | + 'Eric (eric) has already reviewed this', |
| 1338 | + review.reassignReview, reviewer) |
| 1339 | + |
| 1340 | + def test_reassign_to_team_existing(self): |
| 1341 | + # If a team has an existing review, they can have another pending |
| 1342 | + # review assigned to them. |
| 1343 | + bmp, review = self.makeMergeProposalWithReview() |
| 1344 | + reviewer_team = self.factory.makeTeam() |
| 1345 | + team_review = bmp.nominateReviewer( |
| 1346 | + reviewer=reviewer_team, registrant=bmp.registrant) |
| 1347 | + review.reassignReview(reviewer_team) |
| 1348 | + self.assertEqual(reviewer_team, review.reviewer) |
| 1349 | + |
| 1350 | + |
| 1351 | def test_suite(): |
| 1352 | return TestLoader().loadTestsFromName(__name__) |
| 1353 | |
| 1354 | === renamed file 'lib/lp/code/stories/branches/xx-branch-merge-proposals.txt' => 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt' |
| 1355 | --- lib/lp/code/stories/branches/xx-branch-merge-proposals.txt 2010-01-07 21:02:00 +0000 |
| 1356 | +++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-01-13 10:10:29 +0000 |
| 1357 | @@ -233,15 +233,23 @@ |
| 1358 | The claimant can reassign the review to someone else. |
| 1359 | |
| 1360 | >>> foobar_browser.getLink('Reassign').click() |
| 1361 | + >>> foobar_browser.getControl('Reviewer').value = 'no-priv' |
| 1362 | + >>> foobar_browser.getControl('Reassign').click() |
| 1363 | + |
| 1364 | +If the person already has a review, the user gets an error... |
| 1365 | + |
| 1366 | + >>> print_feedback_messages(foobar_browser.contents) |
| 1367 | + There is 1 error. |
| 1368 | + No Privileges Person (no-priv) has already reviewed this |
| 1369 | + |
| 1370 | +... if not, the review is reassigned. |
| 1371 | + |
| 1372 | >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team' |
| 1373 | >>> foobar_browser.getControl('Reassign').click() |
| 1374 | - >>> foobar_browser.open(klingon_proposal) |
| 1375 | - >>> pending = find_tag_by_id( |
| 1376 | - ... foobar_browser.contents, 'code-review-votes') |
| 1377 | |
| 1378 | The review is now reassigned to the HWDB team. |
| 1379 | |
| 1380 | - >>> print extract_text(pending) |
| 1381 | + >>> print_tag_with_id(foobar_browser.contents, 'code-review-votes') |
| 1382 | Reviewer Review Type Date Requested Status... |
| 1383 | HWDB Team claimable ... ago Pending ... |
| 1384 | |
| 1385 | @@ -294,8 +302,7 @@ |
| 1386 | >>> sample_browser.getControl('Merged Revision Number').value = '42' |
| 1387 | >>> sample_browser.getControl('Mark as Merged').click() |
| 1388 | |
| 1389 | - >>> for message in get_feedback_messages(sample_browser.contents): |
| 1390 | - ... print extract_text(message) |
| 1391 | + >>> print_feedback_messages(sample_browser.contents) |
| 1392 | The proposal's merged revision has been updated. |
| 1393 | >>> print_summary(sample_browser) |
| 1394 | Status: Merged |
| 1395 | @@ -436,8 +443,7 @@ |
| 1396 | setting it gives an appropriate error. |
| 1397 | |
| 1398 | >>> nopriv_browser.getControl('Propose Merge').click() |
| 1399 | - >>> for message in get_feedback_messages(nopriv_browser.contents): |
| 1400 | - ... print extract_text(message) |
| 1401 | + >>> print_feedback_messages(nopriv_browser.contents) |
| 1402 | There is 1 error. |
| 1403 | Required input is missing. |
| 1404 | |
| 1405 | @@ -447,8 +453,7 @@ |
| 1406 | ... name='field.target_branch.target_branch').value = ( |
| 1407 | ... 'fooix') |
| 1408 | >>> nopriv_browser.getControl('Propose Merge').click() |
| 1409 | - >>> for message in get_feedback_messages(nopriv_browser.contents): |
| 1410 | - ... print extract_text(message) |
| 1411 | + >>> print_feedback_messages(nopriv_browser.contents) |
| 1412 | There is 1 error. |
| 1413 | Invalid value |
| 1414 | |
| 1415 | |
| 1416 | === modified file 'lib/lp/soyuz/interfaces/buildqueue.py' |
| 1417 | --- lib/lp/soyuz/interfaces/buildqueue.py 2010-01-07 04:07:09 +0000 |
| 1418 | +++ lib/lp/soyuz/interfaces/buildqueue.py 2010-01-13 10:10:29 +0000 |
| 1419 | @@ -13,7 +13,7 @@ |
| 1420 | ] |
| 1421 | |
| 1422 | from zope.interface import Interface, Attribute |
| 1423 | -from zope.schema import Choice, Datetime, Field, Timedelta |
| 1424 | +from zope.schema import Bool, Choice, Datetime, Field, Int, Text, Timedelta |
| 1425 | |
| 1426 | from lazr.restful.fields import Reference |
| 1427 | |
| 1428 | @@ -21,6 +21,8 @@ |
| 1429 | from lp.buildmaster.interfaces.buildfarmjob import ( |
| 1430 | IBuildFarmJob, BuildFarmJobType) |
| 1431 | from lp.services.job.interfaces.job import IJob |
| 1432 | +from lp.soyuz.interfaces.builder import IBuilder |
| 1433 | +from lp.soyuz.interfaces.processor import IProcessor |
| 1434 | |
| 1435 | |
| 1436 | class IBuildQueue(Interface): |
| 1437 | @@ -38,10 +40,21 @@ |
| 1438 | """ |
| 1439 | |
| 1440 | id = Attribute("Job identifier") |
| 1441 | - builder = Attribute("The IBuilder instance processing this job") |
| 1442 | - logtail = Attribute("The current tail of the log of the build") |
| 1443 | - lastscore = Attribute("Last score to be computed for this job") |
| 1444 | - manual = Attribute("Whether or not the job was manually scored") |
| 1445 | + builder = Reference( |
| 1446 | + IBuilder, title=_("Builder"), required=True, readonly=True, |
| 1447 | + description=_("The IBuilder instance processing this job")) |
| 1448 | + logtail = Text( |
| 1449 | + description=_("The current tail of the log of the job")) |
| 1450 | + lastscore = Int(description=_("This job's score.")) |
| 1451 | + manual = Bool( |
| 1452 | + description=_("Whether or not the job was manually scored.")) |
| 1453 | + processor = Reference( |
| 1454 | + IProcessor, title=_("Processor"), required=False, readonly=True, |
| 1455 | + description=_("The processor required by this build farm job.")) |
| 1456 | + virtualized = Bool( |
| 1457 | + required=False, |
| 1458 | + description=_( |
| 1459 | + "The virtualization setting required by this build farm job.")) |
| 1460 | |
| 1461 | job = Reference( |
| 1462 | IJob, title=_("Job"), required=True, readonly=True, |
| 1463 | |
| 1464 | === modified file 'lib/lp/soyuz/model/build.py' |
| 1465 | --- lib/lp/soyuz/model/build.py 2010-01-11 23:48:25 +0000 |
| 1466 | +++ lib/lp/soyuz/model/build.py 2010-01-13 10:10:29 +0000 |
| 1467 | @@ -682,7 +682,8 @@ |
| 1468 | queue_entry = BuildQueue( |
| 1469 | estimated_duration=duration_estimate, |
| 1470 | job_type=BuildFarmJobType.PACKAGEBUILD, |
| 1471 | - job=job.id) |
| 1472 | + job=job.id, processor=self.processor, |
| 1473 | + virtualized=self.is_virtualized) |
| 1474 | store.add(queue_entry) |
| 1475 | return queue_entry |
| 1476 | |
| 1477 | |
| 1478 | === modified file 'lib/lp/soyuz/model/buildqueue.py' |
| 1479 | --- lib/lp/soyuz/model/buildqueue.py 2010-01-11 23:48:25 +0000 |
| 1480 | +++ lib/lp/soyuz/model/buildqueue.py 2010-01-13 10:10:29 +0000 |
| 1481 | @@ -52,6 +52,9 @@ |
| 1482 | lastscore = IntCol(dbName='lastscore', default=0) |
| 1483 | manual = BoolCol(dbName='manual', default=False) |
| 1484 | estimated_duration = IntervalCol() |
| 1485 | + processor = ForeignKey( |
| 1486 | + dbName='processor', foreignKey='Processor', notNull=True) |
| 1487 | + virtualized = BoolCol(dbName='virtualized') |
| 1488 | |
| 1489 | @property |
| 1490 | def required_build_behavior(self): |
| 1491 | |
| 1492 | === modified file 'lib/lp/soyuz/tests/test_buildqueue.py' |
| 1493 | --- lib/lp/soyuz/tests/test_buildqueue.py 2010-01-11 23:43:59 +0000 |
| 1494 | +++ lib/lp/soyuz/tests/test_buildqueue.py 2010-01-13 10:10:29 +0000 |
| 1495 | @@ -740,3 +740,50 @@ |
| 1496 | self.assertEqual( |
| 1497 | bq.specific_job_classes[BuildFarmJobType.BRANCHBUILD], |
| 1498 | FakeBranchBuild) |
| 1499 | + |
| 1500 | + |
| 1501 | +class TestPlatformData(TestCaseWithFactory): |
| 1502 | + """Tests covering the processor/virtualized properties.""" |
| 1503 | + |
| 1504 | + layer = LaunchpadZopelessLayer |
| 1505 | + |
| 1506 | + def setUp(self): |
| 1507 | + """Set up a native x86 build for the test archive.""" |
| 1508 | + super(TestPlatformData, self).setUp() |
| 1509 | + |
| 1510 | + self.publisher = SoyuzTestPublisher() |
| 1511 | + self.publisher.prepareBreezyAutotest() |
| 1512 | + |
| 1513 | + # First mark all builds in the sample data as already built. |
| 1514 | + store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR) |
| 1515 | + sample_data = store.find(Build) |
| 1516 | + for build in sample_data: |
| 1517 | + build.buildstate = BuildStatus.FULLYBUILT |
| 1518 | + store.flush() |
| 1519 | + |
| 1520 | + # We test builds that target a primary archive. |
| 1521 | + self.non_ppa = self.factory.makeArchive( |
| 1522 | + name="primary", purpose=ArchivePurpose.PRIMARY) |
| 1523 | + self.non_ppa.require_virtualized = False |
| 1524 | + |
| 1525 | + self.builds = [] |
| 1526 | + self.builds.extend( |
| 1527 | + self.publisher.getPubSource( |
| 1528 | + sourcename="gedit", status=PackagePublishingStatus.PUBLISHED, |
| 1529 | + archive=self.non_ppa).createMissingBuilds()) |
| 1530 | + |
| 1531 | + def test_JobPlatformSettings(self): |
| 1532 | + """The `BuildQueue` instance shares the processor/virtualized |
| 1533 | + properties with the associated `Build`.""" |
| 1534 | + build, bq = find_job(self, 'gedit') |
| 1535 | + |
| 1536 | + # Make sure the 'processor' properties are the same. |
| 1537 | + self.assertEqual( |
| 1538 | + bq.processor, build.processor, |
| 1539 | + "The 'processor' property deviates.") |
| 1540 | + |
| 1541 | + # Make sure the 'virtualized' properties are the same. |
| 1542 | + self.assertEqual( |
| 1543 | + bq.virtualized, build.is_virtualized, |
| 1544 | + "The 'virtualized' property deviates.") |
| 1545 | + |
| 1546 | |
| 1547 | === modified file 'lib/lp/testing/__init__.py' |
| 1548 | --- lib/lp/testing/__init__.py 2009-12-22 23:50:27 +0000 |
| 1549 | +++ lib/lp/testing/__init__.py 2010-01-13 10:10:29 +0000 |
| 1550 | @@ -537,7 +537,7 @@ |
| 1551 | return BzrDir.create_branch_convenience( |
| 1552 | branch_url, format=format) |
| 1553 | |
| 1554 | - def create_branch_and_tree(self, tree_location='.', product=None, |
| 1555 | + def create_branch_and_tree(self, tree_location=None, product=None, |
| 1556 | hosted=False, db_branch=None, format=None, |
| 1557 | **kwargs): |
| 1558 | """Create a database branch, bzr branch and bzr checkout. |
| 1559 | @@ -562,6 +562,9 @@ |
| 1560 | if self.real_bzr_server: |
| 1561 | transaction.commit() |
| 1562 | bzr_branch = self.createBranchAtURL(branch_url, format=format) |
| 1563 | + if tree_location is None: |
| 1564 | + tree_location = tempfile.mkdtemp() |
| 1565 | + self.addCleanup(lambda: shutil.rmtree(tree_location)) |
| 1566 | return db_branch, bzr_branch.create_checkout( |
| 1567 | tree_location, lightweight=True) |
| 1568 | |
| 1569 | |
| 1570 | === modified file 'scripts/librarian-report.py' |
| 1571 | --- scripts/librarian-report.py 2009-12-22 02:25:12 +0000 |
| 1572 | +++ scripts/librarian-report.py 2010-01-13 10:10:29 +0000 |
| 1573 | @@ -71,7 +71,7 @@ |
| 1574 | cur.execute(""" |
| 1575 | SELECT |
| 1576 | COALESCE(SUM(filesize), 0), |
| 1577 | - pg_size_pretty(COALESCE(SUM(filesize), 0)), |
| 1578 | + pg_size_pretty(CAST(COALESCE(SUM(filesize), 0) AS bigint)), |
| 1579 | COUNT(*) |
| 1580 | FROM ( |
| 1581 | SELECT DISTINCT ON (LFC.id) LFC.id, LFC.filesize |
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.