Launchpad itself

Merge lp:~stevenk/launchpad/getpublishedsources-too-eager into lp:launchpad

  1. getpublishedsources-too-eager
  2. Merge into devel
Proposed by Steve Kowalik
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 12802
Proposed branch: lp:~stevenk/launchpad/getpublishedsources-too-eager
Merge into: lp:launchpad
Diff against target: 55 lines (+9/-3)
3 files modified
lib/lp/soyuz/browser/archive.py (+2/-1)
lib/lp/soyuz/interfaces/archive.py (+3/-1)
lib/lp/soyuz/model/archive.py (+4/-1)
To merge this branch: bzr merge lp:~stevenk/launchpad/getpublishedsources-too-eager
Reviewer Review Type Date Requested Status
William Grant code* Approve
Robert Collins (community) Approve
Review via email: mp+57107@code.launchpad.net

Commit message

[no-qa] [r=lifeless,wgrant][bug=744849] Don't eager load by default in IArchive.getPublishedSources(), but instruct the API that it always should.

Description of the change

Add a eager_load parameter to IArchive.getPublishedSources() that disables eager loading for those callsites that do not require it. It defaults to True, so the API/page renders will always get it. So far the only callsite that disables it is the package cloner.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) :
review: Approve
Revision history for this message
William Grant (wgrant) wrote :

Thanks for the default inversion. s/if eager_load is False/if not eager_load/, otherwise fine.

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/soyuz/browser/archive.py'
2--- lib/lp/soyuz/browser/archive.py 2011-03-29 13:49:07 +0000
3+++ lib/lp/soyuz/browser/archive.py 2011-04-12 01:45:06 +0000
4@@ -824,7 +824,8 @@
5 return self.context.getPublishedSources(
6 name=self.specified_name_filter,
7 status=self.getSelectedFilterValue('status_filter'),
8- distroseries=self.getSelectedFilterValue('series_filter'))
9+ distroseries=self.getSelectedFilterValue('series_filter'),
10+ eager_load=True)
11
12 @property
13 def default_status_filter(self):
14
15=== modified file 'lib/lp/soyuz/interfaces/archive.py'
16--- lib/lp/soyuz/interfaces/archive.py 2011-02-24 15:30:54 +0000
17+++ lib/lp/soyuz/interfaces/archive.py 2011-04-12 01:45:06 +0000
18@@ -987,11 +987,13 @@
19 required=False))
20 # Really returns ISourcePackagePublishingHistory, see below for
21 # patch to avoid circular import.
22+ @call_with(eager_load=True)
23 @operation_returns_collection_of(Interface)
24 @export_read_operation()
25 def getPublishedSources(name=None, version=None, status=None,
26 distroseries=None, pocket=None,
27- exact_match=False, created_since_date=None):
28+ exact_match=False, created_since_date=None,
29+ eager_load=False):
30 """All `ISourcePackagePublishingHistory` target to this archive.
31
32 :param name: source name filter (exact match or SQL LIKE controlled
33
34=== modified file 'lib/lp/soyuz/model/archive.py'
35--- lib/lp/soyuz/model/archive.py 2011-03-23 18:29:09 +0000
36+++ lib/lp/soyuz/model/archive.py 2011-04-12 01:45:06 +0000
37@@ -465,7 +465,8 @@
38
39 def getPublishedSources(self, name=None, version=None, status=None,
40 distroseries=None, pocket=None,
41- exact_match=False, created_since_date=None):
42+ exact_match=False, created_since_date=None,
43+ eager_load=False):
44 """See `IArchive`."""
45 # clauses contains literal sql expressions for things that don't work
46 # easily in storm : this method was migrated from sqlobject but some
47@@ -526,6 +527,8 @@
48 resultset = store.find(SourcePackagePublishingHistory,
49 *storm_clauses).order_by(
50 *orderBy)
51+ if not eager_load:
52+ return resultset
53 # Its not clear that this eager load is necessary or sufficient, it
54 # replaces a prejoin that had pathological query plans.
55 def eager_load(rows):