[issue11959] smtpd cannot be used without affecting global state

Roundup Robot added the comment: New changeset ed498f477549 by Vinay Sajip in branch 'default': Closes #11959: SMTPServer and SMTPChannel now take an optional map, use of which avoids affecting global state. http://hg.python.org/cpython/rev/ed498f477549 -- nosy: +python-dev resolution:

[issue11959] smtpd cannot be used without affecting global state

R. David Murray added the comment: Looks good to me too. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: htt

[issue11959] smtpd cannot be used without affecting global state

Giampaolo Rodola' added the comment: LGTM now. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.p

[issue11959] smtpd cannot be used without affecting global state

Changes by Vinay Sajip : Added file: http://bugs.python.org/file30480/34974b9bc4d0.diff ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue11959] smtpd cannot be used without affecting global state

Giampaolo Rodola' added the comment: Changes to Doc/library/asyncore.rst should be reverted. Also I would do: - asynchat.async_chat.__init__(self, conn, map) + asynchat.async_chat.__init__(self, conn, map=map) -- ___ Python tracker

[issue11959] smtpd cannot be used without affecting global state

Barry A. Warsaw added the comment: The changes to smtpd.py seem reasonable to me. I see no reason not to make this change, so +1. -- ___ Python tracker ___

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: Patch now updated to revert asyncore changes. The changes are now: smtpd.py - changed SMTPChannel and SMTPServer to accept map argument test_logging.py - removed subclassed SMTPChannel, not needed since the base SMTPChannel class now accepts a map, and simplified

[issue11959] smtpd cannot be used without affecting global state

Changes by Vinay Sajip : Added file: http://bugs.python.org/file30441/d7c50c15468d.diff ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: I wasn't suggesting changing the signature of create_socket, I just thought that it needed to be reimplemented because it didn't pass a map to set_socket. I've had a look at it again, and a reimplementation of create_socket doesn't seem to be needed, after all, b

[issue11959] smtpd cannot be used without affecting global state

Giampaolo Rodola' added the comment: Looking back at this I think that allowing a map argument to be passed to SMTPChannel in order to allow running handlers in separate threads can be reasonable after all. I don't understand why create_socket() signature needs to be changed though. --

[issue11959] smtpd cannot be used without affecting global state

Changes by Barry A. Warsaw : -- nosy: +barry ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python

[issue11959] smtpd cannot be used without affecting global state

Changes by Vinay Sajip : -- versions: +Python 3.4 -Python 3.3 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: h

[issue11959] smtpd cannot be used without affecting global state

Changes by Vinay Sajip : -- assignee: -> vinay.sajip stage: -> patch review ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue11959] smtpd cannot be used without affecting global state

Changes by Vinay Sajip : -- keywords: +patch Added file: http://bugs.python.org/file25815/fa55dc894947.diff ___ Python tracker ___ ___

[issue11959] smtpd cannot be used without affecting global state

Changes by Vinay Sajip : -- hgrepos: +129 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.or

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: > > Well, I would argue that asyncore's design is thoroughly broken, and > passing a socket map is a poor kludge to avoid global state; in a > sophisticated event loop, the socket map wouldn't be the only piece of > state to pass around. > I don't disagree with y

[issue11959] smtpd cannot be used without affecting global state

Antoine Pitrou added the comment: > Given that asyncore's design allows for a socket map to be passed in > (at least in part - RDM's comment), ISTM that it should support this > consistently, and also that smtpd should support this mode of use. Well, I would argue that asyncore's design is thor

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: > Well, other tests manage it even without using a private socket map. > Leaving dangling sockets in the socket map could mean your code > forgets to close them, for example. This issue is not about getting test_logging to work in a particular way; test_logging i

[issue11959] smtpd cannot be used without affecting global state

Antoine Pitrou added the comment: > I tried to find a way of avoiding changing global state in my test - > but because of the problem I mention, I couldn't see a way of using > smtpd without affecting global state. Well, other tests manage it even without using a private socket map. Leaving dan

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: > If you get a warning, it means your tests lack proper cleanup, so you > should fix that instead of trying to make the warning disappear by > circumventing regrtest's detection mechanism. What makes you say I was trying to circumvent regrtest's detection mechanis

[issue11959] smtpd cannot be used without affecting global state

Antoine Pitrou added the comment: > I initially came across this because I got some warnings from > regrtest.py about changed state, when I was trying to implement a > TestSMTPServer class (derived from smtpd.SMTPServer) to test the > SMTPHandler in logging. If you get a warning, it means your

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: > So if we add a map argument to that and only pass it to socket if it > is non-None, wouldn't that maintain backward compatibility with > current asyncore behavior? Sorry I was being a bit dense ... it's been a while since I looked at this. I think you are right

