launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02333
[Merge] lp:~edwin-grubbs/launchpad/bug-663857-product-packages-timeout into lp:launchpad
Edwin Grubbs has proposed merging lp:~edwin-grubbs/launchpad/bug-663857-product-packages-timeout into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#663857 Product:+packages timesout with no packages and one series
https://bugs.launchpad.net/bugs/663857
For more details, see:
https://code.launchpad.net/~edwin-grubbs/launchpad/bug-663857-product-packages-timeout/+merge/46222
Summary
-------
This branch fixes the timeout on the $project/+packages page. It
restricts the results to series that are active or have packages. It
also batches the results.
I cleaned up the formatting, since a list of tables is fairly
confusing. I also sorted the results descending. If the series are named
by version number, the latest versions will appear at the top.
Implementation details
----------------------
lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/packaging-views.txt
lib/lp/registry/interfaces/product.py
lib/lp/registry/model/product.py
lib/lp/registry/templates/product-packages.pt
Tests
-----
./bin/test -vv -t 'packaging-views.txt|xx-product-package-pages.txt'
Demo and Q/A
------------
* Open https://code.edge.launchpad.net/obsolete-junk/+packages
* The page should not timeout.
Lint
----
The one remaining lint item shouldn't be fixed.
Linting changed files:
lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/packaging-views.txt
lib/lp/registry/interfaces/product.py
lib/lp/registry/model/product.py
lib/lp/registry/templates/product-packages.pt
./lib/lp/registry/interfaces/product.py
981: E301 expected 1 blank line, found 2
--
https://code.launchpad.net/~edwin-grubbs/launchpad/bug-663857-product-packages-timeout/+merge/46222
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~edwin-grubbs/launchpad/bug-663857-product-packages-timeout into lp:launchpad.
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2010-12-30 16:22:22 +0000
+++ lib/lp/registry/browser/product.py 2011-01-14 03:33:35 +0000
@@ -1117,25 +1117,10 @@
page_title = label
@cachedproperty
- def series_packages(self):
- """A hierarchy of product series, packaging and field data.
-
- A dict of series and packagings. Each packaging is a dict of the
- packaging and a hidden HTML field for forms:
- [{series: <hoary>,
- packagings: {
- packaging: <packaging>,
- field: '<input type=''hidden' ...>},
- }]
- """
- packaged_series = []
- for series in self.context.series:
- packagings = []
- for packaging in series.packagings:
- packagings.append(packaging)
- packaged_series.append(dict(
- series=series, packagings=packagings))
- return packaged_series
+ def series_batch(self):
+ """A batch of series that are active or have packages."""
+ return BatchNavigator(
+ self.context.active_or_packaged_series, self.request)
@property
def distro_packaging(self):
=== modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
--- lib/lp/registry/browser/tests/packaging-views.txt 2010-05-27 04:22:46 +0000
+++ lib/lp/registry/browser/tests/packaging-views.txt 2011-01-14 03:33:35 +0000
@@ -224,20 +224,19 @@
>>> print view.label
Linked packages
-The view provides the series_packages property that returns a list of
-dicts. Each dict as a series and a list of packages.
+The view provides the series_batch property.
>>> def print_packages(view):
- ... for series_dict in view.series_packages:
- ... print series_dict['series'].name
- ... for package in series_dict['packagings']:
- ... print package.distroseries.name
+ ... for series in view.series_batch.batch:
+ ... print series.name
+ ... for package in series.packagings:
+ ... print ' Package:', package.distroseries.name
>>> print_packages(view)
+ trunk
+ hotter
+ Package: grumpy
+ Package: hoary
cold
- hotter
- grumpy
- hoary
- trunk
The view provides the distro_packaging property that is a list of
dictionaries for the distributions and their packaging. The list is
@@ -255,11 +254,11 @@
>>> view = create_initialized_view(
... product, name='+packages', principal=a_user)
>>> print_packages(view)
+ trunk
+ hotter
+ Package: grumpy
+ Package: hoary
cold
- hotter
- grumpy
- hoary
- trunk
# There are links to the +remove-packaging page.
>>> table = find_tag_by_id(view.render(), 'packages-hotter')
@@ -270,8 +269,8 @@
http://launchpad.dev/ubuntu/hoary/+source/thunderbird/+remove-packaging
>>> [hoary_package] = [
- ... package for series_dict in view.series_packages
- ... for package in series_dict['packagings']
+ ... package for series in view.series_batch.batch
+ ... for package in series.packagings
... if package.distroseries.name == 'hoary']
>>> form = {'field.actions.unlink': 'Unlink'}
>>> unlink_view = create_initialized_view(
@@ -279,13 +278,13 @@
>>> unlink_view.errors
[]
- # The view has to be reloaded since view.series_packages is cached.
+ # The view has to be reloaded since view.series_batch is cached.
>>> view = create_initialized_view(product, name='+packages')
>>> print_packages(view)
+ trunk
+ hotter
+ Package: grumpy
cold
- hotter
- grumpy
- trunk
Distro series +packaging view
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2010-12-31 03:39:53 +0000
+++ lib/lp/registry/interfaces/product.py 2011-01-14 03:33:35 +0000
@@ -732,6 +732,9 @@
"Some bug trackers host multiple projects at the same URL "
"and require an identifier for the specific project.")))
+ active_or_packaged_series = Attribute(
+ _("Series that are active and/or have been packaged."))
+
def getVersionSortedSeries(statuses=None, filter_statuses=None):
"""Return all the series sorted by the name field as a version.
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2010-12-02 16:13:51 +0000
+++ lib/lp/registry/model/product.py 2011-01-14 03:33:35 +0000
@@ -28,13 +28,17 @@
SQLObjectNotFound,
StringCol,
)
-from storm.expr import NamedFunc
+from storm.expr import (
+ LeftJoin,
+ NamedFunc,
+ )
from storm.locals import (
And,
Desc,
Int,
Join,
Not,
+ Or,
Select,
SQL,
Store,
@@ -48,9 +52,6 @@
)
from zope.security.proxy import removeSecurityProxy
-from canonical.launchpad.components.decoratedresultset import (
- DecoratedResultSet,
- )
from canonical.database.constants import UTC_NOW
from canonical.database.datetimecol import UtcDateTimeCol
from canonical.database.enumcol import EnumCol
@@ -59,6 +60,9 @@
SQLBase,
sqlvalues,
)
+from canonical.launchpad.components.decoratedresultset import (
+ DecoratedResultSet,
+ )
from canonical.launchpad.interfaces.launchpad import (
IHasIcon,
IHasLogo,
@@ -74,7 +78,6 @@
IStoreSelector,
MAIN_STORE,
)
-from lp.registry.model.series import ACTIVE_STATUSES
from lp.answers.interfaces.faqtarget import IFAQTarget
from lp.answers.interfaces.questioncollection import (
QUESTION_STATUS_DEFAULT_SEARCH,
@@ -145,6 +148,7 @@
from lp.registry.model.commercialsubscription import CommercialSubscription
from lp.registry.model.distribution import Distribution
from lp.registry.model.distroseries import DistroSeries
+from lp.registry.model.hasdrivers import HasDriversMixin
from lp.registry.model.karma import KarmaContextMixin
from lp.registry.model.milestone import (
HasMilestonesMixin,
@@ -156,7 +160,7 @@
from lp.registry.model.productlicense import ProductLicense
from lp.registry.model.productrelease import ProductRelease
from lp.registry.model.productseries import ProductSeries
-from lp.registry.model.hasdrivers import HasDriversMixin
+from lp.registry.model.series import ACTIVE_STATUSES
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.registry.model.structuralsubscription import (
StructuralSubscriptionTargetMixin,
@@ -801,6 +805,22 @@
orderBy='name')
@property
+ def active_or_packaged_series(self):
+ store = Store.of(self)
+ tables = [
+ ProductSeries,
+ LeftJoin(Packaging, Packaging.productseries == ProductSeries.id),
+ ]
+ result = store.using(*tables).find(
+ ProductSeries,
+ ProductSeries.product == self,
+ Or(ProductSeries.status.is_in(ACTIVE_STATUSES),
+ Packaging.id != None))
+ result = result.order_by(Desc(ProductSeries.name))
+ result.config(distinct=True)
+ return result
+
+ @property
def name_with_project(self):
"""See lib.canonical.launchpad.interfaces.IProduct"""
if self.project and self.project.name != self.name:
@@ -1572,7 +1592,7 @@
def getTranslatables(self):
"""See `IProductSet`"""
- # XXX j.c.sackett 2010-11-19 bug=677532 It's less than ideal that
+ # XXX j.c.sackett 2010-11-19 bug=677532 It's less than ideal that
# this query is using _translations_usage, but there's no cleaner
# way to deal with it. Once the bug above is resolved, this should
# should be fixed to use translations_usage.
=== modified file 'lib/lp/registry/templates/product-packages.pt'
--- lib/lp/registry/templates/product-packages.pt 2010-03-03 18:35:08 +0000
+++ lib/lp/registry/templates/product-packages.pt 2011-01-14 03:33:35 +0000
@@ -10,28 +10,44 @@
<body>
<div metal:fill-slot="main">
- <div class="top-portlet">
+ <div class="top-portlet"
+ tal:define="batch_nav view/series_batch">
<h2>Packages by project series</h2>
- <tal:series_dict repeat="series_dict view/series_packages">
- <tal:series_packagings
- define="series series_dict/series;
- packagings series_dict/packagings">
+ <tal:multipage tal:condition="batch_nav/has_multiple_pages">
+ <tal:navigation
+ replace="structure batch_nav/@@+navigation-links-upper"/>
+ </tal:multipage>
+ <table class="listing">
+ <tr tal:repeat="series batch_nav/batch">
+ <td tal:define="packagings series/packagings">
+ <table style="width: 100%; margin: 0.5em 0 0 0">
+ <tr>
+ <td style="width: 33%">
<h3>
<a tal:attributes="href series/name">
<span tal:replace="series/name">main</span> series
</a>
</h3>
+ </td>
- <p tal:condition="not: packagings">
+ <tal:no-packages condition="not: packagings">
+ <td style="width: 33%; text-align: center">
No packages linked to this series.
- </p>
+ </td>
+ <td style="width: 33%; padding: 0 1.5em 0 0; text-align: right">
+ <a tal:condition="series/menu:overview/ubuntupkg/linked"
+ tal:replace="structure series/menu:overview/ubuntupkg/fmt:link" />
+ </td>
+ </tal:no-packages>
+ </tr>
+ </table>
<tal:comment condition="nothing">
This DIV is necessary for the table-actions:nth-child stylesheet.
</tal:comment>
<div>
- <table class="listing"
+ <table class="listing" style="border: 0"
tal:condition="packagings"
tal:attributes="id series/name/fmt:css-id/packages-">
<thead>
@@ -73,15 +89,19 @@
</tbody>
</table>
- <ul class="table-actions"
- tal:condition="series/menu:overview/ubuntupkg/linked">
- <li>
+ <ul class="table-actions" tal:condition="packagings">
+ <li tal:condition="series/menu:overview/ubuntupkg/linked">
<a tal:replace="structure series/menu:overview/ubuntupkg/fmt:link" />
</li>
</ul>
</div>
- </tal:series_packagings>
- </tal:series_dict>
+ </td>
+ </tr>
+ </table>
+ <tal:multipage condition="batch_nav/has_multiple_pages">
+ <tal:navigation
+ replace="structure batch_nav/@@+navigation-links-lower"/>
+ </tal:multipage>
</div>
<div class="portlet">
