[merge] MVC progress bars, and indication of network traffic

[Vincent Ladeuil]

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

    jam> ...

    martin> :param relpath: The relative path to the file
    martin> """
    martin> -        return self.get(relpath).read()
    martin> +        f = self.get(relpath)
    martin> +        try:
    martin> +            return f.read()
    martin> +        finally:
    martin> +            f.close()
    >> 
    >> 
    >> EErk ! I'm pretty sure you just closed the http socket here (the
    >> http transport doesn't redefine get_bytes) ! If no test is
    >> failing there, we need one. On the other hand, I'm not sure you
    >> need that part in your patch.
    >> 

    jam> Unless something has changed, HTTPTransport.get() does:

    jam> code, response_file = self._get(relpath, None)
    jam> # FIXME: some callers want an iterable... One step forward, three steps
    jam> # backwards :-/ And not only an iterable, but an iterable that can be
    jam> # seeked backwards, so we will never be able to do that.  One such
    jam> # known client is bzrlib.bundle.serializer.v4.get_bundle_reader. At the
    jam> # time of this writing it's even the only known client -- vila20071203
    jam>   return StringIO(response_file.read())

That's get() not get_bytes(). The patch is about get_bytes().

As a side note, this is why I review the patches in a cloned
branch, the patch didn't say which method the modification was
in.

And my remark doesn't apply to pycurl either, which use a
StringIO too.

    jam> Mostly I'm pointing that we don't return the raw socket,
    jam> we wrap it in a StringIO.

Only because of the FIXME (which I still hope to fix one day :)

get_bytes and readv gives access to the socket, wrapping the
read()/seek() calls in an http.response.RangeFile object but
still accessing the socket.

...

But RangeFile doesn't define close(), so may be we are safe after
all :)

    jam> Originally we needed that because "gzip.py" does a
    jam> seek() on its input, but we changed that in
    jam> "tuned_gzip.py"

We still need it because get_bundle_reader read its format twice
IIRC.

By the way, I know python-2.6 made some changes in gzip.py, did
you review it lately ?

    jam> So it is possible you might change it at some point in
    jam> the future, though I think not dealing in a raw socket
    jam> may work out best for everyone.

AFAIUI the good performances of http are due to streaming, adding
a buffering level again can't help.

    jam> (For example, making sure all the bytes are consumed
    jam> before the next request is sent.)

This is taken care of when we need to issue another request.

     Vincent