Issue 2550870
Created on 2015-02-25 21:07 by ber, last changed 2015-03-20 14:17 by jerrykan.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | Remove |
test_mailgw-20150225.patch | ber, 2015-02-25 22:53 | |||
issue2550870-1.patch | jerrykan, 2015-02-27 12:58 |
Messages | |||
---|---|---|---|
msg5223 | Author: [hidden] (ber) | Date: 2015-02-25 21:07 | |
Roundup as in 1.5.0 or devel rev:2c3cc4ccd024 still uses the rfc822 module. https://docs.python.org/2.7/library/rfc822.html reads "Deprecated since version 2.3: The email package should be used in preference to the rfc822 module. This module is present only to maintain backward compatibility, and has been removed in Python 3." I'm not quite sure what deprecated means, but as we still have a roundup/rfc2822.py which says "Some rfc822 functions taken from the new (python2.3) "email" module.", there is some code cleanup and possibly there are defects fixed in the newer versions of the email module that have not been backported to the rfc822 module. |
|||
msg5224 | Author: [hidden] (r.david.murray) | Date: 2015-02-25 21:20 | |
Deprecated in this case means (as it says) gone in Python3, but it also means unmaintained for longer than that. And yes, there have been a number of fixes to header handling. |
|||
msg5225 | Author: [hidden] (ber) | Date: 2015-02-25 22:53 | |
Attaching test_mailgw-20150225.patch which is an INCOMPLETE start at replacing the rfc822 module with the email module in this single file. Needs more work. |
|||
msg5230 | Author: [hidden] (jerrykan) | Date: 2015-02-27 12:58 | |
I've attached a patch that migrates from the rfc822 module to the email module for files that import it: - roundup/cgi/client.py - roundup/mailgw.py - test/test_mailgw.py The 'roundup/mailgw.py' stuff is pretty straight forward and the tests pass The the section of code in 'roundup/cgi/client.py' that uses the rfc822 module are not covered by the tests, so it may be worth someone double checking these changes. The 'test/test_mailgw.py' is mostly converted but there are a few tests that are failing so I have included skip() decorators for the time being[1]. I've spent a bit of time looking into why the failures are occurring and I think there are a few things contributing to the errors... A lot of the message strings in the tests seem to have headers defined twice (ie. 'FROM:' & 'From:', 'TO:' & 'To:'), and the behaviour for parsing these seems to be different between the rfc822 and email modules. I'm not sure if the duplicate headers were originally intentional - to comply with how the rfc822 module works - or not. The second thing is that the 'roundup/mailgw.py' seems to use the mimetypes module to generate outgoing emails. This is another deprecated module that should probably be replaced with the email module. It also seems to generate messages with duplicate headers. Replacing the mimetypes module would also seem to be a reasonable sized undertaking. So that is about as far as I got with this. The next step would seem to be to try and resolve the issues around the tests that are currently being skipped, so if anyone can shed any light on the duplicate headers things or have any other advice on how to proceed that would be helpful. -- [1] note that the 'skip()' decorators are only available in python 2.7, but that should be OK for the time being as they should be removed in the final patch that fixes the reaming issues. |
|||
msg5231 | Author: [hidden] (r.david.murray) | Date: 2015-02-27 14:08 | |
The mimetypes module is not deprecated. There is a bug in either this tracker or the python.org metatracker (bugs about the python bug tracker) about roundup emitting messages with duplicate headers, and some mailers rejecting the emails because of this. |
|||
msg5232 | Author: [hidden] (jerrykan) | Date: 2015-02-27 14:13 | |
On 28/02/15 01:08, R David Murray wrote: > The mimetypes module is not deprecated. Sorry, that should have been mimetools: https://docs.python.org/2/library/mimetools.html mimetools.Message is a subclass of rfc822.Message, and roundup.mailgw.Message is a subclass of mimetools.Message which I think may be one source of the problems. |
|||
msg5233 | Author: [hidden] (ber) | Date: 2015-02-27 14:22 | |
> I've attached a patch that migrates from the rfc822 module to the email > module for files that import it: Cool John! > A lot of the message strings in the tests seem to have headers defined > twice (ie. 'FROM:' & 'From:', 'TO:' & 'To:'), and the behaviour for > parsing these seems to be different between the rfc822 and email > modules. I'm not sure if the duplicate headers were originally > intentional - to comply with how the rfc822 module works - or not. see issue2550869 it seems this is unwanted behaviour, the rfc822 module was used in a way that the duplicate headers were not discovered in test_mailgw.py. > The second thing is that the 'roundup/mailgw.py' seems to use the > mimetypes module to generate outgoing emails. This is another deprecated > module that should probably be replaced with the email module. Yes, I also saw that mimetypes (und mimetools) should do away. Thanks for the progress! |
|||
msg5239 | Author: [hidden] (ber) | Date: 2015-03-02 16:11 | |
Thanks John for the get_body() function, here is my first commit. This leads to 119 failures, but I believe this is the right next step as the test itself was to lax, missing problems which the test authors probably did not understand. changeset: 4965:a850f8bae536 tag: tip user: Bernhard Reiter <bernhard@intevation.de> date: Mon Mar 02 17:08:40 2015 +0100 files: test/test_mailgw.py description: Moved test_mailgw to email module and make message comparison sharper to flag differences in headers with the same name. |
|||
msg5241 | Author: [hidden] (ber) | Date: 2015-03-02 16:23 | |
Okay, mimetools still to do, what about the rfc2822 module (that is internal to roundup, could possibly go). |
|||
msg5243 | Author: [hidden] (jerrykan) | Date: 2015-03-02 23:58 | |
I have a patch floating around to remove the rfc2822 module. I'll dig it up a bit later and create a new issue for people to review it before committing it. On 03/03/15 03:23, Bernhard Reiter wrote: > > Bernhard Reiter added the comment: > > Okay, mimetools still to do, what about the rfc2822 module (that is > internal to roundup, could possibly go). > > ---------- > title: Still using the rfc822 module, shoud be migrated to the email module -> Still using the rfc822 module, should be migrated to the email module > > ________________________________________________ > Roundup tracker <issues@roundup-tracker.org> > <http://issues.roundup-tracker.org/issue2550870> > ________________________________________________ > |
|||
msg5246 | Author: [hidden] (jerrykan) | Date: 2015-03-03 01:33 | |
Patch for removing rfc2822.py can be found in issue2550874 |
|||
msg5247 | Author: [hidden] (ber) | Date: 2015-03-03 08:46 | |
On Tuesday 03 March 2015 at 00:58:52, John Kristensen wrote: > I have a patch floating around to remove the rfc2822 module. I'll dig it > up a bit later Nice! It would be okay to do it as part of this issue as well. ;) |
|||
msg5261 | Author: [hidden] (jerrykan) | Date: 2015-03-06 07:55 | |
On 03/03/15 03:11, Bernhard Reiter wrote: > Thanks John for the get_body() function, here is my first commit. > This leads to 119 failures, but I believe this is the right next > step as the test itself was to lax, missing problems which the test > authors probably did not understand. > > changeset: 4965:a850f8bae536 > tag: tip > user: Bernhard Reiter <bernhard@intevation.de> > date: Mon Mar 02 17:08:40 2015 +0100 > files: test/test_mailgw.py > description: > Moved test_mailgw to email module and make message comparison sharper to > flag differences in headers with the same name. As you mentioned this commit is breaking the tests which makes it difficult to determine if the test are failing because of these known broken tests or some new change. Are there any objections to reverting this commit until it is more fleshed-out and not breaking the tests? |
|||
msg5262 | Author: [hidden] (ber) | Date: 2015-03-06 08:41 | |
On Friday 06 March 2015 at 08:55:54, John Kristensen wrote: > Are there any objections to reverting this commit until it is more > fleshed-out and not breaking the tests? Yes, the tests in mailgw show bad behaviour, so they should stay in. Just because we swtich off the tests do not make the behaviour good, so there is no additional knowledge that you gain from running (partly) broken tests. (If it helps you, you could temporarily revert the patch in a working copy, but please do not commit..) |
|||
msg5287 | Author: [hidden] (ber) | Date: 2015-03-17 08:23 | |
As you may have seen, the tests pass now and anypy has a small "patch" to the email.Header class so it behaves consistently over python 2.5, 2.6 and 2.7. Just wanted to add that if we move to the email module as part of this issue, we also want to remove some part of anypy.email_. The ones that are not needed anymore (FeedParser and decode_header). |
|||
msg5293 | Author: [hidden] (jerrykan) | Date: 2015-03-19 12:23 | |
I have applied the patch to remove the rest of the rfc822 imports (rev13f8f88ad984): http://hg.code.sf.net/p/roundup/code/rev/13f8f88ad984 |
|||
msg5296 | Author: [hidden] (ber) | Date: 2015-03-19 14:54 | |
John, thanks for adding the patch. What else is missing to make this migration? What about the anypy.email_ functions? At least FeedParser and decode_header could possibly be removed. |
|||
msg5297 | Author: [hidden] (jerrykan) | Date: 2015-03-20 07:54 | |
On 20/03/15 01:54, Bernhard Reiter wrote: > > Bernhard Reiter added the comment: > > John, thanks for adding the patch. > What else is missing to make this migration? > What about the anypy.email_ functions? > At least FeedParser and decode_header could possibly be removed. > As far as I can tell all the rfc822 (and rfc2822) imports have now been removed, so we can probably close this issue. I do have a patch almost reading to commit that will remove most of the roundup.anypy.email_ imports, but there seems to be some sticking points before we could look at removing roundup/anypy/email_.py, which might be best discussed in a new issue. |
|||
msg5298 | Author: [hidden] (ber) | Date: 2015-03-20 08:08 | |
On Friday 20 March 2015 at 08:54:24, John Kristensen wrote: > As far as I can tell all the rfc822 (and rfc2822) imports have now been > removed, so we can probably close this issue. Cool! Please also write a nice CHANGES.txt entry. Can you state what the problem is with removing anypy.email_ decode_header and FeedParser? The ws_continuation "patch" have to stay, so have the imports, but the code of decode_header and FeedParser should be able to go. |
|||
msg5299 | Author: [hidden] (jerrykan) | Date: 2015-03-20 13:36 | |
On 20/03/15 19:08, Bernhard Reiter wrote: > > Bernhard Reiter added the comment: > > On Friday 20 March 2015 at 08:54:24, John Kristensen wrote: >> As far as I can tell all the rfc822 (and rfc2822) imports have now been >> removed, so we can probably close this issue. > > Cool! Please also write a nice CHANGES.txt entry. Sure. I'll commit a note and close this issue. > > Can you state what the problem is with > removing anypy.email_ decode_header and FeedParser? > The ws_continuation "patch" have to stay, so have the imports, > but the code of decode_header and FeedParser should be able to go. Removing the FeedParser stuff if fairly straight forward, and I'll commit a patch doing exactly that shortly. Given that this issue is essentially resolved, I'll open a new issue outlining to problem with roundup/anypy/email_.py |
|||
msg5302 | Author: [hidden] (jerrykan) | Date: 2015-03-20 14:17 | |
CHANGES.txt note committed: http://hg.code.sf.net/p/roundup/code/rev/20786c5152e4 Discussion of roundup.anypy.email_ can continue in issue2550879 |
History | |||
---|---|---|---|
Date | User | Action | Args |
2015-03-20 14:17:07 | jerrykan | set | status: new -> closed messages: + msg5302 |
2015-03-20 13:36:49 | jerrykan | set | messages: + msg5299 |
2015-03-20 08:08:45 | ber | set | messages: + msg5298 |
2015-03-20 07:54:24 | jerrykan | set | messages: + msg5297 |
2015-03-19 14:54:19 | ber | set | messages: + msg5296 |
2015-03-19 12:23:00 | jerrykan | set | messages: + msg5293 |
2015-03-17 08:23:16 | ber | set | messages: + msg5287 |
2015-03-06 08:41:18 | ber | set | messages: + msg5262 |
2015-03-06 07:55:54 | jerrykan | set | messages:
+ msg5261 title: Still using the rfc822 module, should be migrated to the email module -> Still using the rfc822 module, shoud be migrated to the email module |
2015-03-03 08:46:31 | ber | set | messages: + msg5247 |
2015-03-03 01:33:15 | jerrykan | set | dependencies:
+ Remove the rfc2822 module messages: + msg5246 |
2015-03-02 23:58:52 | jerrykan | set | messages: + msg5243 |
2015-03-02 16:23:31 | ber | set | messages:
+ msg5241 title: Still using the rfc822 module, shoud be migrated to the email module -> Still using the rfc822 module, should be migrated to the email module |
2015-03-02 16:11:10 | ber | set | messages: + msg5239 |
2015-02-27 14:22:58 | ber | set | messages: + msg5233 |
2015-02-27 14:13:38 | jerrykan | set | messages: + msg5232 |
2015-02-27 14:08:24 | r.david.murray | set | messages: + msg5231 |
2015-02-27 12:58:19 | jerrykan | set | files:
+ issue2550870-1.patch messages: + msg5230 |
2015-02-26 01:46:04 | jerrykan | set | nosy: + jerrykan |
2015-02-25 22:54:00 | ber | set | files:
+ test_mailgw-20150225.patch keywords: + patch messages: + msg5225 |
2015-02-25 21:20:06 | r.david.murray | set | nosy:
+ r.david.murray messages: + msg5224 |
2015-02-25 21:07:45 | ber | create |