Antoine Pitrou <[email protected]> added the comment:
Andrew, thank you for posting a patch.
I have a couple of comments:
* the command-line option for the SSLContext won't work, since a context is a
custom object, not a string; I would rework this part anyway, since I don't
think separate options for the "SSL host" are useful (I'd rather add a
SSL-enabling flag; also, STARTTLS can be queried from the capabilities)
* on a stylistic note, there are a couple of places where you use tabs for
indentation; also, comments should have a space after the '#' ('# xxx' and not
'#xxx')
* not moving methods around (such as getwelcome) would make it easier to review
the real changes
* in test_starttls, I would clearly report that the server doesn't support
STARTTLS, e.g.:
except nntplib.NNTPPermanentError:
self.skip("STARTTLS not supported by server")
* starttls() should probably test the `tls_on` attribute first and raise a
ValueError if True (as you point out, a client mustn't attempt to start a new
TLS session if one is already active).
All in all, it looks good though.
----------
_______________________________________
Python tracker <[email protected]>
<http://bugs.python.org/issue1926>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
