Hi Curtis,

This is a nice improvement. I have some comments below.

-Edwin


>=== modified file 'lib/lp/services/mailman/monkeypatches/lpsize.py'
>--- lib/lp/services/mailman/monkeypatches/lpsize.py	2010-08-20 20:31:18 +0000
>+++ lib/lp/services/mailman/monkeypatches/lpsize.py	2010-10-06 15:36:24 +0000
>@@ -3,15 +3,43 @@
> 
> """A pipeline handler for checking message sizes."""
> 
>-# pylint: disable-msg=F0401
>+import email
>+
> from Mailman import (
>     Errors,
>+    Message,
>     mm_cfg,
>     )
> from Mailman.Handlers.LPModerate import hold
> from Mailman.Logging.Syslog import syslog
> 
> 
>+def truncated_message(original_message, limit=10000):
>+    """Create a smaller version of the message for moderation."""
>+    message = email.message_from_string(
>+        original_message.as_string(), Message.Message)
>+    for part in email.iterators.typed_subpart_iterator(message, 'multipart'):
>+        subparts = part.get_payload()
>+        removeable = []
>+        for subpart in subparts:
>+            if subpart.get_content_maintype() == 'multipart':
>+                # This part is handled in the outer loop.
>+                continue
>+            elif subpart.get_content_type() == 'text/plain':



Shouldn't it truncate text/html or any other "text/" content-type,
since the user's mail client might create an empty text/plain section?



>+                # Truncate the message at 10kb so that there is enough
>+                # information for the moderator to make a decision.
>+                content = subpart.get_payload().strip()
>+                if len(content) > limit:
>+                    subpart.set_payload(
>+                        content[:limit] + ' [truncated for moderation]',



It seems like [truncated for moderation] will be easier to see if
it is on a new line.



>+                        subpart.get_charset())
>+            else:
>+                removeable.append(subpart)
>+        for subpart in removeable:
>+            subparts.remove(subpart)
>+    return message
>+
>+
> def process(mlist, msg, msgdata):
>     """Check the message size (approximately) against hard and soft limits.
> 
>=== added file 'lib/lp/services/mailman/tests/test_lpsize.py'
>--- lib/lp/services/mailman/tests/test_lpsize.py	1970-01-01 00:00:00 +0000
>+++ lib/lp/services/mailman/tests/test_lpsize.py	2010-10-06 15:36:24 +0000
>@@ -0,0 +1,129 @@
>+# Copyright 20010 Canonical Ltd.  This software is licensed under the



I think the copyright will expire by the time Bender and Leela can use
it.



>+# GNU Affero General Public License version 3 (see the file LICENSE).
>+"""Test the lpsize monekypatches"""
>+
>+from __future__ import with_statement
>+
>+__metaclass__ = type
>+__all__ = []
>+
>+from email.mime.application import MIMEApplication
>+
>+from Mailman import Errors
>+from Mailman.Handlers import LPSize
>+
>+from canonical.config import config
>+from canonical.testing import (
>+    DatabaseFunctionalLayer,
>+    LaunchpadFunctionalLayer,
>+    )
>+from lp.services.mailman.testing import MailmanTestCase
>+
>+
>+class TestLPSizeTestCase(MailmanTestCase):
>+    """Test LPSize.
>+
>+    Mailman process() methods quietly return. They may set msg_data key-values
>+    or raise an error to end processing. This group of tests tests often check



s/tests tests/tests/
Either, use "These tests" instead of "This group of tests" or change
"check" to "checks".


>+    for errors, but that does not mean there is an error condition, it only
>+    means message processing has reached a final decision. Messages that do
>+    not cause a final decision pass-through and the process() methods ends


"pass-through" with a hyphen looks like a noun instead of a verb.
Add a comma before the "and".


>+    without a return.
>+    """
>+
>+    layer = LaunchpadFunctionalLayer
>+
>+    def setUp(self):
>+        super(TestLPSizeTestCase, self).setUp()
>+        self.team, self.mailing_list = self.factory.makeTeamAndMailingList(
>+            'team-1', 'team-1-owner')
>+        self.mm_list = self.makeMailmanList(self.mailing_list)
>+        self.subscriber_email = self.team.teamowner.preferredemail.email
>+
>+    def tearDown(self):
>+        super(TestLPSizeTestCase, self).tearDown()
>+        self.cleanMailmanList(self.mm_list)
>+
>+    def test_process_size_under_soft_limit(self):
>+        # Any message under 40kb is sent to the list.
>+        attachment = MIMEApplication(
>+            '\n'.join(['x' * 20] * 1000), 'octet-stream')
>+        message = self.makeMailmanMessage(
>+            self.mm_list, self.subscriber_email, 'subject', 'content',
>+            attachment=attachment)
>+        msg_data = {}
>+        silence = LPSize.process(self.mm_list, message, msg_data)
>+        self.assertEqual(None, silence)
>+
>+    def test_process_size_over_soft_limit_held(self):
>+        # Messages over 40km held for moderation.



Uh, based on how many millimeters per character?



>+        self.assertEqual(40000, config.mailman.soft_max_size)
>+        attachment = MIMEApplication(
>+            '\n'.join(['x' * 40] * 1000), 'octet-stream')
>+        message = self.makeMailmanMessage(
>+            self.mm_list, self.subscriber_email, 'subject', 'content',
>+            attachment=attachment)
>+        msg_data = {}
>+        args = (self.mm_list, message, msg_data)
>+        self.assertRaises(
>+            Errors.HoldMessage, LPSize.process, *args)
>+        self.assertEqual(1, self.mailing_list.getReviewableMessages().count())
>+
>+    def test_process_size_over_hard_limit_discarded(self):
>+        # Messages over 1MB are discarded.
>+        self.assertEqual(1000000, config.mailman.hard_max_size)
>+        attachment = MIMEApplication(
>+            '\n'.join(['x' * 1000] * 1000), 'octet-stream')
>+        message = self.makeMailmanMessage(
>+            self.mm_list, self.subscriber_email, 'subject', 'content',
>+            attachment=attachment)
>+        msg_data = {}
>+        args = (self.mm_list, message, msg_data)
>+        self.assertRaises(
>+            Errors.DiscardMessage, LPSize.process, *args)
>+        self.assertEqual(0, self.mailing_list.getReviewableMessages().count())
>+