Launchpad itself

Merge lp:~leonardr/launchpad/optimized-length-2 into lp:launchpad/db-devel

  1. optimized-length-2
  2. Merge into db-devel
Proposed by Leonard Richardson
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: 9705
Proposed branch: lp:~leonardr/launchpad/optimized-length-2
Merge into: lp:launchpad/db-devel
Diff against target: 126 lines (+51/-21)
3 files modified
lib/canonical/launchpad/pagetests/webservice/datamodel.txt (+29/-19)
lib/canonical/launchpad/webapp/batching.py (+20/-0)
versions.cfg (+2/-2)
To merge this branch: bzr merge lp:~leonardr/launchpad/optimized-length-2
Reviewer Review Type Date Requested Status
Michael Nelson (community) code Approve
Graham Binns (community) code Approve
Aaron Bentley (community) Approve
Review via email: mp+33417@code.launchpad.net

Description of the change

This branch integrates the newest version of lazr.restful, which sends 'total_size' instead of 'total_size_link' if it's easy to figure out the total size of a collection. The changes to datamodel.txt show when this happens.

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) :
review: Approve
Revision history for this message
Leonard Richardson (leonardr) wrote :

I need a follow-up review. I fixed a minor test failure and added a workaround for bug 620508. I'm fairly sure it won't screw up anything else, but the tests will say for sure.

Revision history for this message
Graham Binns (gmb) wrote :

r=me for r11392.

review: Approve (code)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

r=me for the change pasted via irc:

