launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01939
[Merge] lp:~jcsackett/launchpad/convert-sql-627631-storm into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/convert-sql-627631-storm into lp:launchpad with lp:~jcsackett/launchpad/convert-sql-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.
This branch was split out separately from the one it depends on b/c it seemed that storm queries needed special handling, and were related to some other issues in the code base.
Proposed fix
============
Replace locations making Storm queries and update them to use the EnumCol and ServiceUsage DBItem.
Preimplementation talk
======================
Spoke with Curtis Hovey.
Implementation details
======================
As in proposed. Additionally, as low hanging fruit a query was converted to storm to fix a related issue in projectgroups.
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
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 =
2
3 Checking for conflicts and issues in changed files.
4
5 Linting changed files:
6 lib/canonical/launchpad/database/launchpadstatistic.py
7 lib/lp/answers/model/question.py
8 lib/lp/registry/doc/product.txt
9 lib/lp/registry/model/product.py
10 lib/lp/registry/model/projectgroup.py
11 lib/lp/translations/doc/translationsoverview.txt
12 lib/lp/translations/model/translationsoverview.py
13 lib/lp/translations/model/translationsperson.py
14 lib/lp/translations/scripts/translations_to_branch.py
15
--
https://code.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/convert-sql-627631-storm into lp:launchpad.
=== modified file 'lib/canonical/launchpad/database/launchpadstatistic.py'
--- lib/canonical/launchpad/database/launchpadstatistic.py 2010-11-17 00:29:11 +0000
+++ lib/canonical/launchpad/database/launchpadstatistic.py 2010-11-17 00:29:14 +0000
@@ -105,8 +105,7 @@
self.update(
'products_using_malone',
- Product.selectBy(official_malone=True).count()
- )
+ Product.selectBy(official_malone=True).count())
ztm.commit()
cur = cursor()
@@ -166,8 +165,8 @@
def _updateRosettaStatistics(self, ztm):
self.update(
'products_using_rosetta',
- Product.selectBy(translations_usage=ServiceUsage.LAUNCHPAD).count()
- )
+ Product.selectBy(
+ translations_usage=ServiceUsage.LAUNCHPAD).count())
self.update('potemplate_count', POTemplate.select().count())
ztm.commit()
self.update('pofile_count', POFile.select().count())
@@ -225,4 +224,3 @@
"FROM Question")
self.update("projects_with_questions_count", cur.fetchone()[0] or 0)
ztm.commit()
-
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2010-11-17 00:29:11 +0000
+++ lib/lp/registry/model/product.py 2010-11-17 00:29:14 +0000
@@ -1570,7 +1570,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)
=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py 2010-11-17 00:29:11 +0000
+++ lib/lp/registry/model/projectgroup.py 2010-11-17 00:29:14 +0000
@@ -104,6 +104,7 @@
)
from lp.services.worlddata.model.language import Language
from lp.translations.enums import TranslationPermission
+from lp.translations.model.potemplate import POTemplate
class ProjectGroup(SQLBase, BugTargetBase, HasSpecificationsMixin,
@@ -184,22 +185,16 @@
def translatables(self):
"""See `IProjectGroup`."""
- return Product.select('''
- Product.project = %s AND
- Product.translations_usage = %s AND
- Product.id = ProductSeries.product AND
- POTemplate.productseries = ProductSeries.id
- ''' % sqlvalues(ServiceUsage.LAUNCHPAD, self),
- clauseTables=['ProductSeries', 'POTemplate'],
- distinct=True)
+ store = Store.of(Product)
+ return store.find(Product,
+ Product.project == self,
+ Product._translations_usage == ServiceUsage.LAUNCHPAD,
+ Product == ProductSeries.product,
+ POTemplate.productseries == ProductSeries).config(distinct=True)
def has_translatable(self):
"""See `IProjectGroup`."""
- # XXX: BradCrittenden 2010-10-12 bug=659078: The test should be
- # converted to use is_empty but the implementation in storm's
- # sqlobject wrapper is broken.
- # return not self.translatables().is_empty()
- return self.translatables().count() != 0
+ return self.translatables().is_empty()
def has_branches(self):
""" See `IProjectGroup`."""
=== modified file 'lib/lp/translations/model/translationsperson.py'
--- lib/lp/translations/model/translationsperson.py 2010-11-17 00:29:11 +0000
+++ lib/lp/translations/model/translationsperson.py 2010-11-17 00:29:14 +0000
@@ -24,6 +24,7 @@
from canonical.database.sqlbase import sqlvalues
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.enum import ServiceUsage
from lp.registry.interfaces.person import IPerson
from lp.registry.model.distribution import Distribution
from lp.registry.model.distroseries import DistroSeries
@@ -277,7 +278,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)
@@ -286,7 +287,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-11-17 00:29:11 +0000
+++ lib/lp/translations/scripts/translations_to_branch.py 2010-11-17 00:29:14 +0000
@@ -16,8 +16,8 @@
from bzrlib.errors import NotBranchError
import pytz
from storm.expr import (
+ And,
Join,
- SQL,
)
from zope.component import getUtility
@@ -32,6 +32,7 @@
MAIN_STORE,
SLAVE_FLAVOR,
)
+from lp.app.enum import ServiceUsage
from lp.code.interfaces.branchjob import IRosettaUploadJobSource
from lp.code.model.directbranchcommit import (
ConcurrentUpdateError,
@@ -310,8 +311,9 @@
ProductSeries, Product, ProductSeries.product == Product.id)
productseries = self.store.using(product_join).find(
ProductSeries,
- ProductSeries, SQL(
- "official_rosetta AND translations_branch IS NOT NULL"))
+ And(
+ Product.translations_usage == ServiceUsage.LAUNCHPAD,
+ Product.translations_branch != None))
# Anything deterministic will do, and even that is only for
# testing.
productseries = productseries.order_by(ProductSeries.id)
