launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/convert-sql-627631 into lp:launchpad with lp:~jcsackett/launchpad/migrate-official-bool-data-627631 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #627631 Queries need to be updated to use usage enums, where possible
  https://bugs.launchpad.net/bugs/627631


Summary
=======

There's an ongoing effort to phase out the official_* booleans which indicate how a product/distribution uses the various service on Launchpad. While previous branches cleaned up a lot of model code, they didn't address queries made to the db that used the Boolean columns.

This branch rectifies that by cleaning up those sections of code to reference the EnumCol, so that we can (if we choose to) drop the official_* code completely.

Proposed fix
============

Replace locations making SqlObject or Storm queries and update them to use the EnumCol and ServiceUsage DBItem.

Preimplementation talk
======================

Spoke with Curtis Hovey (several weeks ago).

Implementation details
======================

As in proposed.

Tests
=====

Because of the number of things that end up effected, most modules need testing. Testing registry alone is a reasonable proxy, but not ideal.

bin/test -m lp.registry.tests

OR

bin/test -m lp.registry
bin/test -m lp.translations
bin/test -m lp.answers
bin/test -t update_stats

Demo & QA
=========

Operation of the various main pages (e.g. +translations, +questions) for a product or distribution shouldn't fail in anyway or appear different; b/c of prior work this change should be invisible.

Lint
====

make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/database/launchpadstatistic.py
  lib/lp/answers/model/question.py
  lib/lp/registry/model/product.py
  lib/lp/registry/model/projectgroup.py
  lib/lp/translations/model/translationsoverview.py
  lib/lp/translations/model/translationsperson.py
  lib/lp/translations/scripts/translations_to_branch.py


-- 
https://code.launchpad.net/~jcsackett/launchpad/convert-sql-627631/+merge/40021
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/convert-sql-627631 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/database/launchpadstatistic.py'
--- lib/canonical/launchpad/database/launchpadstatistic.py	2010-10-03 15:30:06 +0000
+++ lib/canonical/launchpad/database/launchpadstatistic.py	2010-11-03 20:51:30 +0000
@@ -32,6 +32,7 @@
     QuestionStatus,
     )
 from lp.answers.model.question import Question
+from lp.app.enums import ServiceUsage
 from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugtask import BugTask
 from lp.registry.model.product import Product
@@ -165,7 +166,7 @@
     def _updateRosettaStatistics(self, ztm):
         self.update(
                 'products_using_rosetta',
-                Product.selectBy(official_rosetta=True).count()
+                Product.selectBy(translations_usage=ServiceUsage.LAUNCHPAD).count()
                 )
         self.update('potemplate_count', POTemplate.select().count())
         ztm.commit()

=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py	2010-10-03 15:30:06 +0000
+++ lib/lp/answers/model/question.py	2010-11-03 20:51:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212
@@ -88,6 +88,7 @@
 from lp.answers.model.answercontact import AnswerContact
 from lp.answers.model.questionmessage import QuestionMessage
 from lp.answers.model.questionsubscription import QuestionSubscription
+from lp.app.enums import ServiceUsage
 from lp.bugs.interfaces.buglink import IBugLinkTarget
 from lp.bugs.interfaces.bugtask import BugTaskStatus
 from lp.bugs.model.buglinktarget import BugLinkTargetMixin
@@ -680,8 +681,8 @@
                     LEFT OUTER JOIN Distribution ON (
                         Question.distribution = Distribution.id)
                 WHERE
-                    (Product.official_answers is True
-                    OR Distribution.official_answers is TRUE)
+                    (Product.answers_usage = %s 
+                    OR Distribution.answers_usage = %s)
                     AND Question.datecreated > (
                         current_timestamp -interval '60 days')
                 LIMIT 5000
@@ -689,7 +690,8 @@
             GROUP BY product, distribution
             ORDER BY question_count DESC
             LIMIT %s
-            """ % sqlvalues(limit))
+            """ % sqlvalues(
+                    ServiceUsage.LAUNCHPAD, ServiceUsage.LAUNCHPAD, limit))
 
         projects = []
         product_set = getUtility(IProductSet)

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-10-28 16:14:00 +0000
+++ lib/lp/registry/model/product.py	2010-11-03 20:51:30 +0000
@@ -1592,7 +1592,7 @@
             Product.active == True,
             Product.id == ProductSeries.productID,
             POTemplate.productseriesID == ProductSeries.id,
-            Product.official_rosetta == True,
+            Product.translations_usage == ServiceUsage.LAUNCHPAD,
             Person.id == Product._ownerID,
             ).config(distinct=True).order_by(Product.title)
 
@@ -1612,12 +1612,12 @@
                         ProductSeries.Product = Product.id
                     JOIN POTemplate ON
                         POTemplate.productseries = ProductSeries.id
-                    WHERE Product.active AND Product.official_rosetta
+                    WHERE Product.active AND Product.translations_usage = %s
                     ORDER BY place
                 ) AS randomized_products
                 LIMIT %s
             )
-            ''' % quote(maximumproducts),
+            ''' % quote(ServiceUsage.LAUNCHPAD, maximumproducts),
             distinct=True,
             orderBy='Product.title')
 