14:51 < noodles775> leonardr: I don't see a one-liner? It's r11442 that needs to be reviewed right? This: http://bazaar.launchpad.net/~leonardr/launchpad/optimized-length-2/revision/11442
14:59 < leonardr> noodles775, 11442 corrects a typo made to the original one-liner. i was hoping to just give you the code so it wouldn't be so complicated
14:59 < leonardr> i'll just do another merge proposal and get in the queue
15:01 < noodles775> leonardr: I'm happy to review just the one-line that's changed, but currently I can only see it in 11442 along with a few other changes. If the rest is already reviewed, just paste a diff and I can review that, otherwise yes, a new MP is probably easier.
15:04 < leonardr> noodles775: i see the problem. i had to create a new branch with the old changes, so all the old changes show up as one revision
15:04 < leonardr> this code is the only code that hasn't been reviewed yet:
15:04 < leonardr> + if hasattr(resultset, '_select'):
15:04 < leonardr> + resultset._select.limit = Undef
15:04 < leonardr> + resultset._select.offset = Undef
15:05 < noodles775> leonardr: r=me (I assume the second two lines were there previously, just not indented). Thanks!
15:05 < leonardr> noodles, yeah

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/pagetests/webservice/datamodel.txt'
2--- lib/canonical/launchpad/pagetests/webservice/datamodel.txt 2010-08-19 14:38:10 +0000
3+++ lib/canonical/launchpad/pagetests/webservice/datamodel.txt 2010-08-26 12:53:02 +0000
4@@ -7,29 +7,39 @@
5 end-to-end tests of code paths that, on the surface, look like they're
6 already tested in lazr.restful.
7
8- >>> def get_collection(version, start=0, size=2):
9+ >>> def get_collection(version="devel", start=0, size=2):
10 ... collection = webservice.get(
11 ... ("/people?ws.op=find&text=s&ws.start=%s&ws.size=%s" %
12 ... (start, size)),
13 ... api_version=version)
14 ... return collection.jsonBody()
15
16-Even if an entire collection fits on one page (so that the total size
17-of the collection is obvious), 'total_size_link' will be served
18-instead of 'total_size'. (It would be nice to fix this.)
19-
20- >>> collection = get_collection("devel", size=100)
21- >>> print sorted(collection.keys())
22- [u'entries', u'start', u'total_size_link']
23- >>> print webservice.get(collection['total_size_link']).jsonBody()
24- 9
25-
26-Even if the last page of the collection is fetched (so that the total
27-size of the collection is semi-obvious), 'total_size_link' will be
28-served instead of 'total_size'. (It would be nice to fix this.)
29-
30- >>> collection = get_collection("devel", start=8)
31- >>> print sorted(collection.keys())
32- [u'entries', u'prev_collection_link', u'start', u'total_size_link']
33- >>> print webservice.get(collection['total_size_link']).jsonBody()
34+
35+Normally, the total size of a collection is not served along with the
36+collection; it's available by following the total_size_link.
37+
38+ >>> collection = get_collection()
39+ >>> print sorted(collection.keys())
40+ [u'entries', u'next_collection_link', u'start', u'total_size_link']
41+ >>> print webservice.get(collection['total_size_link']).jsonBody()
42+ 9
43+
44+If an entire collection fits on one page (making the size of the
45+collection obvious), 'total_size' is served instead of
46+'total_size_link'.
47+
48+ >>> collection = get_collection(size=100)
49+ >>> print sorted(collection.keys())
50+ [u'entries', u'start', u'total_size']
51+ >>> print collection['total_size']
52+ 9
53+
54+If the last page of the collection is fetched (making the total size
55+of the collection semi-obvious), 'total_size' is served instead of
56+'total_size_link'.
57+
58+ >>> collection = get_collection(start=8)
59+ >>> print sorted(collection.keys())
60+ [u'entries', u'prev_collection_link', u'start', u'total_size']
61+ >>> print collection['total_size']
62 9
63
64=== modified file 'lib/canonical/launchpad/webapp/batching.py'
65--- lib/canonical/launchpad/webapp/batching.py 2010-08-20 20:31:18 +0000
66+++ lib/canonical/launchpad/webapp/batching.py 2010-08-26 12:53:02 +0000
67@@ -4,11 +4,13 @@
68 __metaclass__ = type
69
70 import lazr.batchnavigator
71+from storm import Undef
72 # and ISQLObjectResultSet
73 from storm.zope.interfaces import IResultSet
74 from zope.component import adapts
75 from zope.interface import implements
76 from zope.interface.common.sequence import IFiniteSequence
77+from zope.security.proxy import removeSecurityProxy
78
79 from canonical.config import config
80 from canonical.launchpad.webapp.interfaces import ITableBatchNavigator
81@@ -30,6 +32,24 @@
82 return iter(self.context)
83
84 def __len__(self):
85+ # XXX 2010-08-24 leonardr bug=620508
86+ #
87+ # Slicing a ResultSet object returns a copy with ._limit and
88+ # ._offset set appropriately. The original object's ._limit
89+ # and ._offset are not affected. However, the original and
90+ # the copy share a _select object, which means the original
91+ # object's ._select.limit and ._select.offset are shared with
92+ # the copy.
93+ #
94+ # This breaks Storm--count() is not supported on a ResultSet
95+ # that has a limit or offset set. This code sets
96+ # ._select.limit and ._select.offset to the appropriate values
97+ # before running the count() query, just as __getitem__ sets
98+ # those values before running its query.
99+ resultset = removeSecurityProxy(self.context)
100+ if hasattr(resultset, '_select'):
101+ resultset._select.limit = Undef
102+ resultset._select.offset = Undef
103 return self.context.count()
104
105
106
107=== modified file 'versions.cfg'
108--- versions.cfg 2010-08-25 18:51:51 +0000
109+++ versions.cfg 2010-08-26 12:53:02 +0000
110@@ -24,14 +24,14 @@
111 grokcore.component = 1.6
112 httplib2 = 0.6.0
113 ipython = 0.9.1
114-launchpadlib = 1.6.4
115+launchpadlib = 1.6.5
116 lazr.authentication = 0.1.1
117 lazr.batchnavigator = 1.2.2
118 lazr.config = 1.1.3
119 lazr.delegates = 1.2.0
120 lazr.enum = 1.1.2
121 lazr.lifecycle = 1.1
122-lazr.restful = 0.11.1
123+lazr.restful = 0.11.3
124 lazr.restfulclient = 0.10.0
125 lazr.smtptest = 1.1
126 lazr.testing = 0.1.1

Subscribers

People subscribed via source and target branches

to status/vote changes: