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

John Arbash Meinel john at arbash-meinel.com
Wed Dec 17 17:31:14 GMT 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
>>>>>> "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.

Right, but it is using "self.get()" which .... is the underlying object
so again, we are using a StringIO and not closing the raw socket.

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

I'm pretty sure you just said that HTTP doesn't implement its own
get_bytes(). Which means that self.get() is using the wrapped StringIO,
and then just unwrapping it again...

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAklJN2IACgkQJdeBCYSNAANk3ACgsfvVEBvCL/wrkl1lQQYoLi/E
XDMAn1ViW7gvRf5jX6iGbNgf87ypVxQk
=DARA
-----END PGP SIGNATURE-----



More information about the bazaar mailing list