classification
Title: Enhance Policy framework in preparation for adding email6 policies as provisional
Type: enhancement Stage: patch review
Components: email Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: r.david.murray Nosy List: barry, eric.araujo, hynek, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2012-05-05 18:14 by r.david.murray, last changed 2012-05-26 02:33 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
676f9c8c28c6.diff r.david.murray, 2012-05-05 18:16 review
Repositories containing patches
http://hg.python.org/features/email6#nemail6base
Messages (8)
msg160015 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-05-05 18:14
As discussed in my email to python-dev, I'm planning to add the new header parsing to Python 3.3 as a provisional extension, by adding a (set) of policies that are clearly marked provisional in the documentation.  In order for this to work, I first need to make certain enhancements to the Policy framework that I previously committed to default.

When reviewing the patch, please start with the 'architecture.rst' file in the Lib/email directory, which provides an overview of the changes and the plan.

The primary changes are to add some new policy hooks that feedparser, message, and generator call.  These hooks are then implemented on the renamed default policy (now called 'compat32') in such a way as to replicate the behavior of Python 3.2.  The other significant change is that Message objects now have a policy.  In order to accommodate this, this patch adds 'policy propagation' logic.  That is, if you pass a policy to feedparser, then all the Message objects it creates get that policy.  And when you flatten a message with a Generator, it uses the policy attached to the message unless you explicitly override it.

I also factored policy into _basepolicy.py and policy.py.  This doesn't mean much for this patch, but when the new policies land they go in policy.py, while the default policy is imported from _policybase.  This means that if a Python 3.3 program does not explicitly use a policy, it will not import any of the new code.

The remaining changes in the patch are some test reorganization.  Since the goal is now 100% backward compatibility with Python 3.2 (including bugs in some cases), the patch removes some tests that were previously added to test.test_email.test_email and puts them into module specific test files (test_generator, test_parser, test_policy).  Additional tests are also added.

The end result of the test refactoring is that if you diff test_email.py after this patch against the 3.2 test_email.py, you should find a few places where the scaffolding is changed and the addition of some tests for bug fixes in the 3.2 code.  Otherwise the tests should be identical to 3.2, with all the policy related tests moving into test_policy.

The longer term plan for this is to replicate all of the test_email.py tests in appropriate module-specific test files (I've already marked a few such tests that got replicated in test_policy).  By the time we get to Python4 the plan is for all the tests to be replicated, so that we can at that point drop compat32 by removing the policy and the test_email.py test file, as well as the code that will be at that point obsolete.

The primary review here should be to make sure I am in fact producing 100% backward compatibility.  Any other review comments will be welcome, of course.

PS: I also changed the 'must_be_7bit' policy control, whose name I never liked, to be 'cte_type', which can be '7bit' or '8bit'.  As noted in the architecture.rst file, the extra motivation for this is that there will eventually be a 'cte_type=unicode' which will support unicode display of formatted messages and, perhaps even more important, rfc 5335.
msg161228 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-05-20 19:58
A few thoughts:

- If policies are intended to be expanded with the clone method, I think the classes (here Compat32) don’t need to be made public.

- IIUC the policy framework supersedes various arguments like manglefrom or max_line_length, right?  If so there could be a bit of doc explaining how to switch from old-style usage to new policy goodness.
msg161229 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-05-20 19:59
Oh and a minor thing to avoid whitespace-only commits later: our rst docs use three-space indents.
msg161231 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-05-20 20:18
I checked the indent fix into the wrong branch in my repo.  It's in the other patch.  I could fix that if desired.

The reason for making the base class public is to allow subclassing.  Perhaps that is not needed for Compat32, though.

I don't think that the policy argument obsoletes the existing method options.  It is easier to write:

  gen.flatten(msg, linesep='\r\n')

than it is to write

  gen.flatten(msg, policy=msg.policy.clone(linesep='\r\n'))

That is, when you want to make a change in just one method call, as opposed to globally, then the existing arguments are more convenient.

That said, a section in What's New describing when it might be a good idea to move to a global policy setting vs using the existing arguments may be worthwhile.
msg161601 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-25 19:02
New changeset 9388c671d52d by R David Murray in branch 'default':
#14731: refactor email policy framework.
http://hg.python.org/cpython/rev/9388c671d52d
msg161602 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-05-25 19:05
I decided to keep the Compat32 class public both because it simplifies the documentation and because there doesn't seem to be a good reason to hide it.

I'm leaving this issue open pending adding some discussion to What's New.
msg161626 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-25 22:00
New changeset 8ba99b810b40 by R David Murray in branch 'default':
#14731: add preliminary What's New entry for policy framework.
http://hg.python.org/cpython/rev/8ba99b810b40
msg161639 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-26 02:33
New changeset 09e97829ed1e by R David Murray in branch 'default':
Add news entries for #14731 and #12586.
http://hg.python.org/cpython/rev/09e97829ed1e
History
Date User Action Args
2012-05-26 02:33:51python-devsetmessages: + msg161639
2012-05-25 22:00:36r.david.murraysetstatus: open -> closed
resolution: fixed
2012-05-25 22:00:09python-devsetmessages: + msg161626
2012-05-25 20:24:37r.david.murraysetassignee: r.david.murray
2012-05-25 19:05:04r.david.murraysetmessages: + msg161602
2012-05-25 19:02:47python-devsetnosy: + python-dev
messages: + msg161601
2012-05-24 03:10:03r.david.murraylinkissue1590744 dependencies
2012-05-20 20:18:51r.david.murraysetmessages: + msg161231
2012-05-20 19:59:40eric.araujosetmessages: + msg161229
2012-05-20 19:58:37eric.araujosetnosy: + eric.araujo
messages: + msg161228
2012-05-19 23:42:05r.david.murraylinkissue12586 dependencies
2012-05-19 23:37:11r.david.murraysetcomponents: + email
2012-05-05 18:55:22hyneksetnosy: + hynek
2012-05-05 18:18:53r.david.murraysettitle: Enhance Policy framework in preparation for adding "eamil6" policy as provisional -> Enhance Policy framework in preparation for adding email6 policies as provisional
2012-05-05 18:16:43r.david.murraysetfiles: + 676f9c8c28c6.diff
keywords: + patch
2012-05-05 18:14:47r.david.murraycreate