Merge lp:~edwin-grubbs/launchpad/bug-534462-project-index-timeout into lp:launchpad

Proposed by Edwin Grubbs
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~edwin-grubbs/launchpad/bug-534462-project-index-timeout
Merge into: lp:launchpad
Diff against target: 246 lines (+72/-35)
7 files modified
lib/lp/registry/browser/product.py (+3/-4)
lib/lp/registry/doc/distribution.txt (+17/-0)
lib/lp/registry/interfaces/distribution.py (+8/-3)
lib/lp/registry/model/distribution.py (+23/-7)
lib/lp/registry/model/distributionsourcepackage.py (+14/-8)
lib/lp/registry/stories/product/xx-product-index.txt (+1/-1)
lib/lp/registry/templates/product-portlet-packages.pt (+6/-12)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-534462-project-index-timeout
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+20980@code.launchpad.net

Commit message

Fixed timeout on gcc project index page due to the Packages in Ubuntu portlet.

Description of the change

Summary
-------

Fixed timeout issue caused by the "Packages in Ubuntu" portlet
on the project index page. I also made DistroSourcePackage.upstream_product
more efficient, even though that is no longer needed by the view,
since searchSourcePackages() can now do the filtering for us.

Implementation details
----------------------

Added has_packaging parameter to searchSourcePackages() so we
do not have to check whether the package has an upstream_product.
    lib/lp/registry/browser/product.py
    lib/lp/registry/doc/distribution.txt
    lib/lp/registry/interfaces/distribution.py
    lib/lp/registry/model/distribution.py

Optimized DistroSourcePackage.upstream_product:
    lib/lp/registry/model/distributionsourcepackage.py

The portlet would either have the title "Packages in Ubuntu" or
"Packages in distributions" depending on the content, which didn't
really make sense, so now there is a single title.
    lib/lp/registry/stories/product/xx-product-index.txt
    lib/lp/registry/templates/product-portlet-packages.pt

Tests
-----

./bin/test -vv -t 'doc/distribution.txt|xx-product-index.txt|distribution-sourcepackage.txt|xx-distribution-source-package.txt'

Demo and Q/A
------------

On launchpad.dev:
* Open http://launchpad.dev/firefox/+packages
  * Remove all the linked packages.
* Open http://launchpad.dev/firefox/+packages
  * The "Packages in Ubuntu" portlet should show
    the mozilla-firefox sourcepackage and a button
    to link to it. (If it doesn't show up, try
    running cronscripts/update-pkgcache.py)

On edge:
* Open https://edge.launchpad.net/gcc
  * This should not timeout.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/product.py'
