Use aiosmtpd for the smtp.Server class #30
Conversation
This change should make it a bit more explicit what form the message details are supposed to take.
In this commit I've completely rewritten the smtp.Server class using aiosmtpd instead of Python's standard smtpd module, which is deprecated and scheduled for removal in Python 3.12. Migrating the main functionality to aiosmtpd is intrinsically not hard; it just required creating the Handler class, which is about ten lines and can be used as-is with aiosmtpd.controller.Controller. The rest of the changes here are to reproduce the interface of the existing Server class well enough to pass the tests. We've been testing some features of the Server class which don't exactly have equivalents in aiosmtpd, so I had to use some wrappers and hacks to offer the same interface: for example, I reimplemented the Server.stop() method to make it idempotent and accept a timeout argument, and I had to use some private attributes of Controller to make Server.is_alive() and Server.accepting behave the way they used to. I also added some additional hacks to support older versions of aiosmtpd that work with Python 3.5. In retrospect, that probably wasn't worth it, but I started trying to do it without realizing how much work it'd be, and once the work was done, I figured I might as well keep it. We should definitely drop support for Python 3.5 soon, though, and that will simplify the implementation.
The address of an IPv6 socket as returned by getsockname() is a four-element tuple, rather than a two-element tuple as expected by the existing pytest_localserver.smtp module code. In this commit I'm extracting the first two elements of the tuple so that a Server's addr attribute will be a 2-tuple of (host, port) regardless of whether the underlying socket is an IPv4 or IPv6 socket.
In this commit I'm adding a test which ensures that the Server address is a 2-tuple of (host, port), with the host being a string and the port being a positive integer. This format is expected by existing code.
|
@nicoddemus if you don't mind, I'd appreciate your thoughts about introducing a dependency on |
|
Hi @diazona, Thanks a lot for the work on this! I didn't review it in depth, but the code looks good and well documented.
Unfortunately I don't, AFAIK @coordt and @redtoad are still the maintainers, might have been lack of time to go over the PR yet. If you are willing to maintain a plugin, and given that it is possible to have a different/incompatible API that fits the async model better, to have this as a separate plugin altogether: if users need to port the code, might make sense to migrate the plugin as well. |
|
@nicoddemus Thanks for the review! Sorry if I'm misunderstanding what you meant, but it sounds like you might have missed that I'm also a maintainer of this package ( The main thing I wanted a second opinion on is whether it makes sense to introduce this dependency on |
Ahh indeed I missed that, sorry about that!
Ahh sorry I don't have any insight on that. Reaching them sounds good. 👍 Sorry about the confusion on my part! |
I made these changes to switch from using Python's standard smtpd module, which is deprecated, to the external aiosmtpd package. The changes are somewhat substantial, but most of them are just meant to keep the Server class interface backward compatible and to support old versions of Python and smtpd. Once we drop support for older versions, it will simplify the implementation quite a bit.
One thing to note: it's not entirely clear how actively aiosmtpd is maintained, since the last commit on Github was over a year ago and there's a featured issue asking for help with the package. So I can imagine a future where the suggested replacement for smtpd changes to something other than aiosmtpd. It's possible we might want to defer this merge request until the status of aiosmtpd as a replacement for smtpd becomes more clear.
Closes #12