← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~bac/launchpad/db-bug-613442 into lp:launchpad

 

Brad Crittenden has proposed merging lp:~bac/launchpad/db-bug-613442 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #613442 Create multi-state official services attributes
  https://bugs.launchpad.net/bugs/613442


= Summary =

This branch adds db and interface support for eventually replacing the
boolean official_answers, etc, indicators with an enum-based approach.

== Proposed fix ==

Add new columns to the db where needed and provide derived properties
for the two that do not need db support.

== Pre-implementation notes ==

Talks with Curtis.

== Implementation details ==

This branch adds the new columns but does not replace the old ones.
That work will be done next cycle and will be easier once the new
columns are in place.

== Tests ==

Tests have not been updated yet as the first instance of this MP is for
db review only, due to time constraints.  The tests will be updated
prior to requesting a code review.

== Demo and Q/A ==


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/enums.py
  database/schema/patch-2207-99-0.sql
  lib/canonical/launchpad/interfaces/launchpad.py
  lib/lp/registry/interfaces/product.py
  lib/lp/app/interfaces/launchpad.py
  lib/lp/bugs/doc/bugtracker.txt
  lib/lp/registry/interfaces/distribution.py
  lib/lp/registry/model/product.py
  lib/lp/registry/model/distribution.py


== Schema ==

The new sampledata has been generated but is not yet attached to keep
the size of the reviewed branch reasonable.

database/sampledata/current.sql
    database/sampledata/lintdata.sql differs from
database/sampledata/current.sql.
    Patches to the schema, or manual edits to
database/sampledata/current.sql
    do not match the dump of the launchpad_ftest_template database.
    If database/sampledata/lintdata.sql is correct, copy it to
    database/sampledata/current.sql.
    Otherwise update database/sampledata/current.sql and run:
        make schema
        make newsampledata
        cd database/sampledata
        cp newsampledata.sql database/sampledata/current.sql
    Run make schema again to update the test/dev database.

database/sampledata/current.sql
    database/sampledata/lintdata-dev.sql differs from
database/sampledata/current-dev.sql.
    Patches to the schema, or manual edits to
database/sampledata/current-dev.sql
    do not match the dump of the launchpad_dev_template database.
    If database/sampledata/lintdata-dev.sql is correct, copy it to
    database/sampledata/current-dev.sql.
    Otherwise update database/sampledata/current-dev.sql and run:
        make schema
        make newsampledata
        cd database/sampledata
        cp newsampledata-dev.sql database/sampledata/current-dev.sql
    Run make schema again to update the test/dev database.


These lint issues will be addressed before landing.

./lib/canonical/launchpad/interfaces/launchpad.py
      24: 'UnsafeFormGetSubmissionError' imported but unused
      24: 'IBasicLaunchpadRequest' imported but unused
      24: 'IOpenLaunchBag' imported but unused
      24: 'ILaunchpadRoot' imported but unused
      24: 'ILaunchBag' imported but unused
     525: E301 expected 1 blank line, found 0
     467: Line exceeds 78 characters.
./lib/lp/registry/interfaces/product.py
      58: redefinition of unused 'ILaunchpadUsage' from line 55
     897: E301 expected 1 blank line, found 2
./lib/lp/app/interfaces/launchpad.py
      77: E231 missing whitespace after ','
./lib/lp/bugs/doc/bugtracker.txt
     205: narrative uses a moin header.
     295: narrative uses a moin header.
     359: narrative exceeds 78 characters.
     475: want exceeds 78 characters.
     532: narrative exceeds 78 characters.
./lib/lp/registry/model/product.py
      88: redefinition of unused 'ILaunchpadUsage' from line 35
     265: E301 expected 1 blank line, found 0
     283: E301 expected 1 blank line, found 0
     292: E301 expected 1 blank line, found 0
     300: E301 expected 1 blank line, found 0
./lib/lp/registry/model/distribution.py
     202: E301 expected 1 blank line, found 2
     250: E301 expected 1 blank line, found 0
     253: E301 expected 1 blank line, found 0
     479: E203 whitespace before ':'
     694: E222 multiple spaces after operator
    1019: E202 whitespace before ')'
    1027: E202 whitespace before ']'
    1068: E202 whitespace before ')'
    1088: E202 whitespace before ')'
    1121: E231 missing whitespace after ','
    1125: E231 missing whitespace after ','
    1137: E231 missing whitespace after ','
    1153: E202 whitespace before ')'
    1401: E203 whitespace before ':'
