Python 3 compatibility (#7) and RESPONSE_RE fix (#9) #11

duk3luk3 · 2017-03-29T07:38:09Z

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, the do_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.

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

duk3luk3 · 2017-03-29T07:48:29Z

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.

akissa · 2017-03-29T10:24:15Z

I will not be able to merge a PR that breaks py2.6

duk3luk3 · 2017-03-29T10:59:28Z

Errr... you told me you want to support python 2.7 minimum: #9 (comment) :/

akissa · 2017-03-29T11:02:30Z

Hmmm, not sure how i got that totally wrong. I use 2.6 in production on RHEL6. This would break that.

akissa · 2017-03-29T11:08:34Z

We could make this the 2.x.x branch

duk3luk3 · 2017-03-29T11:09:53Z

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?

akissa · 2017-03-29T11:11:17Z

No you cannot drop python 2, RHEL 7 still uses that and will for a long time

duk3luk3 · 2017-03-29T13:51:19Z

Okay, I will just get this to work then. But can't you get python 3 from EPEL or SCL?

codecov-io · 2017-03-31T08:35:53Z

Codecov Report

Merging #11 into master will decrease coverage by 0.44%.
The diff coverage is 89.18%.

@@            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

Impacted Files	Coverage Δ
spamc/regex.py	`100% <ø> (ø)`	⬆️
spamc/conn.py	`94.73% <100%> (+0.37%)`	⬆️
spamc/client.py	`98.2% <87.09%> (-0.82%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da50732...64fff1e. Read the comment docs.

duk3luk3 · 2017-04-11T06:47:49Z

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:

What's with the spurious Broken Pipe / Connection Reset errors
The way the compression handling and file sending is intermingled is pretty unclean, resulting now in having opportunistic encode() statements littered through the code. I think I'll clean that up.
Clean up commit history
Remove debug print statements

But I'd like to see if you have any feedback at this point!

tripleee · 2019-08-22T04:45:31Z

There's an import types left at the top of spamc/client.py which is no longer necessary after you changed all the uses to just use str.

tripleee · 2019-08-22T05:09:42Z

In conn.py in the sendfile method, you want to add and isinstance(binarydata, str) before calling encode. The interface should (nay, must) support passing in email messages as bytes.

duk3luk3 added 4 commits March 29, 2017 09:13

Update spamc for python3 compatibility

98c1e6d

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

Add future dependency for easier import switches

e6c93e5

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

Update tests for py3 compatibility

fb75790

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

Add python3 testing

c0e72ae

TEST: Encoding

3a68da7

duk3luk3 force-pushed the py3 branch from 7cddc0b to 3a68da7 Compare March 29, 2017 10:57

duk3luk3 force-pushed the py3 branch 9 times, most recently from a429879 to 9e90aec Compare April 7, 2017 08:53

Use spamd mock test as well

381c987

duk3luk3 force-pushed the py3 branch from 9e90aec to 381c987 Compare April 7, 2017 08:58

duk3luk3 added 2 commits April 7, 2017 11:17

Better encoding handling

a3a065c

Debug print

51b32c6

duk3luk3 force-pushed the py3 branch from 83db340 to 51b32c6 Compare April 7, 2017 10:13

duk3luk3 force-pushed the py3 branch 3 times, most recently from d569c7f to 45722e6 Compare April 11, 2017 07:30

Add encode for py3 to compression paths

197011e

duk3luk3 force-pushed the py3 branch from 45722e6 to 197011e Compare April 11, 2017 07:37

Add python 3.6 test

64fff1e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3 compatibility (#7) and RESPONSE_RE fix (#9) #11

Python 3 compatibility (#7) and RESPONSE_RE fix (#9) #11

duk3luk3 commented Mar 29, 2017

duk3luk3 commented Mar 29, 2017 •

edited

akissa commented Mar 29, 2017

duk3luk3 commented Mar 29, 2017

akissa commented Mar 29, 2017

akissa commented Mar 29, 2017

duk3luk3 commented Mar 29, 2017

akissa commented Mar 29, 2017

duk3luk3 commented Mar 29, 2017

codecov-io commented Mar 31, 2017 •

edited

duk3luk3 commented Apr 11, 2017 •

edited

tripleee commented Aug 22, 2019

tripleee commented Aug 22, 2019

Python 3 compatibility (#7) and RESPONSE_RE fix (#9) #11

Are you sure you want to change the base?

Python 3 compatibility (#7) and RESPONSE_RE fix (#9) #11

Conversation

duk3luk3 commented Mar 29, 2017

Python 3 Compat (#7)

RESPONSE_RE (#9)

duk3luk3 commented Mar 29, 2017 • edited

akissa commented Mar 29, 2017

duk3luk3 commented Mar 29, 2017

akissa commented Mar 29, 2017

akissa commented Mar 29, 2017

duk3luk3 commented Mar 29, 2017

akissa commented Mar 29, 2017

duk3luk3 commented Mar 29, 2017

codecov-io commented Mar 31, 2017 • edited

Codecov Report

duk3luk3 commented Apr 11, 2017 • edited

tripleee commented Aug 22, 2019

tripleee commented Aug 22, 2019

duk3luk3 commented Mar 29, 2017 •

edited

codecov-io commented Mar 31, 2017 •

edited

duk3luk3 commented Apr 11, 2017 •

edited