[issue1926] NNTPS support in nntplib

[Antoine Pitrou]

Antoine Pitrou <pitrou at free.fr> 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 <report at bugs.python.org>
<http://bugs.python.org/issue1926>
_______________________________________