=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py	2010-10-18 13:04:50 +0000
+++ lib/lp/registry/model/projectgroup.py	2010-11-03 20:51:30 +0000
@@ -211,17 +211,12 @@
 
     def translatables(self):
         """See `IProjectGroup`."""
-        # XXX j.c.sackett 2010-08-30 bug=627631: Once data migration has
-        # happened for the usage enums, this sql needs to be updated to
-        # check for the translations_usage, not official_rosetta.  At that
-        # time it should also be converted to a Storm query and the issue with
-        # has_translatables resolved.
         return Product.select('''
             Product.project = %s AND
-            Product.official_rosetta = TRUE AND
+            Product.translations_usage = %s AND
             Product.id = ProductSeries.product AND
             POTemplate.productseries = ProductSeries.id
-            ''' % sqlvalues(self),
+            ''' % sqlvalues(self, ServiceUsage.LAUNCHPAD),
             clauseTables=['ProductSeries', 'POTemplate'],
             distinct=True)
 

=== modified file 'lib/lp/translations/model/translationsoverview.py'
--- lib/lp/translations/model/translationsoverview.py	2010-08-31 23:03:45 +0000
+++ lib/lp/translations/model/translationsoverview.py	2010-11-03 20:51:30 +0000
@@ -7,6 +7,7 @@
 from zope.interface import implements
 
 from canonical.database.sqlbase import cursor
+from lp.app.enums import ServiceUsage
 from lp.registry.model.distribution import Distribution
 from lp.registry.model.product import Product
 from lp.translations.interfaces.translationsoverview import (
@@ -45,9 +46,6 @@
     def getMostTranslatedPillars(self, limit=50):
         """See `ITranslationsOverview`."""
 
-        # XXX j.c.sackett 2010-08-30 bug=627631 Once data migration has
-        # happened for the usage enums, this sql needs to be updated
-        # to check for the translations_usage, not official_rosetta.
         query = """
         SELECT LOWER(COALESCE(product_name, distro_name)) AS name,
                product_id,
@@ -67,14 +65,16 @@
                      distribution=distribution.id
               WHERE category=3 AND
                     (product IS NOT NULL OR distribution IS NOT NULL) AND
-                    (product.official_rosetta OR
-                        distribution.official_rosetta)
+                    (product.translations_usage = %s OR
+                        distribution.tranlsations_usage = %s)
               GROUP BY product.displayname, product.id,
                        distribution.displayname, distribution.id
               HAVING SUM(karmavalue) > 0
               ORDER BY total_karma DESC
               LIMIT %d) AS something
-          ORDER BY name""" % int(limit)
+          ORDER BY name""" % (ServiceUsage.LAUNCHPAD,
+                              ServiceUsage.LAUNCHPAD,
+                              int(limit))
         cur = cursor()
         cur.execute(query)
 

=== modified file 'lib/lp/translations/model/translationsperson.py'
--- lib/lp/translations/model/translationsperson.py	2010-08-31 23:03:45 +0000
+++ lib/lp/translations/model/translationsperson.py	2010-11-03 20:51:30 +0000
@@ -262,9 +262,6 @@
         The added joins may make the overall query non-distinct, so be
         sure to enforce distinctness.
         """
-        # XXX j.c.sackett 2010-08-30 bug=627631 Once data migration has
-        # happened for the usage enums, this query needs to be updated
-        # to check for the translations_usage, not official_rosetta.
 
         POTemplateJoin = Join(POTemplate, And(
             POTemplate.id == POFile.potemplateID,
@@ -282,7 +279,7 @@
         # translation focus.
         distrojoin_conditions = And(
             Distribution.id == DistroSeries.distributionID,
-            Distribution.official_rosetta == True,
+            Distribution.translations_usage == ServiceUsage.LAUNCHPAD,
             Distribution.translation_focusID == DistroSeries.id)
 
         DistroJoin = LeftJoin(Distribution, distrojoin_conditions)
@@ -291,7 +288,7 @@
             ProductSeries, ProductSeries.id == POTemplate.productseriesID)
         ProductJoin = LeftJoin(Product, And(
             Product.id == ProductSeries.productID,
-            Product.official_rosetta == True))
+            Product.translations_usage == ServiceUsage.LAUNCHPAD))
 
         ProjectJoin = LeftJoin(
             ProjectGroup, ProjectGroup.id == Product.projectID)

=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
--- lib/lp/translations/scripts/translations_to_branch.py	2010-10-02 11:41:43 +0000
+++ lib/lp/translations/scripts/translations_to_branch.py	2010-11-03 20:51:30 +0000
@@ -306,14 +306,12 @@
 
         self.store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
 
-        # XXX j.c.sackett 2010-08-30 bug=627631 Once data migration has
-        # happened for the usage enums, this sql needs to be updated to
-        # check for the translations_usage, not official_rosetta.
         product_join = Join(
             ProductSeries, Product, ProductSeries.product == Product.id)
         productseries = self.store.using(product_join).find(
             ProductSeries, SQL(
-                "official_rosetta AND translations_branch IS NOT NULL"))
+                "translations_usage = %s AND translations_branch IS NOT NULL"
+                % ServiceUsage.LAUNCHPAD))
 
         # Anything deterministic will do, and even that is only for
         # testing.