[Python-Dev] HTTP/1.1 capable httplib module

[Greg Stein]

I've integrated all of these changes into the httplib.py posted on my
pages at:
    http://www.lyra.org/greg/python/

The actual changes are visible thru ViewCVS at:
    http://www.lyra.org/cgi-bin/viewcvs.cgi/gjspy/httplib.py/


The test code is still in there, until a test_httplib can be written.

Still missing: doc for the new-style semantics.

Cheers,
-g

On Fri, 19 May 2000, Greg Stein wrote:
> 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/
> 
> 

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