launchpad-reviewers team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Gary Poster has proposed merging lp:~gary/launchpad/bug627442 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #627442 lazr.restful and batching cause a FeatureError
  https://bugs.launchpad.net/bugs/627442


This branch fixes bug 620506 and bug 627442.  These are both caused by underlying bugs in Storm.  I brought the pertinent fix back to 0.17, so the new, custom storm release is 0.17 with only the cherrypick of the pertinent fix.  The Storm branch includes a test, which passes, and which I am considering to be the test for this branch.

I was able to remove a previous hack around the problem because of this change.  Previous tests added at that time continue to pass.

Local QA: https://api.launchpad.dev/beta/~landscape-developers/members?ws.size=1 breaks on devel, and works with this change.

Lint is happy.

This is a release-critical candidate.
-- 
https://code.launchpad.net/~gary/launchpad/bug627442/+merge/34701
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug627442 into lp:launchpad/devel.

=== modified file 'lib/canonical/launchpad/webapp/batching.py'
--- lib/canonical/launchpad/webapp/batching.py	2010-08-25 19:32:37 +0000
+++ lib/canonical/launchpad/webapp/batching.py	2010-09-06 18:54:46 +0000
@@ -4,13 +4,10 @@
 __metaclass__ = type
 
 import lazr.batchnavigator
-from storm import Undef
-# and ISQLObjectResultSet
 from storm.zope.interfaces import IResultSet
 from zope.component import adapts
 from zope.interface import implements
 from zope.interface.common.sequence import IFiniteSequence
-from zope.security.proxy import removeSecurityProxy
 
 from canonical.config import config
 from canonical.launchpad.webapp.interfaces import ITableBatchNavigator
@@ -32,24 +29,6 @@
         return iter(self.context)
 
     def __len__(self):
-        # XXX 2010-08-24 leonardr bug=620508
-        #
-        # Slicing a ResultSet object returns a copy with ._limit and
-        # ._offset set appropriately. The original object's ._limit
-        # and ._offset are not affected. However, the original and
-        # the copy share a _select object, which means the original
-        # object's ._select.limit and ._select.offset are shared with
-        # the copy.
-        #
-        # This breaks Storm--count() is not supported on a ResultSet
-        # that has a limit or offset set. This code sets
-        # ._select.limit and ._select.offset to the appropriate values
-        # before running the count() query, just as __getitem__ sets
-        # those values before running its query.
-        resultset = removeSecurityProxy(self.context)
-        if hasattr(resultset, '_select'):
-            resultset._select.limit = Undef
-            resultset._select.offset = Undef
         return self.context.count()
 
 

=== modified file 'versions.cfg'
--- versions.cfg	2010-09-05 10:46:30 +0000
+++ versions.cfg	2010-09-06 18:54:46 +0000
@@ -64,7 +64,10 @@
 simplesettings = 0.4
 SimpleTal = 4.1
 sourcecodegen = 0.6.9
-storm = 0.17
+# lp:~gary/storm/0.17-launchpad has the branch for this distribution.  It is
+# 0.17 plus r374 from storm trunk for fixing lp:620508 (in order to address
+# lp:627442).
+storm = 0.17-launchpad-1
 testtools = 0.9.6
 transaction = 1.0.0
 Twisted = 10.1.0