-- 
https://code.launchpad.net/~bac/launchpad/db-bug-613442/+merge/31955
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/db-bug-613442 into lp:launchpad.
=== added file 'database/schema/patch-2207-99-0.sql'
--- database/schema/patch-2207-99-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-99-0.sql	2010-08-06 13:36:00 +0000
@@ -0,0 +1,17 @@
+-- Copyright 2010 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+SET client_min_messages=ERROR;
+
+-- The default value of 10 from lp.app.enums is UNKNOWN.
+ALTER TABLE Product
+    ADD COLUMN answers_usage INTEGER NOT NULL DEFAULT 10,
+    ADD COLUMN blueprints_usage INTEGER NOT NULL DEFAULT 10,
+    ADD COLUMN translations_usage INTEGER NOT NULL DEFAULT 10,
+    DROP COLUMN official_codehosting;
+
+ALTER TABLE Distribution
+    ADD COLUMN answers_usage INTEGER NOT NULL DEFAULT 10,
+    ADD COLUMN blueprints_usage INTEGER NOT NULL DEFAULT 10,
+    ADD COLUMN translations_usage INTEGER NOT NULL DEFAULT 10;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 99, 0);

=== modified file 'lib/canonical/launchpad/interfaces/launchpad.py'
--- lib/canonical/launchpad/interfaces/launchpad.py	2010-08-02 02:13:52 +0000
+++ lib/canonical/launchpad/interfaces/launchpad.py	2010-08-06 13:36:00 +0000
@@ -49,7 +49,6 @@
     'ILaunchpadCelebrities',
     'ILaunchpadRoot',
     'ILaunchpadSearch',
-    'ILaunchpadUsage',
     'INotificationRecipientSet',
     'IOpenLaunchBag',
     'IPasswordChangeApp',
@@ -593,26 +592,3 @@
 
         :param recipient_set: An `INotificationRecipientSet`.
         """
-
-class ILaunchpadUsage(Interface):
-    """How the project uses Launchpad."""
-    official_answers = Bool(
-        title=_('People can ask questions in Launchpad Answers'),
-        required=True)
-    official_blueprints = Bool(
-        title=_('This project uses blueprints'), required=True)
-    official_codehosting = Bool(
-        title=_('Code for this project is published in Bazaar branches on'
-                ' Launchpad'),
-        required=True)
-    official_malone = Bool(
-        title=_('Bugs in this project are tracked in Launchpad'),
-        required=True)
-    official_rosetta = Bool(
-        title=_('Translations for this project are done in Launchpad'),
-        required=True)
-    official_anything = Bool (
-        title=_('Uses Launchpad for something'),)
-    enable_bug_expiration = Bool(
-        title=_('Expire "Incomplete" bug reports when they become inactive'),
-        required=True)

=== added file 'lib/lp/app/enums.py'
--- lib/lp/app/enums.py	1970-01-01 00:00:00 +0000
+++ lib/lp/app/enums.py	2010-08-06 13:36:00 +0000
@@ -0,0 +1,44 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Enumerations used in the lp/app modules."""
+
+__metaclass__ = type
+__all__ = [
+    'ServiceUsage',
+    ]
+
+from lazr.enum import DBEnumeratedType, DBItem
+
+
+class ServiceUsage(DBEnumeratedType):
+    """Launchpad application usages.
+
+    Indication of a pillar's usage of Launchpad for the various services:
+    bug tracking, translations, code hosting, blueprint, and answers.
+    """
+
+    UNKNOWN = DBItem(10, """
+    Unknown
+
+    The maintainers have not indicated usage.  This value is the default for
+    new pillars.
+    """)
+
+    LAUNCHPAD = DBItem(20, """
+    Launchpad
+
+    Launchpad is used to provide this service.
+    """)
+
+    EXTERNAL = DBItem(30, """
+    External
+
+    The service is provided external to Launchpad.
+    """)
+
+    NOT_APPLICABLE = DBItem(40, """
+    Not Applicable
+
+    The pillar does not use this type of service in Launchpad or externally.
+    """)

