This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author ajvant
Recipients ajvant, brian.curtin, chasonr, christian.heimes, giampaolo.rodola, janssen, jelie, pitrou
Date 2010-11-02.23:18:37
SpamBayes Score 2.7755576e-16
Marked as misclassified No
Message-id <4CD09C4E.3058.AC5AA3B@ajvant.gmail.com>
In-reply-to <1288717539.98.0.790128608117.issue1926@psf.upfronthosting.co.za>
Content
On 2 Nov 2010 at 17:05, Antoine Pitrou wrote:

> * 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)

I'm not sure I understand which part you're talking about here....the 
tests don't take a context for anything that I can see, and the 
mechanism I use in nntplib (a context as a named argument) is the 
same one used in smtplib and poplib for <prot>_SSL. 

(if you can suggest a method-of-use that breaks it, that may enlighten
me better) 

> * 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')

Meh. My editor wasn't set up properly when I started. I thought I'd 
eliminated all the tabs. Guess not. There's probably a way to grep 
for lines that have tabs in them, I'll fix it.

> * not moving methods around (such as getwelcome) would make it easier
> to review the real changes

I'll see what I can do. Some amount of that was necessary to fit 
STARTTLS in. (because init assumed you wanted to log in right away 
and never again) 

> * in test_starttls, I would clearly report that the server doesn't
> support STARTTLS, e.g.:

Can do. I didn't know about self.skip; I don't really understand the 
testing framework, I was just working by comparison to the other 
tests.

> * 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).

...I actually meant to do exactly that, but forgot to put the check 
in. Thank you. 

--

Andrew.
History
Date User Action Args
2010-11-02 23:18:41ajvantsetrecipients: + ajvant, janssen, pitrou, giampaolo.rodola, christian.heimes, chasonr, brian.curtin, jelie
2010-11-02 23:18:38ajvantlinkissue1926 messages
2010-11-02 23:18:37ajvantcreate