launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00583
[Merge] lp:~sinzui/launchpad/dsp-bug-counts-0 into lp:launchpad/devel
Curtis Hovey has proposed merging lp:~sinzui/launchpad/dsp-bug-counts-0 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This is my branch to fix DSP bug heat calculations.
lp:~sinzui/launchpad/dsp-bug-counts-0
Diff size: 146
Launchpad bug:
https://bugs.launchpad.net/bugs/613610
Test command: ./bin/test -vv -t test_bugheat
Pre-implementation: gmb, bac, jcsackett
Target release: 10.09
Fix DSP bug heat calculations
-----------------------------
+needspackaging should be showing the number of open bugs. It is showing the
total of all bugs for the package. A quick examination shows that most of the
packages list on the first page of results have no open bugs :(
DSP.recalculateBugHeatCache is counting all bugs to get
self.max_bug_heat, self.total_bug_heat, self.bug_count
I think this should only count open bugs. The original query that calculated
total_bug_heat and bug_count used
bugtask.status in UNRESOLVED_BUGTASK_STATUSES
in the query to ensure closed bugs did not taint the priorities.
Rules
-----
* Add a closed bu to the test
* Add a constraint to recalculateBugHeatCache:
bugtask.status.is_in(UNRESOLVED_BUGTASK_STATUSES)
QA
--
I expect this fix will also address part of the problem in the heuristics
that rank the packages that most need linking to a project. The current
10 ten have no open bugs; packages with open bugs have a greater need to be
linked so that bugs can be forwarded.
* Visit https://edge.launchpad.net/ubuntu/maverick/+needs-packaging
* Verify the counts of bugs show on on the page match the counts
of bugs on the dsp's bug page.
Lint
----
Linting changed files:
lib/lp/bugs/tests/test_bugheat.py
lib/lp/registry/model/distributionsourcepackage.py
lib/lp/testing/factory.py
Test
----
* lib/lp/bugs/tests/test_bugheat.py
* Added a closed bug to the test setup. It must not change the results
of the existing tests.
Implementation
--------------
* lib/lp/registry/model/distributionsourcepackage.py
* Updated the rules to recalculate the DSP bug heat to ignore closed
bugs.
* lib/lp/testing/factory.py
* The test runner was generating oops when bug_heat was accessed
reporting that the attribute should not be set before the
object had a db representation. I added an option to create a DSP
(which is normally arbitrary) with a db row--it uses
removeSecurityProxy because special privileges (a back end proc)
usually makes the db row.
--
https://code.launchpad.net/~sinzui/launchpad/dsp-bug-counts-0/+merge/32479
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/dsp-bug-counts-0 into lp:launchpad/devel.
=== modified file 'lib/lp/bugs/tests/test_bugheat.py'
--- lib/lp/bugs/tests/test_bugheat.py 2010-07-18 00:55:24 +0000
+++ lib/lp/bugs/tests/test_bugheat.py 2010-08-12 17:51:14 +0000
@@ -11,6 +11,7 @@
from canonical.testing import LaunchpadZopelessLayer
+from lp.bugs.interfaces.bugtask import BugTaskStatus
from lp.testing import TestCaseWithFactory
from lp.testing.factory import LaunchpadObjectFactory
@@ -110,9 +111,13 @@
def setUp(self):
TestCaseWithFactory.setUp(self)
- self.target = self.factory.makeDistributionSourcePackage()
+ self.target = self.factory.makeDistributionSourcePackage(with_db=True)
self.bugtask1 = self.factory.makeBugTask(target=self.target)
self.bugtask2 = self.factory.makeBugTask(target=self.target)
+ self.bugtask3 = self.factory.makeBugTask(target=self.target)
+ # A closed bug is not include in DSP bug heat calculations.
+ self.bugtask3.transitionToStatus(
+ BugTaskStatus.FIXRELEASED, self.target.distribution.owner)
# Bug heat gets calculated by complicated rules in a db
# stored procedure. We will override them here to avoid
# testing inconsitencies if those values are calculated
@@ -121,8 +126,10 @@
# automatically by bug.setHeat().
bug1 = self.bugtask1.bug
bug2 = self.bugtask2.bug
+ bug3 = self.bugtask3.bug
bug1.setHeat(7)
bug2.setHeat(19)
+ bug3.setHeat(11)
Store.of(bug1).flush()
self.max_heat = max(bug1.heat, bug2.heat)
self.total_heat = sum([bug1.heat, bug2.heat])
=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py 2010-07-07 16:53:36 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py 2010-08-12 17:51:14 +0000
@@ -33,6 +33,7 @@
from canonical.lazr.utils import smartquote
from lp.answers.interfaces.questiontarget import IQuestionTarget
from lp.bugs.interfaces.bugtarget import IHasBugHeat
+from lp.bugs.interfaces.bugtask import UNRESOLVED_BUGTASK_STATUSES
from lp.bugs.model.bug import Bug, BugSet, get_bug_tags_open_count
from lp.bugs.model.bugtarget import BugTargetBase, HasBugHeatMixin
from lp.bugs.model.bugtask import BugTask
@@ -56,6 +57,7 @@
from lp.translations.model.customlanguagecode import (
CustomLanguageCode, HasCustomLanguageCodesMixin)
+
def is_upstream_link_allowed(spph):
"""Metapackages shouldn't have upstream links.
@@ -67,6 +69,7 @@
class DistributionSourcePackageProperty:
+
def __init__(self, attrname):
self.attrname = attrname
@@ -90,8 +93,8 @@
SourcePackagePublishingHistory.sourcepackagereleaseID ==
SourcePackageRelease.id,
SourcePackageRelease.sourcepackagenameID ==
- obj.sourcepackagename.id
- ).order_by(Desc(SourcePackagePublishingHistory.id)).first()
+ obj.sourcepackagename.id).order_by(
+ Desc(SourcePackagePublishingHistory.id)).first()
obj._new(obj.distribution, obj.sourcepackagename,
is_upstream_link_allowed(spph))
setattr(obj._self_in_database, self.attrname, value)
@@ -185,7 +188,8 @@
(Max(Bug.heat), Sum(Bug.heat), Count(Bug.id)),
BugTask.bug == Bug.id,
BugTask.distributionID == self.distribution.id,
- BugTask.sourcepackagenameID == self.sourcepackagename.id).one()
+ BugTask.sourcepackagenameID == self.sourcepackagename.id,
+ BugTask.status.is_in(UNRESOLVED_BUGTASK_STATUSES)).one()
# Aggregate functions return NULL if zero rows match.
row = list(row)
@@ -329,8 +333,7 @@
# Next, the joins for the ordering by soyuz karma of the
# SPR creator.
KarmaTotalCache.person == SourcePackageRelease.creatorID,
- *extra_args
- )
+ *extra_args)
# Note: If and when we later have a field on IArchive to order by,
# such as IArchive.rank, we will then be able to return distinct
@@ -352,8 +355,7 @@
condition = And(
Packaging.sourcepackagename == self.sourcepackagename,
Packaging.distroseriesID == DistroSeries.id,
- DistroSeries.distribution == self.distribution
- )
+ DistroSeries.distribution == self.distribution)
result = store.find(Packaging, condition)
result.order_by("debversion_sort_key(version) DESC")
if result.count() == 0:
@@ -518,8 +520,7 @@
DistributionSourcePackageInDatabase.sourcepackagename ==
sourcepackagename,
DistributionSourcePackageInDatabase.distribution ==
- distribution
- ).one()
+ distribution).one()
@classmethod
def _new(cls, distribution, sourcepackagename,
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-08-06 10:48:49 +0000
+++ lib/lp/testing/factory.py 2010-08-12 17:51:14 +0000
@@ -2630,7 +2630,7 @@
SuiteSourcePackage(distroseries, pocket, sourcepackagename))
def makeDistributionSourcePackage(self, sourcepackagename=None,
- distribution=None):
+ distribution=None, with_db=False):
# Make sure we have a real sourcepackagename object.
if (sourcepackagename is None or
isinstance(sourcepackagename, basestring)):
@@ -2638,8 +2638,13 @@
sourcepackagename)
if distribution is None:
distribution = self.makeDistribution()
-
- return distribution.getSourcePackage(sourcepackagename)
+ package = distribution.getSourcePackage(sourcepackagename)
+ if with_db:
+ # Create an instance with a database record. That is normally
+ # done by secondary process.
+ removeSecurityProxy(package)._new(
+ distribution, sourcepackagename, False)
+ return package
def makeEmailMessage(self, body=None, sender=None, to=None,
attachments=None, encode_attachments=False):
