launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00309
lp:~edwin-grubbs/launchpad/bug-577492-needspackaging-duplicates into lp:launchpad/devel
Edwin Grubbs has proposed merging lp:~edwin-grubbs/launchpad/bug-577492-needspackaging-duplicates into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#528648 restore needs_packaging for +portlet-package-summary
https://bugs.launchpad.net/bugs/528648
#577492 +needs-packaging lists duplicates
https://bugs.launchpad.net/bugs/577492
Summary
-------
Sped up +needs-packaging page and the needs-packaging portlet on the
distroseries +index page by making getPrioritizedUnlinkedSourcePackages()
return a result set, so that batching will result in a limited query.
Previously, the result set was turned into a list in order to format
each item as a dictionary. Therefore, it would load 13,000 source
packages, and then iterate just over the first batch.
I also fixed the name of getPrioritizedPackagings() and made it
return a result set instead of a list.
Tests
-----
./bin/test -vv -t 'test_distroseries|doc/distroseries.txt'
Demo and Q/A
------------
* Open https://staging.launchpad.net/ubuntu/maverick/+needs-packaging
* It should not timeout.
* Open https://staging.launchpad.net/ubuntu/maverick/
* The "Upstream packaging" portlet should say
"Needs more information or linking to upstream" and list some source
packages.
Lint
----
There are over 700 lint errors due to bad indentation in doctests, so I
will add those changes later, so that reviewers aren't scared away.
--
https://code.launchpad.net/~edwin-grubbs/launchpad/bug-577492-needspackaging-duplicates/+merge/31112
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~edwin-grubbs/launchpad/bug-577492-needspackaging-duplicates into lp:launchpad/devel.
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2010-05-24 22:04:19 +0000
+++ lib/lp/registry/browser/distroseries.py 2010-07-28 02:18:47 +0000
@@ -333,9 +333,7 @@
@cachedproperty
def needs_linking(self):
"""Return a list of 10 packages most in need of upstream linking."""
- # XXX sinzui 2010-02-26 bug=528648: This method causes a timeout.
- # return self.context.getPrioritizedUnlinkedSourcePackages()[:10]
- return None
+ return self.context.getPrioritizedUnlinkedSourcePackages()[:10]
milestone_can_release = False
@@ -481,7 +479,7 @@
@cachedproperty
def cached_packagings(self):
"""The batched upstream packaging links."""
- packagings = self.context.getPrioritizedlPackagings()
+ packagings = self.context.getPrioritizedPackagings()
navigator = BatchNavigator(packagings, self.request, size=20)
navigator.setHeadings('packaging', 'packagings')
return navigator
=== added file 'lib/lp/registry/browser/tests/test_distroseries_views.py'
--- lib/lp/registry/browser/tests/test_distroseries_views.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_distroseries_views.py 2010-07-28 02:18:47 +0000
@@ -0,0 +1,52 @@
+# Copyright 2009 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import unittest
+
+from storm.zope.interfaces import IResultSet
+
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.testing import LaunchpadZopelessLayer
+from lp.testing import TestCaseWithFactory
+from lp.testing.views import create_initialized_view
+
+
+class TestDistroSeriesNeedsPackagesView(TestCaseWithFactory):
+ """Test the distroseries +needs-packaging view."""
+
+ layer = LaunchpadZopelessLayer
+
+ def test_cached_unlinked_packages(self):
+ ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+ distroseries = self.factory.makeDistroSeries(distribution=ubuntu)
+ view = create_initialized_view(distroseries, '+needs-packaging')
+ naked_packages = removeSecurityProxy(view.cached_unlinked_packages)
+ self.assertTrue(
+ IResultSet.providedBy(
+ view.cached_unlinked_packages.currentBatch().list),
+ '%s should batch IResultSet so that slicing will limit the '
+ 'query' % view.cached_unlinked_packages.currentBatch().list)
+
+
+class TestDistroSeriesView(TestCaseWithFactory):
+ """Test the distroseries +index view."""
+
+ layer = LaunchpadZopelessLayer
+
+ def test_needs_linking(self):
+ ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+ distroseries = self.factory.makeDistroSeries(distribution=ubuntu)
+ view = create_initialized_view(distroseries, '+index')
+ self.assertTrue(
+ IResultSet.providedBy(view.needs_linking),
+ '%s should implement IResultSet so that slicing will limit the '
+ 'query' % view.needs_linking)
+
+
+def test_suite():
+ return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/registry/doc/distroseries.txt'
--- lib/lp/registry/doc/distroseries.txt 2010-07-20 09:36:17 +0000
+++ lib/lp/registry/doc/distroseries.txt 2010-07-28 02:18:47 +0000
@@ -457,11 +457,11 @@
linux-source-2.6.15 0 0
-The distroseries getPrioritizedlPackagings() method that returns a prioritized
+The distroseries getPrioritizedPackagings() method that returns a prioritized
list of `IPackaging` that need more information about the upstream project to
share bugs, translations, and code.
- >>> for packaging in hoary.getPrioritizedlPackagings():
+ >>> for packaging in hoary.getPrioritizedPackagings():
... print packaging.sourcepackagename.name
netapplet
evolution
=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py 2010-06-08 15:58:04 +0000
+++ lib/lp/registry/interfaces/distroseries.py 2010-07-28 02:18:47 +0000
@@ -414,7 +414,7 @@
and total_messages (translatable messages).
"""
- def getPrioritizedlPackagings():
+ def getPrioritizedPackagings():
"""Return a list of packagings that need more upstream information."""
def getMostRecentlyLinkedPackagings():
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py 2010-07-15 15:01:18 +0000
+++ lib/lp/registry/model/distroseries.py 2010-07-28 02:18:47 +0000
@@ -19,7 +19,7 @@
BoolCol, StringCol, ForeignKey, SQLMultipleJoin, IntCol,
SQLObjectNotFound, SQLRelatedJoin)
-from storm.locals import And, Desc, Join, SQL
+from storm.locals import Desc, Join, SQL
from storm.store import Store
from zope.component import getUtility
@@ -335,14 +335,19 @@
condition = SQL(conditions + "AND packaging.id IS NULL")
results = IStore(self).using(origin).find(find_spec, condition)
results = results.order_by('score DESC', SourcePackageName.name)
- return [{
- 'package': SourcePackage(
+ results = results.config(distinct=True)
+
+ def decorator(result):
+ spn, score, bug_count, total_messages = result
+ return {
+ 'package': SourcePackage(
sourcepackagename=spn, distroseries=self),
- 'bug_count': bug_count,
- 'total_messages': total_messages}
- for (spn, score, bug_count, total_messages) in results]
+ 'bug_count': bug_count,
+ 'total_messages': total_messages,
+ }
+ return DecoratedResultSet(results, decorator)
- def getPrioritizedlPackagings(self):
+ def getPrioritizedPackagings(self):
"""See `IDistroSeries`.
The prioritization is a heuristic rule using the branch, bug heat,
@@ -352,18 +357,6 @@
# the objects that are implcitly needed to work with a
# Packaging object.
# Avoid circular import failures.
- from lp.registry.model.product import Product
- from lp.registry.model.productseries import ProductSeries
- find_spec = (
- Packaging, SourcePackageName, ProductSeries, Product,
- SQL("""
- CASE WHEN spr.component = 1 THEN 1000 ELSE 0 END +
- CASE WHEN Product.bugtracker IS NULL
- THEN coalesce(total_bug_heat, 10) ELSE 0 END +
- CASE WHEN ProductSeries.translations_autoimport_mode = 1
- THEN coalesce(po_messages, 10) ELSE 0 END +
- CASE WHEN ProductSeries.branch IS NULL THEN 500
- ELSE 0 END AS score"""))
joins, conditions = self._current_sourcepackage_joins_and_conditions
origin = SQL(joins + """
JOIN ProductSeries
@@ -372,10 +365,19 @@
ON ProductSeries.product = Product.id
""")
condition = SQL(conditions + "AND packaging.id IS NOT NULL")
- results = IStore(self).using(origin).find(find_spec, condition)
- results = results.order_by('score DESC, SourcePackageName.name ASC')
- return [packaging
- for (packaging, spn, series, product, score) in results]
+ results = IStore(self).using(origin).find(Packaging, condition)
+ return results.order_by("""
+ (
+ CASE WHEN spr.component = 1 THEN 1000 ELSE 0 END
+ + CASE WHEN Product.bugtracker IS NULL
+ THEN coalesce(total_bug_heat, 10) ELSE 0 END
+ + CASE WHEN ProductSeries.translations_autoimport_mode = 1
+ THEN coalesce(po_messages, 10) ELSE 0 END
+ + CASE WHEN ProductSeries.branch IS NULL THEN 500
+ ELSE 0 END
+ ) DESC,
+ SourcePackageName.name ASC
+ """)
@property
def _current_sourcepackage_joins_and_conditions(self):
=== modified file 'lib/lp/registry/templates/distroseries-portlet-packaging.pt'
--- lib/lp/registry/templates/distroseries-portlet-packaging.pt 2010-05-27 04:10:17 +0000
+++ lib/lp/registry/templates/distroseries-portlet-packaging.pt 2010-07-28 02:18:47 +0000
@@ -56,7 +56,7 @@
</dt>
<dd tal:condition="view/needs_linking">
<ul class="horizontal">
- <li repeat="package view/needs_linking">
+ <li tal:repeat="package view/needs_linking">
<a class="sprite package-source"
tal:attributes="href package/package/fmt:url"
tal:content="package/package/name">evolution</a>
=== modified file 'lib/lp/registry/tests/test_distroseries.py'
--- lib/lp/registry/tests/test_distroseries.py 2010-07-20 17:50:45 +0000
+++ lib/lp/registry/tests/test_distroseries.py 2010-07-28 02:18:47 +0000
@@ -271,17 +271,17 @@
u'main', u'hot-translatable', u'hot', u'translatable', u'normal']
self.assertEqual(expected, names)
- def test_getPrioritizedlPackagings(self):
+ def test_getPrioritizedPackagings(self):
# Verify the ordering of packagings that need more upstream info.
for name in ['main', 'hot-translatable', 'hot', 'translatable']:
self.linkPackage(name)
- packagings = self.series.getPrioritizedlPackagings()
+ packagings = self.series.getPrioritizedPackagings()
names = [packaging.sourcepackagename.name for packaging in packagings]
expected = [
u'main', u'hot-translatable', u'hot', u'translatable', u'linked']
self.assertEqual(expected, names)
- def test_getPrioritizedlPackagings_bug_tracker(self):
+ def test_getPrioritizedPackagings_bug_tracker(self):
# Verify the ordering of packagings with and without a bug tracker.
self.linkPackage('hot')
self.makeSeriesPackage('cold')
@@ -289,12 +289,12 @@
naked_product_series = remove_security_proxy_and_shout_at_engineer(
product_series)
naked_product_series.product.bugtraker = self.factory.makeBugTracker()
- packagings = self.series.getPrioritizedlPackagings()
+ packagings = self.series.getPrioritizedPackagings()
names = [packaging.sourcepackagename.name for packaging in packagings]
expected = [u'hot', u'cold', u'linked']
self.assertEqual(expected, names)
- def test_getPrioritizedlPackagings_branch(self):
+ def test_getPrioritizedPackagings_branch(self):
# Verify the ordering of packagings with and without a branch.
self.linkPackage('translatable')
self.makeSeriesPackage('withbranch')
@@ -302,12 +302,12 @@
naked_product_series = remove_security_proxy_and_shout_at_engineer(
product_series)
naked_product_series.branch = self.factory.makeBranch()
- packagings = self.series.getPrioritizedlPackagings()
+ packagings = self.series.getPrioritizedPackagings()
names = [packaging.sourcepackagename.name for packaging in packagings]
expected = [u'translatable', u'linked', u'withbranch']
self.assertEqual(expected, names)
- def test_getPrioritizedlPackagings_translation(self):
+ def test_getPrioritizedPackagings_translation(self):
# Verify the ordering of translatable packagings that are and are not
# configured to import.
self.linkPackage('translatable')
@@ -318,7 +318,7 @@
naked_product_series.branch = self.factory.makeBranch()
naked_product_series.translations_autoimport_mode = (
TranslationsBranchImportMode.IMPORT_TEMPLATES)
- packagings = self.series.getPrioritizedlPackagings()
+ packagings = self.series.getPrioritizedPackagings()
names = [packaging.sourcepackagename.name for packaging in packagings]
expected = [u'translatable', u'linked', u'importabletranslatable']
self.assertEqual(expected, names)
