Author r.david.murray
Recipients barry, r.david.murray, timb07
Date 2017-06-16.17:44:49
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1497635090.05.0.426894578404.issue30681@psf.upfronthosting.co.za>
In-reply-to
Content
OK, I think I've reloaded my brain at least partially on this topic.

I think my original reason for having prasedate_to_datetime raise errors is that it was my opinion that that is the way it should work, and that parsedate should work the same way (raise errors, not return None).  The logic is that parsedate is not itself part of the *parser* and it is the parser that has a contract to not raise errors but instead register defects.  When you call parsedate from your code (that is, not as part of the parser), it ought to raise an error, IMO, and so I made parsedate_to_datetime do that.

I think I understand the logic behind the original behavior: None as the 'error value', thus being consistent with the parser in not raising errors.  But I think our understanding of Python best practices has evolved (solidified?) since the days when the parsedate API was designed, and raising errors is better.

*However*, consistency is also important, so if the consensus is that parsedate_to_datetime should parallel the parsedate API, I'm not going to argue with it.

Regardless of that, however, I think your notion, Tim, that the *string* value of a date header with an invalid date in it should be the invalid string is a good one.  One can check the validity by inspecting the datetime argument.  Regardless of whether errors are reported via None or an exception, the headerregistry method should catch the error and set the value accordingly (to the invalid string on error, to the normalized string if valid).

A couple of notes on the PR you submitted. (1) this change affects only the new policies, so the test should go somewhere in the new tests, not in test_email, which means you don't need to muck with the test support infrastructure in that file.  There are already date header tests in test_headerregistry, so add the new test there. (2) I'm moving us away from putting 'test emails' in separate files, so include the text under test in the test file.  You only need the date string in the date header test, but you can add your sample (changed to meet Brett's child filter, although I bet any children who will be looking at the python source code will already have seen many such spam emails) to test_inversion (which currently only contains one test message in msg_params, add yours  to that list and make it two :)

As for the decision on the return value vs exception, let's see which side Barry comes down on :)
History
Date User Action Args
2017-06-16 17:44:50r.david.murraysetrecipients: + r.david.murray, barry, timb07
2017-06-16 17:44:50r.david.murraysetmessageid: <1497635090.05.0.426894578404.issue30681@psf.upfronthosting.co.za>
2017-06-16 17:44:50r.david.murraylinkissue30681 messages
2017-06-16 17:44:49r.david.murraycreate