launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00549
[Merge] lp:~bac/launchpad/bug-590813 into lp:launchpad/devel
Brad Crittenden has proposed merging lp:~bac/launchpad/bug-590813 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#590813 ShortListTooBigError configuring project
https://bugs.launchpad.net/bugs/590813
= Summary =
Some products have lots of series...sometimes over 1000, which breaks
ShortList. Snapshotting those series is a huge waste of time.
== Proposed fix ==
Add 'doNotSnapshot' to the interface definition of CollectionFields in
IProduct and IDistribution.
Also did some drive-by lint, spelling, and grammar fixes.
== Pre-implementation notes ==
Talk with Curtis.
== Implementation details ==
As above.
== Tests ==
bin/test -vvt test_snapshot
== Demo and Q/A ==
Nope
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/browser/stringformatter.py
lib/lp/registry/interfaces/product.py
lib/lp/registry/interfaces/distribution.py
lib/lp/registry/tests/test_distribution.py
lib/lp/registry/tests/test_person.py
lib/lp/registry/tests/test_product.py
lib/lp/app/browser/tests/root-views.txt
./lib/lp/app/browser/stringformatter.py
405: E501 line too long (90 characters)
409: E501 line too long (88 characters)
405: Line exceeds 78 characters.
409: Line exceeds 78 characters.
./lib/lp/registry/interfaces/product.py
903: E301 expected 1 blank line, found 2
--
https://code.launchpad.net/~bac/launchpad/bug-590813/+merge/32248
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-590813 into lp:launchpad/devel.
=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py 2010-06-14 04:29:01 +0000
+++ lib/lp/app/browser/stringformatter.py 2010-08-10 19:31:08 +0000
@@ -156,6 +156,7 @@
The text may contain entity references or HTML tags.
"""
+
def replace(match):
if match.group('tag'):
return match.group()
@@ -176,7 +177,7 @@
def nl_to_br(self):
"""Quote HTML characters, then replace newlines with <br /> tags."""
- return cgi.escape(self._stringtoformat).replace('\n','<br />\n')
+ return cgi.escape(self._stringtoformat).replace('\n', '<br />\n')
def escape(self):
return escape(self._stringtoformat)
@@ -580,7 +581,7 @@
if in_fold and line.endswith('</p>') and in_false_paragraph:
# The line ends with a false paragraph in a PGP signature.
# Restore the line break to join with the next paragraph.
- line = '%s<br />\n<br />' % strip_trailing_p_tag(line)
+ line = '%s<br />\n<br />' % strip_trailing_p_tag(line)
elif (in_quoted and self._re_quoted.match(line) is None):
# The line is not quoted like the previous line.
# End fold before we append this line.
@@ -684,7 +685,8 @@
if person is not None and not person.hide_email_addresses:
# Circular dependancies now. Should be resolved by moving the
# object image display api.
- from canonical.launchpad.webapp.tales import ObjectImageDisplayAPI
+ from canonical.launchpad.webapp.tales import (
+ ObjectImageDisplayAPI)
css_sprite = ObjectImageDisplayAPI(person).sprite_css()
text = text.replace(
address, '<a href="%s" class="%s">%s</a>' % (
@@ -770,7 +772,7 @@
letter.
:param prefix: an optional string to prefix to the id. It can be
- used to ensure that the start of the id is predicable.
+ used to ensure that the start of the id is predictable.
"""
if prefix is not None:
raw_text = prefix + self._stringtoformat
=== modified file 'lib/lp/app/browser/tests/root-views.txt'
--- lib/lp/app/browser/tests/root-views.txt 2010-01-08 21:23:15 +0000
+++ lib/lp/app/browser/tests/root-views.txt 2010-08-10 19:31:08 +0000
@@ -4,7 +4,7 @@
The launchpad front page uses the LaunchpadRootIndexView to provide the
special data needed for the layout.
- # The _get_day_of_year() method must be hacked to return a predicable day
+ # The _get_day_of_year() method must be hacked to return a predictable day
# to testing the view.
>>> from lp.app.browser.root import LaunchpadRootIndexView
>>> def day():
=== 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-10 19:31:08 +0000
@@ -23,6 +23,7 @@
from zope.schema import Bool, Choice, Datetime, List, Object, Text, TextLine
from zope.interface import Attribute, Interface
+from lazr.lifecycle.snapshot import doNotSnapshot
from lazr.restful.fields import CollectionField, Reference
from lazr.restful.declarations import (
collection_default_content, export_as_webservice_collection,
@@ -198,25 +199,27 @@
lucilleconfig = TextLine(
title=_("Lucille Config"),
description=_("The Lucille Config."), required=False)
- archive_mirrors = exported(CollectionField(
- description=_("All enabled and official ARCHIVE mirrors of this "
- "Distribution."),
- readonly=True, value_type=Object(schema=IDistributionMirror)))
- cdimage_mirrors = exported(CollectionField(
- description=_("All enabled and official RELEASE mirrors of this "
- "Distribution."),
- readonly=True, value_type=Object(schema=IDistributionMirror)))
+ archive_mirrors = exported(doNotSnapshot(
+ CollectionField(
+ description=_("All enabled and official ARCHIVE mirrors "
+ "of this Distribution."),
+ readonly=True, value_type=Object(schema=IDistributionMirror))))
+ cdimage_mirrors = exported(doNotSnapshot(
+ CollectionField(
+ description=_("All enabled and official RELEASE mirrors "
+ "of this Distribution."),
+ readonly=True, value_type=Object(schema=IDistributionMirror))))
disabled_mirrors = Attribute(
"All disabled and official mirrors of this Distribution.")
unofficial_mirrors = Attribute(
"All unofficial mirrors of this Distribution.")
pending_review_mirrors = Attribute(
"All mirrors of this Distribution that haven't been reviewed yet.")
- series = exported(
+ series = exported(doNotSnapshot(
CollectionField(
title=_("DistroSeries inside this Distribution"),
# Really IDistroSeries, see _schema_circular_imports.py.
- value_type=Reference(schema=Interface)))
+ value_type=Reference(schema=Interface))))
architectures = List(
title=_("DistroArchSeries inside this Distribution"))
uploaders = Attribute(_(
@@ -260,11 +263,11 @@
# Really IArchive, see _schema_circular_imports.py.
schema=Interface))
- all_distro_archives = exported(
+ all_distro_archives = exported(doNotSnapshot(
CollectionField(
title=_("A sequence of the distribution's non-PPA Archives."),
readonly=True, required=False,
- value_type=Reference(schema=Interface)),
+ value_type=Reference(schema=Interface))),
# Really IArchive, see _schema_circular_imports.py.
exported_as='archives')
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2010-08-02 01:37:09 +0000
+++ lib/lp/registry/interfaces/product.py 2010-08-10 19:31:08 +0000
@@ -73,6 +73,7 @@
ITranslationPolicy)
from canonical.launchpad.validators import LaunchpadValidationError
from canonical.launchpad.validators.name import name_validator
+from lazr.lifecycle.snapshot import doNotSnapshot
from lazr.restful.fields import CollectionField, Reference, ReferenceChoice
from lazr.restful.declarations import (
REQUEST_USER, call_with, collection_default_content,
@@ -587,7 +588,8 @@
"this product"))
series = exported(
- CollectionField(value_type=Object(schema=IProductSeries)))
+ doNotSnapshot(
+ CollectionField(value_type=Object(schema=IProductSeries))))
development_focus = exported(
ReferenceChoice(
@@ -604,10 +606,12 @@
"product; otherwise, simply returns the product name."))
releases = exported(
- CollectionField(
- title=_("An iterator over the ProductReleases for this product."),
- readonly=True,
- value_type=Reference(schema=IProductRelease)))
+ doNotSnapshot(
+ CollectionField(
+ title=_("An iterator over the ProductReleases for "
+ "this product."),
+ readonly=True,
+ value_type=Reference(schema=IProductRelease))))
translation_focus = exported(
ReferenceChoice(
=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py 2010-08-03 15:15:16 +0000
+++ lib/lp/registry/tests/test_distribution.py 2010-08-10 19:31:08 +0000
@@ -7,10 +7,13 @@
from zope.security.proxy import removeSecurityProxy
+from lazr.lifecycle.snapshot import Snapshot
from lp.registry.tests.test_distroseries import (
TestDistroSeriesCurrentSourceReleases)
from lp.registry.interfaces.distroseries import NoSuchDistroSeries
from lp.registry.interfaces.series import SeriesStatus
+from lp.registry.interfaces.distribution import IDistribution
+
from lp.soyuz.interfaces.distributionsourcepackagerelease import (
IDistributionSourcePackageRelease)
from lp.testing import TestCaseWithFactory
@@ -140,3 +143,31 @@
series = self.factory.makeDistroSeries(distribution=distro,
name="dappere", version="42.6")
self.assertEquals(series, distro.getSeries("42.6"))
+
+
+class DistroSnapshotTestCase(TestCaseWithFactory):
+ """A TestCase for distribution snapshots."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(DistroSnapshotTestCase, self).setUp()
+ self.distribution = self.factory.makeDistribution(name="boobuntu")
+
+ def test_snapshot(self):
+ """Snapshots of products should not include marked attribues.
+
+ Wrap an export with 'doNotSnapshot' to force the snapshot to not
+ include that attribute.
+ """
+ snapshot = Snapshot(self.distribution, providing=IDistribution)
+ omitted = [
+ 'archive_mirrors',
+ 'cdimage_mirrors',
+ 'series',
+ 'all_distro_archives',
+ ]
+ for attribute in omitted:
+ self.assertFalse(
+ hasattr(snapshot, attribute),
+ "Snapshot should not include %s." % attribute)
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py 2010-08-04 23:27:49 +0000
+++ lib/lp/registry/tests/test_person.py 2010-08-10 19:31:08 +0000
@@ -262,8 +262,8 @@
transaction.commit()
logout()
- def _get_testible_account(self, person, date_created, openid_identifier):
- # Return a naked account with predicable attributes.
+ def _get_testable_account(self, person, date_created, openid_identifier):
+ # Return a naked account with predictable attributes.
account = removeSecurityProxy(person.account)
account.date_created = date_created
account.openid_identifier = openid_identifier
@@ -277,7 +277,7 @@
2010, 04, 01, 0, 0, 0, 0, tzinfo=pytz.timezone('UTC'))
# Free an OpenID identifier using merge.
first_duplicate = self.factory.makePerson()
- first_account = self._get_testible_account(
+ first_account = self._get_testable_account(
first_duplicate, test_date, test_identifier)
first_person = self.factory.makePerson()
self._do_premerge(first_duplicate, first_person)
@@ -289,7 +289,7 @@
# Create an account that reuses the freed OpenID_identifier.
test_date = test_date.replace(2010, 05)
second_duplicate = self.factory.makePerson()
- second_account = self._get_testible_account(
+ second_account = self._get_testable_account(
second_duplicate, test_date, test_identifier)
second_person = self.factory.makePerson()
self._do_premerge(second_duplicate, second_person)
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2010-08-02 20:24:24 +0000
+++ lib/lp/registry/tests/test_product.py 2010-08-10 19:31:08 +0000
@@ -18,14 +18,16 @@
from canonical.launchpad.ftests import syncUpdate
+from lazr.lifecycle.snapshot import Snapshot
from lp.registry.interfaces.person import IPersonSet
-from lp.registry.interfaces.product import License
+from lp.registry.interfaces.product import IProduct, License
from lp.registry.model.product import Product
from lp.registry.model.productlicense import ProductLicense
from lp.registry.model.commercialsubscription import (
CommercialSubscription)
from lp.testing import TestCaseWithFactory
+
class TestProduct(TestCaseWithFactory):
"""Tests product object."""
@@ -178,6 +180,32 @@
'new')
+class ProductSnapshotTestCase(TestCaseWithFactory):
+ """A TestCase for product snapshots."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(ProductSnapshotTestCase, self).setUp()
+ self.product = self.factory.makeProduct(name="shamwow")
+
+ def test_snapshot(self):
+ """Snapshots of products should not include marked attribues.
+
+ Wrap an export with 'doNotSnapshot' to force the snapshot to not
+ include that attribute.
+ """
+ snapshot = Snapshot(self.product, providing=IProduct)
+ omitted = [
+ 'series',
+ 'releases',
+ ]
+ for attribute in omitted:
+ self.assertFalse(
+ hasattr(snapshot, attribute),
+ "Snapshot should not include %s." % attribute)
+
+
class BugSupervisorTestCase(TestCaseWithFactory):
"""A TestCase for bug supervisor management."""
Follow ups
