← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~abentley/launchpad/fix-groupby into lp:launchpad/devel

 

Aaron Bentley has proposed merging lp:~abentley/launchpad/fix-groupby into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


= Summary =
Prevent SecurityProxy from erroring when itertools.groupby is used.

== Proposed fix ==
Handle itertools.groupby like other built-in iterators.

== Pre-implementation notes ==
Pre-implementation was with gary

== Implementation details ==
_iteratorChecker is used for built-in python iterators.  A shame it's got an
underscored name.

I guess I could phrase it this way:
checker.BasicTypes[itertools.groupby] = checker.BasicTypes[type(iter([]))]

But that only gets rid of the underscore, not the underlying problem that this
checker is not provided as a public interface.

== Tests ==
bin/test -t test_include_superseded_comments test_branchmergeproposal -v

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/model/branchmergeproposal.py
  lib/lp_sitecustomize.py

./lib/lp_sitecustomize.py
      22: E302 expected 2 blank lines, found 1
      27: E302 expected 2 blank lines, found 1
      37: E302 expected 2 blank lines, found 1
-- 
https://code.launchpad.net/~abentley/launchpad/fix-groupby/+merge/36360
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/fix-groupby into lp:launchpad/devel.
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py	2010-09-15 20:41:46 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2010-09-22 19:03:51 +0000
@@ -809,12 +809,7 @@
             branch_revision for branch_revision, revision, revision_author
             in resultset)
         # Now group by date created.
-        gby = groupby(branch_revisions, lambda r: r.revision.date_created)
-        # Use a generator expression to wrap the custom iterator so it doesn't
-        # get security-proxied.
-        return (
-            (date, (revision for revision in revisions))
-            for date, revisions in gby)
+        return groupby(branch_revisions, lambda r: r.revision.date_created)
 
 
 class BranchMergeProposalGetter:

=== modified file 'lib/lp_sitecustomize.py'
--- lib/lp_sitecustomize.py	2010-07-26 13:35:20 +0000
+++ lib/lp_sitecustomize.py	2010-09-22 19:03:51 +0000
@@ -4,6 +4,7 @@
 # This file is imported by parts/scripts/sitecustomize.py, as set up in our
 # buildout.cfg (see the "initialization" key in the "[scripts]" section).
 
+import itertools
 import os
 import warnings
 import logging
@@ -45,6 +46,7 @@
     os.environ['STORM_CEXTENSIONS'] = '1'
     customizeMimetypes()
     dont_wrap_class_and_subclasses(Branch)
+    checker.BasicTypes[itertools.groupby] = checker._iteratorChecker
     silence_warnings()
     silence_bzr_logger()