Merge lp:~sinzui/launchpad/hold-message into lp:launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 11685 | ||||
Proposed branch: | lp:~sinzui/launchpad/hold-message | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
334 lines (+175/-87) 5 files modified
lib/lp/services/mailman/doc/postings.txt (+0/-76) lib/lp/services/mailman/monkeypatches/lpsize.py (+34/-3) lib/lp/services/mailman/testing/__init__.py (+7/-3) lib/lp/services/mailman/tests/test_lpmoderate.py (+5/-5) lib/lp/services/mailman/tests/test_lpsize.py (+129/-0) |
||||
To merge this branch: | bzr merge lp:~sinzui/launchpad/hold-message | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Edwin Grubbs (community) | code | Approve | |
Review via email: mp+37750@code.launchpad.net |
Description of the change
This is my branch to forward small messages to the moderation queue.
lp:~sinzui/launchpad/hold-message
Diff size: 311
Launchpad bug:
https:/
Test command: ./bin/test -vv \
-t test_lpsize
Pre-
Target release: 10.10
Forward small messages to the moderation queue
-------
OOPS-1726XMLP285 shows that mailman forwarded a message for moderation that
exceeds the db storage limit.
There are two views on this issue. The first is that this is a copy of a
message mailman wants an admin to decide if the real message should be sent to
the list or discarded--the person only needs to see enough to make a decision.
the second issue is that mailman has rules for handling large messages and
they do not appear to agree with Lp. The soft limit enters moderation (which
is this case), and the hard limit is discarded immediately. I recall the soft
limit was raised at the request of statik.
Rules
-----
* Update the lpsize tests to be unittests and delete the tests from
postings.txt (often fails because of timeouts)
* Add a rule to truncate the message so that it is small enough to
be moderated.
* Delete non text/plain parts because Lp only stores text/plain.
* Truncate large text/parts. 10k will be enough for the moderator
to make a decision to discard or forward. This message is shown
in a list of messages; we want to limit the text to ensure the page
is usable.
QA
--
* Send an email with a large attachment to a list on staging.
the attachment must be larger than 11m and not over 20m
* Verify the message appears in the moderation queue
Lint
----
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
Test
----
* lib/lp/
* Removed redundant test that were prone to fail
* lib/lp/
* Fixed the test harness to make a proper email with an attachment.
These tests were the first to really exercise it.
* lib/lp/
* Converted the doctests to unittests
* Added tests for the truncated_message function.
Implementation
--------------
* lib/lp/
* Added truncated_message to create a small message and used it when
calling hold().
Hi Curtis,
This is a nice improvement. I have some comments below.
-Edwin
>=== modified file 'lib/lp/ services/ mailman/ monkeypatches/ lpsize. py' services/ mailman/ monkeypatches/ lpsize. py 2010-08-20 20:31:18 +0000 services/ mailman/ monkeypatches/ lpsize. py 2010-10-06 15:36:24 +0000 Handlers. LPModerate import hold Logging. Syslog import syslog message( original_ message, limit=10000): from_string( message. as_string( ), Message.Message) .typed_ subpart_ iterator( message, 'multipart'): get_content_ maintype( ) == 'multipart': get_content_ type() == 'text/plain':
>--- lib/lp/
>+++ lib/lp/
>@@ -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.
> from Mailman.
>
>
>+def truncated_
>+ """Create a smaller version of the message for moderation."""
>+ message = email.message_
>+ original_
>+ for part in email.iterators
>+ subparts = part.get_payload()
>+ removeable = []
>+ for subpart in subparts:
>+ if subpart.
>+ # This part is handled in the outer loop.
>+ continue
>+ elif subpart.
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 get_payload( ).strip( ) set_payload(
>+ # information for the moderator to make a decision.
>+ content = subpart.
>+ if len(content) > limit:
>+ subpart.
>+ 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( )) append( subpart) remove( subpart) services/ mailman/ tests/test_ lpsize. py' services/ mailman/ tests/test_ lpsize. py 1970-01-01 00:00:00 +0000 services/ mailman/ tests/test_ lpsize. py 2010-10-06 15:36:24 +0000
>+ else:
>+ removeable.
>+ for subpart in removeable:
>+ subparts.
>+ return message
>+
>+
> def process(mlist, msg, msgdata):
> """Check the message size (approximately) against hard and soft limits.
>
>=== added file 'lib/lp/
>--- lib/lp/
>+++ lib/lp/
>@@ -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). application import MIMEApplication nalLayer, onalLayer, mailman. testing import MailmanTestCase ase(MailmanTest Case):
>+"""Test the lpsize monekypatches"""
>+
>+from __future__ import with_statement
>+
>+__metaclass__ = type
>+__all__ = []
>+
>+from email.mime.
>+
>+from Mailman import Errors
>+from Mailman.Handlers import LPSize
>+
>+from canonical.config import config
>+from canonical.testing import (
>+ DatabaseFunctio
>+ LaunchpadFuncti
>+ )
>+from lp.services.
>+
>+
>+class TestLPSizeTestC
>+ """Test LPSize.
>+
>+ Mailman process() methods quietly return. They may...