launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bugsummary-v2-rebuild-no-fixed_upstream into lp:launchpad.

Requested reviews:
  William Grant (wgrant): code
Related bugs:
  Bug #950502 in Launchpad itself: ""Bugs fixed elsewhere" count is expensive and of dubious utility"
  https://bugs.launchpad.net/launchpad/+bug/950502

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bugsummary-v2-rebuild-no-fixed_upstream/+merge/114550

We've run bugsummary-rebuild on production to eliminate rows with fixed_upstream IS DISTINCT FROM false, and we want to drop the column soonish. This branch removes the last uses of it from the code.
-- 
https://code.launchpad.net/~wgrant/launchpad/bugsummary-v2-rebuild-no-fixed_upstream/+merge/114550
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/bugs/scripts/bugsummaryrebuild.py'
--- lib/lp/bugs/scripts/bugsummaryrebuild.py	2012-06-28 01:14:33 +0000
+++ lib/lp/bugs/scripts/bugsummaryrebuild.py	2012-07-12 01:36:25 +0000
@@ -46,13 +46,10 @@
 class RawBugSummary(BugSummary):
     """Like BugSummary, except based on the raw DB table.
 
-    BugSummary is actually based on the combinedbugsummary view, and it omits
-    fixed_upstream, a column that's being removed.
+    BugSummary is actually based on the combinedbugsummary view.
     """
     __storm_table__ = 'bugsummary'
 
-    fixed_upstream = Bool()
-
 
 class BugSummaryJournal(BugSummary):
     """Just the necessary columns of BugSummaryJournal."""