2--- lib/lp/registry/browser/product.py 2010-03-08 01:51:58 +0000
3+++ lib/lp/registry/browser/product.py 2010-03-09 17:46:07 +0000
4@@ -982,14 +982,13 @@
5 """See `LaunchpadFormView`."""
6 super(ProductPackagesPortletView, self).setUpFields()
7 ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
8- source_packages = ubuntu.searchSourcePackages(self.context.name)
9+ source_packages = ubuntu.searchSourcePackages(
10+ self.context.name, has_packaging=False)
11 # Based upon the matches, create a new vocabulary with
12 # term descriptions that include a link to the source package.
13 self.suggestions = []
14 vocab_terms = []
15- for package in source_packages:
16- if package.upstream_product is not None:
17- continue
18+ for package in source_packages[:20]:
19 self.suggestions.append(package)
20 item_url = canonical_url(package)
21 description = """<a href="%s">%s</a>""" % (
22
23=== modified file 'lib/lp/registry/doc/distribution.txt'
24--- lib/lp/registry/doc/distribution.txt 2010-02-15 12:59:55 +0000
25+++ lib/lp/registry/doc/distribution.txt 2010-03-09 17:46:07 +0000
26@@ -202,6 +202,23 @@
27 DistributionSourcePackage: foobar
28 DistributionSourcePackage: commercialpackage
29
30+searchSourcePackages() also has a has_packaging parameter that
31+it just passes on to searchSourcePackageCaches(), and it restricts
32+the results based on whether the source package has an entry
33+in the Packaging table linking it to an upstream project.
34+
35+ >>> packages = ubuntu.searchSourcePackages('a', has_packaging=True)
36+ >>> for dsp in packages:
37+ ... print "%s: %s" % (dsp.__class__.__name__, dsp.name)
38+ DistributionSourcePackage: mozilla-firefox
39+ DistributionSourcePackage: netapplet
40+ DistributionSourcePackage: alsa-utils
41+ >>> packages = ubuntu.searchSourcePackages('a', has_packaging=False)
42+ >>> for dsp in packages:
43+ ... print "%s: %s" % (dsp.__class__.__name__, dsp.name)
44+ DistributionSourcePackage: foobar
45+ DistributionSourcePackage: commercialpackage
46+
47
48 === Searching for binary packages ===
49
50
51=== modified file 'lib/lp/registry/interfaces/distribution.py'
52--- lib/lp/registry/interfaces/distribution.py 2010-03-05 04:12:27 +0000
53+++ lib/lp/registry/interfaces/distribution.py 2010-03-09 17:46:07 +0000
54@@ -219,7 +219,8 @@
55 # properties
56 currentseries = exported(
57 Reference(
58- Interface, # Really IDistroSeries, see _schema_circular_imports.py.
59+ # Really IDistroSeries, see _schema_circular_imports.py.
60+ Interface,
61 title=_("Current series"),
62 description=_(
63 "The current development series of this distribution. "
64@@ -405,17 +406,21 @@
65 # _schema_circular_imports.py.
66 @operation_returns_collection_of(Interface)
67 @export_read_operation()
68- def searchSourcePackages(text):
69+ def searchSourcePackages(text, has_packaging=None):
70 """Search for source packages that correspond to the given text.
71
72 This method just decorates the result of searchSourcePackageCaches()
73 to return DistributionSourcePackages.
74 """
75
76- def searchSourcePackageCaches(text):
77+ def searchSourcePackageCaches(text, has_packaging=None):
78 """Search for source packages that correspond to the given text.
79
80 :param text: The text that will be matched.
81+ :param has_packaging: If True, it will filter out
82+ packages with no packaging (i.e. no link to the upstream
83+ project). False will do the reverse filtering, and None
84+ will do no filtering on this field.
85 :return: A result set containing
86 (DistributionSourcePackageCache, SourcePackageName, rank) tuples
87 ordered by rank.
88
89=== modified file 'lib/lp/registry/model/distribution.py'
90--- lib/lp/registry/model/distribution.py 2010-03-06 21:38:23 +0000
91+++ lib/lp/registry/model/distribution.py 2010-03-09 17:46:07 +0000
92@@ -927,7 +927,8 @@
93 cache.binpkgdescriptions = ' '.join(sorted(binpkgdescriptions))
94 cache.changelog = ' '.join(sorted(sprchangelog))
95
96- def searchSourcePackageCaches(self, text):
97+ def searchSourcePackageCaches(
98+ self, text, has_packaging=None):
99 """See `IDistribution`."""
100 # The query below tries exact matching on the source package
101 # name as well; this is because source package names are
102@@ -948,25 +949,40 @@
103 )
104 ]
105
106+
107+ packaging_query = """
108+ SELECT 1
109+ FROM Packaging
110+ WHERE Packaging.sourcepackagename = SourcePackageName.id
111+ """
112+ has_packaging_condition = ''
113+ if has_packaging is True:
114+ has_packaging_condition = 'AND EXISTS (%s)' % packaging_query
115+ elif has_packaging is False:
116+ has_packaging_condition = 'AND NOT EXISTS (%s)' % packaging_query
117+
118 # Note: When attempting to convert the query below into straight
119 # Storm expressions, a 'tuple index out-of-range' error was always
120 # raised.
121- dsp_caches_with_ranks = store.using(*origin).find(
122- find_spec,
123- """distribution = %s AND
124+ condition = """
125+ distribution = %s AND
126 archive IN %s AND
127 (fti @@ ftq(%s) OR
128 DistributionSourcePackageCache.name ILIKE '%%' || %s || '%%')
129+ %s
130 """ % (quote(self), quote(self.all_distro_archive_ids),
131- quote(text), quote_like(text))
132+ quote(text), quote_like(text), has_packaging_condition)
133+ dsp_caches_with_ranks = store.using(*origin).find(
134+ find_spec, condition
135 ).order_by('rank DESC')
136
137 return dsp_caches_with_ranks
138
139- def searchSourcePackages(self, text):
140+ def searchSourcePackages(self, text, has_packaging=None):
141 """See `IDistribution`."""
142
143- dsp_caches_with_ranks = self.searchSourcePackageCaches(text)
144+ dsp_caches_with_ranks = self.searchSourcePackageCaches(
145+ text, has_packaging=has_packaging)
146
147 # Create a function that will decorate the resulting
148 # DistributionSourcePackageCaches, converting
149
150=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
151--- lib/lp/registry/model/distributionsourcepackage.py 2010-03-08 11:22:05 +0000
152+++ lib/lp/registry/model/distributionsourcepackage.py 2010-03-09 17:46:07 +0000
153@@ -22,6 +22,8 @@
154
155 from canonical.database.sqlbase import sqlvalues
156 from canonical.launchpad.database.emailaddress import EmailAddress
157+from lp.registry.model.distroseries import DistroSeries
158+from lp.registry.model.packaging import Packaging
159 from lp.registry.model.structuralsubscription import (
160 StructuralSubscriptionTargetMixin)
161 from canonical.launchpad.interfaces.lpstorm import IStore
162@@ -315,12 +317,18 @@
163
164 @property
165 def upstream_product(self):
166- for distroseries in self.distribution.series:
167- source_package = distroseries.getSourcePackage(
168- self.sourcepackagename)
169- if source_package.direct_packaging is not None:
170- return source_package.direct_packaging.productseries.product
171- return None
172+ store = Store.of(self.sourcepackagename)
173+ condition = And(
174+ Packaging.sourcepackagename == self.sourcepackagename,
175+ Packaging.distroseriesID == DistroSeries.id,
176+ DistroSeries.distribution == self.distribution
177+ )
178+ result = store.find(Packaging, condition)
179+ result.order_by("debversion_sort_key(version) DESC")
180+ if result.count() == 0:
181+ return None
182+ else:
183+ return result[0].productseries.product
184
185 # XXX kiko 2006-08-16: Bad method name, no need to be a property.
186 @property
187@@ -353,8 +361,6 @@
188
189 def getReleasesAndPublishingHistory(self):
190 """See `IDistributionSourcePackage`."""
191- # Local import of DistroSeries to avoid import loop.
192- from lp.registry.model.distroseries import DistroSeries
193 store = Store.of(self.distribution)
194 result = store.find(
195 (SourcePackageRelease, SourcePackagePublishingHistory),
196
197=== modified file 'lib/lp/registry/stories/product/xx-product-index.txt'
198--- lib/lp/registry/stories/product/xx-product-index.txt 2010-03-01 21:48:15 +0000
199+++ lib/lp/registry/stories/product/xx-product-index.txt 2010-03-09 17:46:07 +0000
200@@ -359,7 +359,7 @@
201 >>> print extract_text(
202 ... find_tag_by_id(user_browser.contents, 'portlet-packages'))
203 All packages
204- Packages in distributions
205+ Packages in Ubuntu
206 “mozilla-firefox” source package in Hoary
207 “mozilla-firefox” source package in Warty Version 0.9 uploaded on...
208
209
210=== modified file 'lib/lp/registry/templates/product-portlet-packages.pt'
211--- lib/lp/registry/templates/product-portlet-packages.pt 2010-03-01 21:48:15 +0000
212+++ lib/lp/registry/templates/product-portlet-packages.pt 2010-03-09 17:46:07 +0000
213@@ -6,14 +6,14 @@
214 define="packages context/sourcepackages">
215
216 <div class="portlet" id="portlet-packages">
217+ <h2>
218+ <span class="see-all"><a
219+ tal:attributes="href context/menu:overview/packages/fmt:url">
220+ All packages</a></span>
221+ Packages in Ubuntu
222+ </h2>
223
224 <tal:has_packages condition="packages">
225- <h2>
226- <span class="see-all"><a
227- tal:attributes="href context/menu:overview/packages/fmt:url">All
228- packages</a></span>
229- Packages in distributions
230- </h2>
231
232 <ul>
233 <tal:pair tal:repeat="package packages">
234@@ -35,12 +35,6 @@
235 </tal:has_packages>
236
237 <tal:has_no_packages condition="not:packages">
238- <h2>
239- <span class="see-all"><a
240- tal:attributes="href context/menu:overview/packages/fmt:url">All
241- packages</a></span>
242- Packages in Ubuntu
243- </h2>
244
245 <p>
246 Launchpad doesn't know which Ubuntu packages this project