Author pitrou
Recipients ajvant, brian.curtin, chasonr, christian.heimes, giampaolo.rodola, janssen, jelie, pitrou
Date 2010-11-02.17:05:35
SpamBayes Score 2.58404e-13
Marked as misclassified No
Message-id <1288717539.98.0.790128608117.issue1926@psf.upfronthosting.co.za>
In-reply-to
Content
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.
History
Date User Action Args
2010-11-02 17:05:40pitrousetrecipients: + pitrou, janssen, giampaolo.rodola, christian.heimes, chasonr, brian.curtin, ajvant, jelie
2010-11-02 17:05:39pitrousetmessageid: <1288717539.98.0.790128608117.issue1926@psf.upfronthosting.co.za>
2010-11-02 17:05:37pitroulinkissue1926 messages
2010-11-02 17:05:35pitroucreate