Closed bitdancer closed 12 years ago
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.
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.
Oh and a minor thing to avoid whitespace-only commits later: our rst docs use three-space indents.
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.
New changeset 9388c671d52d by R David Murray in branch 'default': bpo-14731: refactor email policy framework. http://hg.python.org/cpython/rev/9388c671d52d
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.
New changeset 8ba99b810b40 by R David Murray in branch 'default': bpo-14731: add preliminary What's New entry for policy framework. http://hg.python.org/cpython/rev/8ba99b810b40
New changeset 09e97829ed1e by R David Murray in branch 'default': Add news entries for bpo-14731 and bpo-12586. http://hg.python.org/cpython/rev/09e97829ed1e
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields: ```python assignee = 'https://github.com/bitdancer' closed_at =
created_at =
labels = ['type-feature', 'expert-email']
title = 'Enhance Policy framework in preparation for adding email6 policies as provisional'
updated_at =
user = 'https://github.com/bitdancer'
```
bugs.python.org fields:
```python
activity =
actor = 'python-dev'
assignee = 'r.david.murray'
closed = True
closed_date =
closer = 'r.david.murray'
components = ['email']
creation =
creator = 'r.david.murray'
dependencies = []
files = ['25467']
hgrepos = ['121']
issue_num = 14731
keywords = ['patch']
message_count = 8.0
messages = ['160015', '161228', '161229', '161231', '161601', '161602', '161626', '161639']
nosy_count = 5.0
nosy_names = ['barry', 'eric.araujo', 'r.david.murray', 'python-dev', 'hynek']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue14731'
versions = ['Python 3.3']
```