@@ -142,8 +139,7 @@
     """
     return IStore(RawBugSummary).find(
         (RawBugSummary.status, RawBugSummary.milestone_id,
-         RawBugSummary.importance, RawBugSummary.has_patch,
-         RawBugSummary.fixed_upstream, RawBugSummary.tag,
+         RawBugSummary.importance, RawBugSummary.has_patch, RawBugSummary.tag,
          RawBugSummary.viewed_by_id, RawBugSummary.count),
         *get_bugsummary_constraint(target))
 
@@ -195,8 +191,7 @@
     key_cols = (
         RawBugSummary.status, RawBugSummary.milestone_id,
         RawBugSummary.importance, RawBugSummary.has_patch,
-        RawBugSummary.fixed_upstream, RawBugSummary.tag,
-        RawBugSummary.viewed_by_id)
+        RawBugSummary.tag, RawBugSummary.viewed_by_id)
 
     # Postgres doesn't do bulk updates, so do a delete+add.
     for key, count in updated.iteritems():
@@ -278,8 +273,7 @@
     # (status, milestone, importance, has_patch, tag, viewed_by) rows.
     common_cols = (
         RelevantTask.status, RelevantTask.milestone_id,
-        RelevantTask.importance, RelevantTask.has_patch,
-        Alias(False, 'fixed_upstream'))
+        RelevantTask.importance, RelevantTask.has_patch)
     null_tag = Alias(Cast(None, 'text'), 'tag')
     null_viewed_by = Alias(Cast(None, 'integer'), 'viewed_by')
 
@@ -317,8 +311,7 @@
     proto_key_cols = (
         BugSummaryPrototype.status, BugSummaryPrototype.milestone_id,
         BugSummaryPrototype.importance, BugSummaryPrototype.has_patch,
-        BugSummaryPrototype.fixed_upstream, BugSummaryPrototype.tag,
-        BugSummaryPrototype.viewed_by_id)
+        BugSummaryPrototype.tag, BugSummaryPrototype.viewed_by_id)
     origin = IStore(BugTaskFlat).with_(relevant_tasks).using(
         Alias(unions, 'bugsummary_prototype'))
     results = origin.find(proto_key_cols + (Count(),))

=== modified file 'lib/lp/bugs/scripts/tests/test_bugsummaryrebuild.py'
--- lib/lp/bugs/scripts/tests/test_bugsummaryrebuild.py	2012-06-28 01:14:33 +0000
+++ lib/lp/bugs/scripts/tests/test_bugsummaryrebuild.py	2012-07-12 01:36:25 +0000
@@ -3,8 +3,6 @@
 
 __metaclass__ = type
 
-import subprocess
-
 from testtools.content import text_content
 from testtools.matchers import MatchesRegex
 import transaction
@@ -109,22 +107,22 @@
         with dbuser('bugsummaryrebuild'):
             apply_bugsummary_changes(
                 product,
-                {(NEW, None, HIGH, False, False, None, None): 2,
-                (TRIAGED, None, LOW, False, False, None, None): 4},
+                {(NEW, None, HIGH, False, None, None): 2,
+                (TRIAGED, None, LOW, False, None, None): 4},
                 {}, [])
         self.assertContentEqual(
-            [(NEW, None, HIGH, False, False, None, None, 2),
-             (TRIAGED, None, LOW, False, False, None, None, 4)],
+            [(NEW, None, HIGH, False, None, None, 2),
+             (TRIAGED, None, LOW, False, None, None, 4)],
             get_bugsummary_rows(product))
 
         # Delete one, mutate the other.
         with dbuser('bugsummaryrebuild'):
             apply_bugsummary_changes(
                 product,
-                {}, {(NEW, None, HIGH, False, False, None, None): 3},
-                [(TRIAGED, None, LOW, False, False, None, None)])
+                {}, {(NEW, None, HIGH, False, None, None): 3},
+                [(TRIAGED, None, LOW, False, None, None)])
         self.assertContentEqual(
-            [(NEW, None, HIGH, False, False, None, None, 3)],
+            [(NEW, None, HIGH, False, None, None, 3)],
             get_bugsummary_rows(product))
 
     def test_rebuild_bugsummary_for_target(self):
@@ -174,8 +172,7 @@
         rollup_journal()
         new_rows = set(get_bugsummary_rows(product))
         self.assertContentEqual(
-            [(task.status, None, task.importance, False, False, None, None,
-              1)],
+            [(task.status, None, task.importance, False, None, None, 1)],
             new_rows - orig_rows)
 
 
@@ -189,7 +186,7 @@
         product = self.factory.makeProduct()
         bug = self.factory.makeBug(product=product).default_bugtask
         self.assertContentEqual(
-            [(bug.status, None, bug.importance, False, False, None, None, 1)],
+            [(bug.status, None, bug.importance, False, None, None, 1)],
             calculate_bugsummary_rows(product))
 
     def test_public_tagged(self):
@@ -199,9 +196,9 @@
         bug = self.factory.makeBug(
             product=product, tags=[u'foo', u'bar']).default_bugtask
         self.assertContentEqual(
-            [(bug.status, None, bug.importance, False, False, None, None, 1),
-             (bug.status, None, bug.importance, False, False, u'foo', None, 1),
-             (bug.status, None, bug.importance, False, False, u'bar', None, 1),
+            [(bug.status, None, bug.importance, False, None, None, 1),
+             (bug.status, None, bug.importance, False, u'foo', None, 1),
+             (bug.status, None, bug.importance, False, u'bar', None, 1),
             ], calculate_bugsummary_rows(product))
 
     def test_private_untagged(self):
@@ -213,8 +210,7 @@
             product=product, owner=owner,
             information_type=InformationType.USERDATA).default_bugtask
         self.assertContentEqual(
-            [(bug.status, None, bug.importance, False, False, None, owner.id,
-              1)],
+            [(bug.status, None, bug.importance, False, None, owner.id, 1)],
             calculate_bugsummary_rows(product))
 
     def test_private_tagged(self):
@@ -226,12 +222,9 @@
             product=product, owner=owner, tags=[u'foo', u'bar'],
             information_type=InformationType.USERDATA).default_bugtask
         self.assertContentEqual(
-            [(bug.status, None, bug.importance, False, False, None,
-              owner.id, 1),
-             (bug.status, None, bug.importance, False, False, u'foo',
-              owner.id, 1),
-             (bug.status, None, bug.importance, False, False, u'bar',
-              owner.id, 1)],
+            [(bug.status, None, bug.importance, False, None, owner.id, 1),
+             (bug.status, None, bug.importance, False, u'foo', owner.id, 1),
+             (bug.status, None, bug.importance, False, u'bar', owner.id, 1)],
             calculate_bugsummary_rows(product))
 
     def test_aggregation(self):
@@ -243,9 +236,9 @@
         bug3 = self.factory.makeBug(
             product=product, status=BugTaskStatus.TRIAGED).default_bugtask
         self.assertContentEqual(
-            [(bug1.status, None, bug1.importance, False, False, None, None, 2),
-             (bug3.status, None, bug3.importance, False, False, None, None, 1),
-            ], calculate_bugsummary_rows(product))
+            [(bug1.status, None, bug1.importance, False, None, None, 2),
+             (bug3.status, None, bug3.importance, False, None, None, 1)],
+            calculate_bugsummary_rows(product))
 
     def test_has_patch(self):
         # Bugs with a patch attachment (latest_patch_uploaded is not
@@ -256,9 +249,8 @@
         bug2 = self.factory.makeBug(
             product=product, status=BugTaskStatus.TRIAGED).default_bugtask
         self.assertContentEqual(
-            [(bug1.status, None, bug1.importance, True, False, None, None, 1),
-             (bug2.status, None, bug2.importance, False, False, None, None,
-              1)],
+            [(bug1.status, None, bug1.importance, True, None, None, 1),
+             (bug2.status, None, bug2.importance, False, None, None, 1)],
             calculate_bugsummary_rows(product))
 
     def test_milestone(self):
@@ -272,10 +264,8 @@
             product=product, milestone=mile2,
             status=BugTaskStatus.TRIAGED).default_bugtask
         self.assertContentEqual(
-            [(bug1.status, mile1.id, bug1.importance, False, False, None,
-              None, 1),
-             (bug2.status, mile2.id, bug2.importance, False, False, None,
-              None, 1)],
+            [(bug1.status, mile1.id, bug1.importance, False, None, None, 1),
+             (bug2.status, mile2.id, bug2.importance, False, None, None, 1)],
             calculate_bugsummary_rows(product))
 
     def test_distribution_includes_packages(self):
@@ -292,8 +282,7 @@
         # The DistributionSourcePackage task shows up in the
         # Distribution's rows.
         self.assertContentEqual(
-            [(bug1.status, None, bug1.importance, False, False, None, None,
-              1)],
+            [(bug1.status, None, bug1.importance, False, None, None, 1)],
             calculate_bugsummary_rows(dsp.distribution))
         self.assertContentEqual(
             calculate_bugsummary_rows(dsp.distribution),
@@ -301,8 +290,7 @@
 
         # The SourcePackage task shows up in the DistroSeries' rows.
         self.assertContentEqual(
-            [(bug2.status, None, bug2.importance, False, False, None, None,
-              1)],
+            [(bug2.status, None, bug2.importance, False, None, None, 1)],
             calculate_bugsummary_rows(sp.distroseries))
         self.assertContentEqual(
             calculate_bugsummary_rows(sp.distroseries),