[MERGE][bug #172701] Catch pycurl short reads and allows readv to issue several GET requests

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Nov 30 17:35:28 GMT 2007


>>>>> "john" == John Arbash Meinel <john at arbash-meinel.com> writes:

    john> Vincent Ladeuil wrote:
    >> This patch:
    >> 
    >> - fixes bug #165061 in a more complete way, allowing readv to use
    >> any number of ranges without falling back to single range
    >> requests,
    >> 
    >> - fixes bug #172701 by catching an additional short read
    >> encountered by pycurl (in circumstances yet to be understood,
    >> but the readv will now proceed instead of aborting),
    >> 
    >> - add a -Dhttp flag which would have been helpful for this bug
    >> and allows easier debugging of http (the sources had to be
    >> modified previously), both implementations (pycurl and urllib)
    >> issue the same output, but while urllib use mutter, pycurl
    >> displays its output on stderr (libcurl provides an option to
    >> redirect that but it's not available through pycurl).
    >> 
    >> I will continue to improve the http behavior by rewriting the
    >> response parsing which should address bug #173010 and (may be
    >> also bug #173007).
    >> 
    >> Vincent
    >> 
    >> 

    john> Vincent, you disappoint me. You have the wrong typo:
    john> + * htpp - trace http connections, requests and responses

    john> Isn't this supposed to be "hhtp"?

Don't know what you're talking about:

egrep "h+t+p+" /etc/services
# Updated from http://www.iana.org/assignments/port-numbers and other
# sources like http://www.freebsd.org/cgi/cvsweb.cgi/src/etc/services .
www		80/tcp		http hhtp htpp  # WorldWideWeb HTTP
https		443/tcp		hhtps htpps     # http protocol over TLS/SSL
https		443/udp         hhtps htpps hhtpps hhttppss hhttttppss # DWIM

But I always try to make my reviewers happy (not only by making
them laugh) so I'll fix it.

    john> There is some drive-by code cleanup being done. But I
    john> personally am happy to see it.

Cruft accumulates in my cleanup/* branches otherwise :-/

    john> -        raise TestSkipped('pycurl does not check HTTP_PROXY '
    john> -            'for security reasons')
    john> +        raise tests.TestSkipped('pycurl does not check HTTP_PROXY '
    john> +                                'for security reasons')
    john> ^- At least the message here makes me think it should be a TestNotApplicable
    john> rather than TestSkipped.

Exact. Thanks.

    john> Your NEWS entries seem to miss the fact that the readv
    john> code is rewritten to better support servers that have
    john> problems with lots of ranges.

Done.

    john> Have you tried changing _max_readv_combine and seeing
    john> if it gives some help for bugs like:
    john> https://bugs.launchpad.net/bugs/173010 (no progress
    john> notification for a long time with http)

No, I'm rewriting the http response parsing which should solve
the problem in a better way. If *that* doesn't work I'll look
into it, otherwise I prefer to minimize the requests issued.

Also note that another solution, in the readv case, will be to
try to issue all the GET requests and process the responses as
they come back (httplib seems to be able to handle such a scheme,
but that may be more work than just spawning a thread as paramiko).

      Vincent



More information about the bazaar mailing list