[MERGE] chunked sftp readv

[John Arbash Meinel]

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

Martin Pool wrote:
> Martin Pool has voted tweak.
> Status is now: Conditionally approved
> Comment:
> +class OverlappingReadv(BzrError):
> +    """Raised when a readv() requests overlapping chunks of data.
> +
> +    Not all transports supports this, so the api should generally
> forbid it.
> +    (It isn't a feature we need anyway.
> +    """
> +
> +    _fmt = 'Requested readv ranges overlap'
> +
> +
> 
> Should this subclass a TransportError or something?
> 
> Would it be useful to show the particular values in the error?  In
> general I think it's good to capture the most information possible at
> the time the error occurs but here the raw numbers may not help much
> without more investigation.
> 
> You're missing a ')'.  Use this one --> )  :-)

We can. It never happens, and it was mostly making clear that the api
forbids it. In a way, we could just get away with an "AssertionError".
The readv() api wasn't specific about whether overlapping was allowed,
but we've never needed it, and it allows us to consume data as it comes
in, rather than worrying that it might be needed for another range.

> 
> +
> +    def assertGetRequests(self, expected_requests, offsets):
> 
> I think generally our style is to call things that actually do the test
> checkGetRequests (or check_get_requests), because it's more than just an
> assertion.

Really? I've been using assertFoo for this for a while, I can change if
that is truly the case.

> 
> +    def _coalesce_offsets(offsets, limit=0, fudge_factor=0, max_size=0,
> +                          allow_overlap=False):
> 
> I think this would previously allow you to pass in overlapping ranges
> and as one effect it would clear them up.  That may never have been used
> but does it particularly need to be disallowed at this level?  (I can
> see how it makes sense not to require transports to handle it.)  If you
> do want to keep it, could you say so in the docstring.
> 
> The code change itself seems correct afaics.
> 

It would collapse overlapping ranges, but the higher levels probably
didn't support it. Specifically, the really old ones that just buffered
everything would be fine (I think HTTP falls into this), but newer ones
that deallocate the temporary buffers will fail.

I mostly wanted to assert this in the _sftp_readv and other such helpers
(RemoteTransport._readv could use this as well). And it seemed nice to
put it in a common place between all helpers.

John
=:->


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

iEYEARECAAYFAki/+tsACgkQJdeBCYSNAAO4VACcCWIcTQA6YndZffpWpjj6grSq
XisAnAvAX02pNRBOqzTUq0b/JFi7GCoY
=XpFB
-----END PGP SIGNATURE-----