[issue11959] smtpd cannot be used without affecting global state

R. David Murray added the comment: But it is create_socket you want to change. So if we add a map argument to that and only pass it to socket if it is non-None, wouldn't that maintain backward compatibility with current asyncore behavior? Neither asyncore nor asynchat calls create_socket, a

[issue11959] smtpd cannot be used without affecting global state

Giampaolo Rodola' added the comment: Changing it in asyncore is fine. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Uns

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: > If asyncore and asynchat are (mostly?) supporting an alternate > socket map, why is it necessary to copy create_socket? > Shouldn't we be fixing create_socket in asyncore instead? Well, I don't see how this can be done along with keeping existing behaviour, sin

[issue11959] smtpd cannot be used without affecting global state

R. David Murray added the comment: I'm finally getting back around to this. If asyncore and asynchat are (mostly?) supporting an alternate socket map, why is it necessary to copy create_socket? Shouldn't we be fixing create_socket in asyncore instead? -- ___

[issue11959] smtpd cannot be used without affecting global state

Changes by Michele Orrù : -- nosy: +maker ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.or

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: > I notice that your logging test code doesn't override create_socket. Is that > no longer needed? I removed that code from test_logging, but the code in the BitBucket repo shows how I'd like smtpd to be changed. I would of course prefer to avoid copying stuff

[issue11959] smtpd cannot be used without affecting global state

R. David Murray added the comment: The test failure in #14314 isn't a bug in my code, it is due to the fact that you copied the __init__ method of SMTPChannel in your logging tests, and the __init__ is changed by my patch. Clearly it would be good to resolve this issue one way or another. I

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: The overridden create_socket() method will have the same behaviour for the case when a socket map is *not* passed in to smtpd.__init__(). Users using the existing signature for the constructor will cause the sockmap instance attribute to be set to None, and this

[issue11959] smtpd cannot be used without affecting global state

Terry J. Reedy added the comment: OK, passing self.sockmap=None to setsocket matches its current behavior, so your new code should not change any current smtpd users, while modifying asyncore.dispatcher.create_socket might possibly affect someone. -- _

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: It's create_socket which is being redefined, because the base class implementation https://bitbucket.org/mirror/cpython/src/5661480f7763/Lib/asyncore.py#cl-292 calls set_socket without passing a map. -- ___ Python tr

[issue11959] smtpd cannot be used without affecting global state

Terry J. Reedy added the comment: I looked as the small patch to smptd.py. This strikes me a a reasonable use of dependency injection to make smptd more usable in testing, especially given that asyncx are have it. The only thing I do not understand fully is the redefinition of set_socket, pe

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: I've made a patch. See https://bitbucket.org/vinay.sajip/cpython-smtpd/compare/default..mirror/cpython -- ___ Python tracker ___

[issue11959] smtpd cannot be used without affecting global state

Vinay Sajip added the comment: I don't want to use two different maps - I just want to use a single map which is not the global "socket_map" in asyncore. asyncore.dispatcher and asynchat.async_chat allow for a map to be passed in so that the default global is not used, but smtpd does not allo

[issue11959] smtpd cannot be used without affecting global state

Giampaolo Rodola' added the comment: The fact that you need to keep two separate maps makes me think that the approach you have in mind might be wrong, as in - that's something you usually don't want to do -. The fact that asyncore uses a global socket map is surely unfortunate, but it's some

[issue11959] smtpd cannot be used without affecting global state

Changes by Giampaolo Rodola' : -- Removed message: http://bugs.python.org/msg134825 ___ Python tracker ___ ___ Python-bugs-list mailin

[issue11959] smtpd cannot be used without affecting global state

Giampaolo Rodola' added the comment: The fact that you need to keep two separate maps makes me think that the approach you have in mind might be wrong, as in - that's something you usually don't want to do -. The fact that asyncore uses a default socket map is surely unfortunate, but it's som

[issue11959] smtpd cannot be used without affecting global state

Changes by R. David Murray : -- nosy: +r.david.murray ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://ma

[issue11959] smtpd cannot be used without affecting global state

New submission from Vinay Sajip : It seems not possible to use smtpd in certain contexts, because it forces use of global state. For example, I'm looking at implementing a test SMTP server to test logging's SMTPHandler. Neither SMTPServer nor SMTPChannel allow a map to be passed in, forcing us