-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Python 3 compatibility (#7) and RESPONSE_RE fix (#9) #11
base: master
Are you sure you want to change the base?
Conversation
The spamd response "SPAMD/1.0 69 Service Unavailable: TELL commands are not enabled, set the --allow-tell switch." was not captured by the very restrictive `RESPONSE_RE`. This commit changes the regex to allow any text in the "message" part. Fixes akissa#9
Some library interfaces have changed in Python 3, for example `IOBase` and the `str`/`bytes` unicode split. Thanks to the great modularization of spamc this requires only a handful of changes. For akissa#7
Due to the large reorganization in python 3, we need to do a lot of conditional importing. `future` does some of that for us, e.g. with socketserver vs. SocketServer. For akissa#7
The tests needed extensive rewriting because they used very old python components and do a lot of data parsing and handling which has to be changed. Fixes akissa#7
That build failure is very interesting, since I can't reproduce it... EDIT: Ah, I suppose it's because it actually tests a real spamassassin. Devious. |
I will not be able to merge a PR that breaks py2.6 |
Errr... you told me you want to support python 2.7 minimum: #9 (comment) :/ |
Hmmm, not sure how i got that totally wrong. I use 2.6 in production on RHEL6. This would break that. |
We could make this the 2.x.x branch |
Sure, but then I could drop python 2 compatibility completely, and could have saved a lot of work for myself ;-) Also, why did you disable the python 2.6 travis tests? |
No you cannot drop python 2, RHEL 7 still uses that and will for a long time |
Okay, I will just get this to work then. But can't you get python 3 from EPEL or SCL? |
Codecov Report
@@ Coverage Diff @@
## master #11 +/- ##
==========================================
- Coverage 98.17% 97.72% -0.45%
==========================================
Files 10 10
Lines 328 352 +24
==========================================
+ Hits 322 344 +22
- Misses 6 8 +2
Continue to review full report at Codecov.
|
a429879
to
9e90aec
Compare
So I'm still working on this, with more tests configured, running here: https://travis-ci.org/duk3luk3/spamc/pull_requests And I get a lot of what appear to be spurious Connection resets from the real spamassassin tests, e.g. here: https://travis-ci.org/duk3luk3/spamc/jobs/219625099 Do you have any idea why those connection resets happen? EDIT: Now I have a fully passing build: https://travis-ci.org/duk3luk3/spamc/builds/220854875 There is still some TODO left:
But I'd like to see if you have any feedback at this point! |
d569c7f
to
45722e6
Compare
There's an |
In |
Python 3 Compat (#7)
The main work is in getting the tests to work. Since
mimetools
is gone, I had to implement my own "headers only" pre-parser (parse_headers
in_s.py
). Also, thedo_HEADERS
function got a rewrite.Testing was only done using the unit tests against Python 3.6.0 and Python 2.7.13 (arch linux package versions). I still have to do testing against a real spamd.
Compatibility with Python < 2.7 is probably no longer present (e.g. because
b
prefix for bytestrings is used now).RESPONSE_RE (#9)
Just like discussed in #9.
Fixes #7.
Fixes #9.