launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02314
[Merge] lp:~jcsackett/launchpad/shortlist-696913 into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/shortlist-696913 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#696913 Product:+edit or +configure-* error 'Cannot update project details: shortlist exceeded 1000'
https://bugs.launchpad.net/bugs/696913
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/shortlist-696913/+merge/45938
Summary
=======
Because we are currently snapshotting most fields in pillars when snapshot is called for. However, certain collections don't need to be snapshotted, and cause an error because the number of objects in the collection exceed hardcoded limits.
This branch adds doNotSnapshot to those collections.
Proposed Fix
============
Add doNotSnapshot to the offending fields. A better solution would involve altering Snapshot to ignore (or be able to ignore) CollectionFields, but as this bug is holding up other work, the simple fix should be landed first. The better solution has been filed as bug 701715.
Preimplementation Talk
======================
Spoke with Curtis Hovey about the problem and the interactions of Snapshot and the various fields. Came up with both the simple fix and the more comprehensive one.
Implementation
==============
lib/lp/blueprints/interfaces/specificationtarget.py
lib/lp/registry/interfaces/milestones.py
lib/lp/registry/interfaces/productseries.py
-------------------------------------------
Added doNotSnapshot to the CollectionFields in IHasSpecifications and IHasMilestones. doNotSnapshot had to be added to IProductSeries milestone CollectionFields directly, b/c the interface redefines those fields, but they still connect to probable causes of the Shortlist error.
lib/lp/blueprints/tests/test_hasspecifications.py
lib/lp/registry/tests/test_milestone.py
lib/lp/registry/tests/test_productseries.py
-------------------------------------------
Added tests.
Test
====
bin/test -t test_productseries
bin/test -t test_milestone
bin/test -t test_hasspecifications
QA
==
On staging, hit the +edit or +configure* links for Launchpad. It should not error with shortlist
Lint
====
make lint output:
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/blueprints/interfaces/specificationtarget.py
lib/lp/blueprints/tests/test_hasspecifications.py
lib/lp/registry/interfaces/milestone.py
lib/lp/registry/interfaces/productseries.py
lib/lp/registry/tests/test_milestone.py
lib/lp/registry/tests/test_productseries.py
---------------------------------------------------
--
https://code.launchpad.net/~jcsackett/launchpad/shortlist-696913/+merge/45938
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/shortlist-696913 into lp:launchpad.
=== modified file 'lib/lp/blueprints/interfaces/specificationtarget.py'
--- lib/lp/blueprints/interfaces/specificationtarget.py 2010-11-25 19:09:11 +0000
+++ lib/lp/blueprints/interfaces/specificationtarget.py 2011-01-11 23:42:00 +0000
@@ -19,6 +19,7 @@
)
from zope.schema import TextLine
+from lazr.lifecycle.snapshot import doNotSnapshot
from lazr.restful.declarations import (
exported,
export_as_webservice_entry,
@@ -42,21 +43,21 @@
associated with them, and you can use this interface to query those.
"""
- all_specifications = exported(
+ all_specifications = exported(doNotSnapshot(
CollectionField(
title=_("All specifications"),
value_type=Reference(schema=Interface), # ISpecification, really.
readonly=True,
description=_(
'A list of all specifications, regardless of status or '
- 'approval or completion, for this object.')),
+ 'approval or completion, for this object.'))),
('devel', dict(exported=True)), exported=False)
has_any_specifications = Attribute(
'A true or false indicator of whether or not this object has any '
'specifications associated with it, regardless of their status.')
- valid_specifications = exported(
+ valid_specifications = exported(doNotSnapshot(
CollectionField(
title=_("Valid specifications"),
value_type=Reference(schema=Interface), # ISpecification, really.
@@ -64,7 +65,7 @@
description=_(
'All specifications that are not obsolete. When called from '
'an ISpecificationGoal it will also exclude the ones that '
- 'have not been accepted for that goal')),
+ 'have not been accepted for that goal'))),
('devel', dict(exported=True)), exported=False)
latest_specifications = Attribute(
=== modified file 'lib/lp/blueprints/tests/test_hasspecifications.py'
--- lib/lp/blueprints/tests/test_hasspecifications.py 2010-11-29 18:04:07 +0000
+++ lib/lp/blueprints/tests/test_hasspecifications.py 2011-01-11 23:42:00 +0000
@@ -5,11 +5,17 @@
__metaclass__ = type
+from lazr.lifecycle.snapshot import Snapshot
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.blueprints.interfaces.specification import (
SpecificationDefinitionStatus,
)
+from lp.registry.interfaces.product import IProduct
+from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.productseries import IProductSeries
+from lp.registry.interfaces.projectgroup import IProjectGroup
from lp.testing import TestCaseWithFactory
@@ -178,3 +184,49 @@
product=product, name="spec3")
self.assertNamesOfSpecificationsAre(
["spec1"], person.valid_specifications)
+
+
+class HasSpecificationsSnapshotTestCase(TestCaseWithFactory):
+ """A TestCase for snapshots of specification targets."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(HasSpecificationsSnapshotTestCase, self).setUp()
+
+ def check_omissions(self, target, providing):
+ """snapshots of objects with IHasSpecification should not snapshot
+ marked attributes.
+
+ wrap an export with 'donotsnapshot' to force the snapshot to not
+ include that attribute.
+ """
+ snapshot = Snapshot(target, providing=providing)
+ omitted = [
+ 'all_specifications',
+ 'valid_specifications',
+ ]
+ for attribute in omitted:
+ self.assertFalse(
+ hasattr(snapshot, attribute),
+ "snapshot should not include %s." % attribute)
+
+ def test_product(self):
+ product = self.factory.makeProduct()
+ self.check_omissions(product, IProduct)
+
+ def test_distribution(self):
+ distribution = self.factory.makeDistribution()
+ self.check_omissions(distribution, IDistribution)
+
+ def test_productseries(self):
+ productseries = self.factory.makeProductSeries()
+ self.check_omissions(productseries, IProductSeries)
+
+ def test_distroseries(self):
+ distroseries = self.factory.makeDistroSeries()
+ self.check_omissions(distroseries, IDistroSeries)
+
+ def test_projectgroup(self):
+ projectgroup = self.factory.makeProject()
+ self.check_omissions(projectgroup, IProjectGroup)
=== modified file 'lib/lp/registry/interfaces/milestone.py'
--- lib/lp/registry/interfaces/milestone.py 2010-12-01 22:26:50 +0000
+++ lib/lp/registry/interfaces/milestone.py 2011-01-11 23:42:00 +0000
@@ -15,6 +15,7 @@
'IProjectGroupMilestone',
]
+from lazr.lifecycle.snapshot import doNotSnapshot
from lazr.restful.declarations import (
call_with,
export_as_webservice_entry,
@@ -255,18 +256,18 @@
has_milestones = Bool(title=_("Whether the object has any milestones."))
- milestones = exported(
+ milestones = exported(doNotSnapshot(
CollectionField(
title=_("The visible and active milestones associated with this "
"object, ordered by date expected."),
- value_type=Reference(schema=IMilestone)),
+ value_type=Reference(schema=IMilestone))),
exported_as='active_milestones')
- all_milestones = exported(
+ all_milestones = exported(doNotSnapshot(
CollectionField(
title=_("All milestones associated with this object, ordered by "
"date expected."),
- value_type=Reference(schema=IMilestone)))
+ value_type=Reference(schema=IMilestone))))
class ICanGetMilestonesDirectly(Interface):
=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py 2010-11-27 05:40:38 +0000
+++ lib/lp/registry/interfaces/productseries.py 2011-01-11 23:42:00 +0000
@@ -16,6 +16,7 @@
'ITimelineProductSeries',
]
+from lazr.lifecycle.snapshot import doNotSnapshot
from lazr.restful.declarations import (
export_as_webservice_entry,
export_factory_operation,
@@ -212,20 +213,20 @@
sourcepackages = Attribute(_("List of distribution packages for this "
"product series"))
- milestones = exported(
+ milestones = exported(doNotSnapshot(
CollectionField(
title=_("The visible milestones associated with this "
"project series, ordered by date expected."),
readonly=True,
- value_type=Reference(schema=IMilestone)),
+ value_type=Reference(schema=IMilestone))),
exported_as='active_milestones')
- all_milestones = exported(
+ all_milestones = exported(doNotSnapshot(
CollectionField(
title=_("All milestones associated with this project series, "
"ordered by date expected."),
readonly=True,
- value_type=Reference(schema=IMilestone)))
+ value_type=Reference(schema=IMilestone))))
branch = exported(
ReferenceChoice(
=== modified file 'lib/lp/registry/tests/test_milestone.py'
--- lib/lp/registry/tests/test_milestone.py 2010-10-04 19:50:45 +0000
+++ lib/lp/registry/tests/test_milestone.py 2011-01-11 23:42:00 +0000
@@ -7,6 +7,7 @@
import unittest
+from lazr.lifecycle.snapshot import Snapshot
from zope.component import getUtility
from canonical.launchpad.ftests import (
@@ -14,12 +15,22 @@
login,
logout,
)
-from canonical.testing.layers import LaunchpadFunctionalLayer
-
+from canonical.testing.layers import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
from lp.app.errors import NotFoundError
from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.milestone import IMilestoneSet
-from lp.registry.interfaces.product import IProductSet
+from lp.registry.interfaces.product import (
+ IProduct,
+ IProductSet
+ )
+from lp.registry.interfaces.productseries import IProductSeries
+from lp.registry.interfaces.projectgroup import IProjectGroup
+from lp.testing import TestCaseWithFactory
class MilestoneTest(unittest.TestCase):
@@ -81,6 +92,48 @@
[1, 2, 3])
+class HasMilestonesSnapshotTestCase(TestCaseWithFactory):
+ """A TestCase for snapshots of specification targets."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(HasMilestonesSnapshotTestCase, self).setUp()
+
+ def check_omissions(self, target, providing):
+ """snapshots of objects with IHasSpecification should not snapshot
+ marked attributes.
+
+ wrap an export with 'donotsnapshot' to force the snapshot to not
+ include that attribute.
+ """
+ snapshot = Snapshot(target, providing=providing)
+ omitted = [
+ 'milestones',
+ 'all_milestones',
+ ]
+ for attribute in omitted:
+ self.assertFalse(
+ hasattr(snapshot, attribute),
+ "snapshot should not include %s." % attribute)
+
+ def test_product(self):
+ product = self.factory.makeProduct()
+ self.check_omissions(product, IProduct)
+
+ def test_distribution(self):
+ distribution = self.factory.makeDistribution()
+ self.check_omissions(distribution, IDistribution)
+
+ def test_distroseries(self):
+ distroseries = self.factory.makeDistroSeries()
+ self.check_omissions(distroseries, IDistroSeries)
+
+ def test_projectgroup(self):
+ projectgroup = self.factory.makeProject()
+ self.check_omissions(projectgroup, IProjectGroup)
+
+
def test_suite():
"""Return the test suite for the tests in this module."""
return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/registry/tests/test_productseries.py'
--- lib/lp/registry/tests/test_productseries.py 2010-10-18 20:40:05 +0000
+++ lib/lp/registry/tests/test_productseries.py 2011-01-11 23:42:00 +0000
@@ -7,6 +7,7 @@
from unittest import TestLoader
+from lazr.lifecycle.snapshot import Snapshot
from zope.component import getUtility
from canonical.launchpad.ftests import login
@@ -17,7 +18,10 @@
)
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.distroseries import IDistroSeriesSet
-from lp.registry.interfaces.productseries import IProductSeriesSet
+from lp.registry.interfaces.productseries import (
+ IProductSeries,
+ IProductSeriesSet,
+ )
from lp.testing import TestCaseWithFactory
from lp.translations.interfaces.translations import (
TranslationsBranchImportMode,
@@ -301,5 +305,32 @@
self.productseries.getLatestRelease())
+class ProductSeriesSnapshotTestCase(TestCaseWithFactory):
+ """A TestCase for snapshots of productseries."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(ProductSeriesSnapshotTestCase, self).setUp()
+
+ def test_productseries(self):
+ """snapshots of objects with IHasSpecification should not snapshot
+ marked attributes.
+
+ wrap an export with 'donotsnapshot' to force the snapshot to not
+ include that attribute.
+ """
+ productseries = self.factory.makeProductSeries()
+ snapshot = Snapshot(productseries, providing=IProductSeries)
+ omitted = [
+ 'milestones',
+ 'all_milestones',
+ ]
+ for attribute in omitted:
+ self.assertFalse(
+ hasattr(snapshot, attribute),
+ "snapshot should not include %s." % attribute)
+
+
def test_suite():
return TestLoader().loadTestsFromName(__name__)
