[Python-Dev] HTTP/1.1 capable httplib module

[Greg Stein]

On Fri, 19 May 2000, Guido van Rossum wrote:
> > I applied the recent changes to the CVS httplib to Greg's httplib
> > (call it httplib11) this afternoon.  The result is included below.  I
> > think this is quite close to checking in,

I'll fold the changes into my copy here (at least), until we're ready to
check into Python itself.

THANK YOU for doing this work. It is the "heavy lifting" part that I just
haven't had a chance to get to myself.

I have a small, local change dealing with the 'Host' header (it shouldn't
be sent automatically for HTTP/1.0; some httplib users already send it
and having *two* in the output headers will make some servers puke).

> > but it could use a slightly
> > better test suite.
> 
> Thanks -- but note that I don't have the time to review the code.

I'm reviewing it, too. Gotta work around the fact that Jeremy re-indented
the code, though... :-)

> > There are a few outstanding questions.
> > 
> > httplib11 does not implement the debuglevel feature.  I don't think
> > it's important, but it is currently documented and may be used.
> > Guido, should we implement it?
> 
> I think the solution is to provide the API ignore the call or
> argument.

Can do: ignore the debuglevel feature.

> > httplib w/SSL uses a constructor with this prototype:
> >     def __init__(self, host='', port=None, **x509):
> > It looks like the x509 dictionary should contain two variables --
> > key_file and cert_file.  Since we know what the entries are, why not
> > make them explicit?
> >     def __init__(self, host='', port=None, cert_file=None, key_file=None):
> > (Or reverse the two arguments if that is clearer.)
> 
> The reason for the **x509 syntax (I think -- I didn't introduce it) is
> that it *forces* the user to use keyword args, which is a good thing
> for such an advanced feature.  However there should be code that
> checks that no other keyword args are present.

Can do: raise an error if other keyword args are present.

> > The FakeSocket class in CVS has a comment after the makefile def line
> > that says "hopefully, never have to write."  It won't do at all the
> > right thing when called with a write mode, so it ought to raise an
> > exception.  Any reason it doesn't?
> 
> Probably laziness of the code.  Thanks for this code review (I guess I
> was in a hurry when I checked that code in :-).

+1 on raising an exception.

> 
> > I'd like to add a couple of test cases that use HTTP/1.1 to get some
> > pages from python.org, including one that uses the chunked encoding.
> > Just haven't gotten around to it.  Question on that front: Does it
> > make sense to incorporate the test function in the module with the std
> > regression test suite?  In general, I would think so.  In this
> > particular case, the test could fail because of host networking
> > problems.  I think that's okay as long as the error message is clear
> > enough. 
> 
> Yes, I agree.  Maybe it should raise ImportError when the network is
> unreachable -- this is the one exception that the regrtest module
> considers non-fatal.

+1 on shifting to the test modules.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/