=== added file 'lib/lp/app/interfaces/launchpad.py'
--- lib/lp/app/interfaces/launchpad.py	1970-01-01 00:00:00 +0000
+++ lib/lp/app/interfaces/launchpad.py	2010-08-06 13:36:00 +0000
@@ -0,0 +1,80 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interfaces for the Launchpad application.
+
+Note that these are not interfaces to application content objects.
+"""
+
+__metaclass__ = type
+
+__all__ = [
+    'ILaunchpadUsage',
+    'IServiceUsage',
+    ]
+
+from zope.interface import Interface
+from zope.schema import Bool, Choice
+
+from canonical.launchpad import _
+from lp.app.enums import ServiceUsage
+
+
+class IServiceUsage(Interface):
+    """Pillar service usages."""
+
+    # XXX: BradCrittenden 2010-08-06 bug=n/a:  I hate using the term 'pillar'
+    # but cannot use 'project' or 'distribution'.  The phrase 'Where does'
+    # implies an actual location not an answer of "Launchpad, externally, or
+    # neither."
+    answers_usage = Choice(
+        title=_('Type of service for answers application.'),
+        description=_("Where does this pillar have an Answers forum?"),
+        default=ServiceUsage.UNKNOWN,
+        vocabulary=ServiceUsage)
+    blueprints_usage = Choice(
+        title=_('Type of service for blueprints application.'),
+        description=_("Where does this pillar host blueprints?"),
+        default=ServiceUsage.UNKNOWN,
+        vocabulary=ServiceUsage)
+    codehosting_usage = Choice(
+        title=_('Type of service for hosting code.'),
+        description=_("Where does this pillar host code?"),
+        default=ServiceUsage.UNKNOWN,
+        vocabulary=ServiceUsage)
+    translations_usage = Choice(
+        title=_('Type of service for translations application.'),
+        description=_("Where does this pillar do translations?"),
+        default=ServiceUsage.UNKNOWN,
+        vocabulary=ServiceUsage)
+    bug_tracking_usage = Choice(
+        title=_('Type of service for tracking bugs.'),
+        description=_("Where does this pillar track bugs?"),
+        default=ServiceUsage.UNKNOWN,
+        vocabulary=ServiceUsage)
+    uses_launchpad = Bool(
+        title=_('Uses Launchpad for something.'))
+
+
+class ILaunchpadUsage(Interface):
+    """How the project uses Launchpad."""
+    official_answers = Bool(
+        title=_('People can ask questions in Launchpad Answers'),
+        required=True)
+    official_blueprints = Bool(
+        title=_('This project uses blueprints'), required=True)
+    official_codehosting = Bool(
+        title=_('Code for this project is published in Bazaar branches on'
+                ' Launchpad'),
+        required=True)
+    official_malone = Bool(
+        title=_('Bugs in this project are tracked in Launchpad'),
+        required=True)
+    official_rosetta = Bool(
+        title=_('Translations for this project are done in Launchpad'),
+        required=True)
+    official_anything = Bool(
+        title=_('Uses Launchpad for something'),)
+    enable_bug_expiration = Bool(
+        title=_('Expire "Incomplete" bug reports when they become inactive'),
+        required=True)

=== modified file 'lib/lp/bugs/doc/bugtracker.txt'
--- lib/lp/bugs/doc/bugtracker.txt	2010-03-26 13:48:53 +0000
+++ lib/lp/bugs/doc/bugtracker.txt	2010-08-06 13:36:00 +0000
@@ -12,7 +12,7 @@
     >>> from canonical.launchpad.interfaces import IBugTrackerSet
     >>> bugtracker_set = getUtility(IBugTrackerSet)
     >>> mozilla_bugzilla = bugtracker_set.getByName('mozilla.org')
-    >>> now = datetime.now(pytz.timezone('UTC'))
+    >>> now = datetime.now(pytz.UTC)
 
     >>> def print_watches(bugtracker):
     ...     watches = sorted(

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2010-08-02 01:37:09 +0000
+++ lib/lp/registry/interfaces/distribution.py	2010-08-06 13:36:00 +0000
@@ -48,7 +48,8 @@
 from lp.soyuz.interfaces.buildrecords import IHasBuildRecords
 from lp.registry.interfaces.karma import IKarmaContext
 from canonical.launchpad.interfaces.launchpad import (
-    IHasAppointedDriver, IHasDrivers, ILaunchpadUsage)
+    IHasAppointedDriver, IHasDrivers)
+from lp.app.interfaces.launchpad import ILaunchpadUsage, IServiceUsage
 from lp.registry.interfaces.role import IHasOwner
 from lp.registry.interfaces.mentoringoffer import IHasMentoringOffers
 from lp.registry.interfaces.milestone import (
@@ -94,7 +95,8 @@
     IHasBuildRecords, IHasDrivers, IHasMentoringOffers, IHasMilestones,
     IHasOwner, IHasSecurityContact, IHasSprints, ITranslationPolicy,
     IKarmaContext, ILaunchpadUsage, IMakesAnnouncements,
-    IOfficialBugTagTargetPublic, IPillar, ISpecificationTarget):
+    IOfficialBugTagTargetPublic, IPillar, IServiceUsage,
+    ISpecificationTarget):
     """Public IDistribution properties."""
 
     id = Attribute("The distro's unique number.")

=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py	2010-08-03 08:49:19 +0000
+++ lib/lp/registry/interfaces/product.py	2010-08-06 13:36:00 +0000
@@ -55,6 +55,7 @@
 from canonical.launchpad.interfaces.launchpad import (
     IHasAppointedDriver, IHasDrivers, IHasExternalBugTracker, IHasIcon,
     IHasLogo, IHasMugshot, ILaunchpadUsage)
+from lp.app.interfaces.launchpad import ILaunchpadUsage, IServiceUsage
 from lp.registry.interfaces.role import IHasOwner
 from lp.registry.interfaces.milestone import (
     ICanGetMilestonesDirectly, IHasMilestones)
@@ -347,13 +348,10 @@
     IHasLogo, IHasMentoringOffers, IHasMergeProposals, IHasMilestones,
     IHasMugshot, IHasOwner, IHasSecurityContact, IHasSprints,
     ITranslationPolicy, IKarmaContext, ILaunchpadUsage, IMakesAnnouncements,
-    IOfficialBugTagTargetPublic, IPillar, ISpecificationTarget, IHasRecipes,
-    IHasCodeImports):
+    IOfficialBugTagTargetPublic, IPillar, IServiceUsage,
+    ISpecificationTarget, IHasRecipes, IHasCodeImports):
     """Public IProduct properties."""
 
-    # XXX Mark Shuttleworth 2004-10-12: Let's get rid of ID's in interfaces
-    # unless we really need them. BradB says he can remove the need for them
-    # in SQLObject soon.
     id = Int(title=_('The Project ID'))
 
     project = exported(

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2010-08-05 22:07:46 +0000
+++ lib/lp/registry/model/distribution.py	2010-08-06 13:36:00 +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
@@ -92,7 +92,9 @@
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.interfaces.distroseries import NoSuchDistroSeries
 from canonical.launchpad.interfaces.launchpad import (
-    IHasIcon, IHasLogo, IHasMugshot, ILaunchpadCelebrities, ILaunchpadUsage)
+    IHasIcon, IHasLogo, IHasMugshot, ILaunchpadCelebrities)
+from lp.app.interfaces.launchpad import ILaunchpadUsage, IServiceUsage
+from lp.app.enum import ServiceUsage
 from lp.soyuz.interfaces.queue import PackageUploadStatus
 from lp.registry.interfaces.packaging import PackagingType
 from lp.registry.interfaces.pillar import IPillarNameSet
@@ -129,7 +131,7 @@
     implements(
         IDistribution, IFAQTarget, IHasBugHeat, IHasBugSupervisor,
         IHasBuildRecords, IHasIcon, IHasLogo, IHasMugshot, ILaunchpadUsage,
-        IQuestionTarget)
+        IQuestionTarget, IServiceUsage)
 
     _table = 'Distribution'
     _defaultOrder = 'name'
@@ -178,10 +180,6 @@
         schema=TranslationPermission, default=TranslationPermission.OPEN)
     lucilleconfig = StringCol(
         dbName='lucilleconfig', notNull=False, default=None)
-    official_answers = BoolCol(dbName='official_answers', notNull=True,
-        default=False)
-    official_blueprints = BoolCol(dbName='official_blueprints', notNull=True,
-        default=False)
     active = True # Required by IPillar interface.
     max_bug_heat = Int()
 
@@ -214,6 +212,15 @@
 
         return distro_uploaders
 
+    official_answers = BoolCol(dbName='official_answers', notNull=True,
+        default=False)
+    official_blueprints = BoolCol(dbName='official_blueprints', notNull=True,
+        default=False)
+    official_malone = BoolCol(dbName='official_malone', notNull=True,
+        default=False)
+    official_rosetta = BoolCol(dbName='official_rosetta', notNull=True,
+        default=False)
+
     @property
     def official_codehosting(self):
         # XXX: Aaron Bentley 2008-01-22
@@ -223,16 +230,38 @@
         # absolutely no sense at all.
         return False
 
-    official_malone = BoolCol(dbName='official_malone', notNull=True,
-        default=False)
-    official_rosetta = BoolCol(dbName='official_rosetta', notNull=True,
-        default=False)
-
     @property
     def official_anything(self):
         return True in (self.official_malone, self.official_rosetta,
                         self.official_blueprints, self.official_answers)
 
+    answers_usage = EnumCol(
+        dbName="answers_usage", notNull=True,
+        schema=ServiceUsage,
+        default=ServiceUsage.UNKNOWN)
+    blueprints_usage = EnumCol(
+        dbName="blueprints_usage", notNull=True,
+        schema=ServiceUsage,
+        default=ServiceUsage.UNKNOWN)
+    translations_usage = EnumCol(
+        dbName="translations_usage", notNull=True,
+        schema=ServiceUsage,
+        default=ServiceUsage.UNKNOWN)
+    @property
+    def codehosting_usage(self):
+        return ServiceUsage.NOT_APPLICABLE
+    @property
+    def bug_tracking_usage(self):
+        if not self.official_malone:
+            return ServiceUsage.UNKNOWN
+        else:
+            return ServiceUsage.LAUNCHPAD
+
+    @property
+    def uses_launchpad(self):
+        """Does this distribution actually use Launchpad?"""
+        return self.official_anything
+
     enable_bug_expiration = BoolCol(dbName='enable_bug_expiration',
         notNull=True, default=False)
     translation_focus = ForeignKey(dbName='translation_focus',

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-08-03 08:49:19 +0000
+++ lib/lp/registry/model/product.py	2010-08-06 13:36:00 +0000
@@ -32,6 +32,9 @@
 from canonical.database.sqlbase import quote, SQLBase, sqlvalues
 from canonical.launchpad.interfaces.lpstorm import IStore
 from lp.app.errors import NotFoundError
+from lp.app.interfaces.launchpad import ILaunchpadUsage, IServiceUsage
+from lp.app.enum import ServiceUsage
+from lp.code.enums import BranchType
 from lp.code.model.branchvisibilitypolicy import (
     BranchVisibilityPolicyMixin)
 from lp.code.model.hasbranches import (
@@ -178,7 +181,7 @@
     implements(
         IFAQTarget, IHasBugHeat, IHasBugSupervisor, IHasCustomLanguageCodes,
         IHasIcon, IHasLogo, IHasMugshot, ILaunchpadUsage, IProduct,
-        IQuestionTarget)
+        IQuestionTarget, IServiceUsage)
 
     _table = 'Product'
 
@@ -259,17 +262,54 @@
         # XXX Need to remove official_codehosting column from Product
         # table.
         return self.development_focus.branch is not None
-
-    def _getMilestoneCondition(self):
-        """See `HasMilestonesMixin`."""
-        return (Milestone.product == self)
-
     @property
     def official_anything(self):
         return True in (self.official_malone, self.official_rosetta,
                         self.official_blueprints, self.official_answers,
                         self.official_codehosting)
 
+    answers_usage = EnumCol(
+        dbName="answers_usage", notNull=True,
+        schema=ServiceUsage,
+        default=ServiceUsage.UNKNOWN)
+    blueprints_usage = EnumCol(
+        dbName="blueprints_usage", notNull=True,
+        schema=ServiceUsage,
+        default=ServiceUsage.UNKNOWN)
+    translations_usage = EnumCol(
+        dbName="translations_usage", notNull=True,
+        schema=ServiceUsage,
+        default=ServiceUsage.UNKNOWN)
+    @property
+    def codehosting_usage(self):
+        if self.development_focus.branch is None:
+            return ServiceUsage.UNKNOWN
+        elif self.development_focus.branch.type == BranchType.HOSTED:
+            return ServiceUsage.LAUNCHPAD
+        elif self.development_focus.branch.type == BranchType.MIRRORED:
+            return ServiceUsage.EXTERNAL
+        return ServiceUsage.NOT_APPLICABLE
+    @property
+    def bug_tracking_usage(self):
+        if self.official_malone:
+            return ServiceUsage.LAUNCHPAD
+        elif self.bugtracker is None:
+            return ServiceUsage.UNKNOWN
+        else:
+            return ServiceUsage.EXTERNAL
+    @property
+    def uses_launchpad(self):
+        """Does this distribution actually use Launchpad?"""
+        return ServiceUsage.LAUNCHPAD in (self.answers_usage,
+                                          self.blueprints_usage,
+                                          self.translations_usage,
+                                          self.codehosting_usage,
+                                          self.bug_tracking_usage)
+
+    def _getMilestoneCondition(self):
+        """See `HasMilestonesMixin`."""
+        return (Milestone.product == self)
+
     enable_bug_expiration = BoolCol(dbName='enable_bug_expiration',
         notNull=True, default=False)
     license_reviewed = BoolCol(dbName='reviewed', notNull=True, default=False)


Follow ups