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

Changed the code to use ValueError, as it is either programmer fault, or data
errors. And I got rid of the parameter, because nothing was going to be
supplying it anyway.

> +
> +    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.

So I consider this an assertFoo because it basically just wraps

self.assertEqual(expected, Reader(offsets, 'foo')._get_requests())

It is slightly more than

self.assertEqual(expected, foo(params))

But I guess it *is* more, so I'll switch.

> 
> +    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.

I went ahead and got rid of the parameter. I had left it for (optional)
backwards compatibility, but it honestly isn't needed.

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

iD8DBQFIwaNgJdeBCYSNAAMRAqnXAKCgc7sZ2EbgGQ1EXZOFuIjIUz/hpACePooL
dKU4XLi0WeBCFAR0IyccdM8=
=lDDY
-----END PGP SIGNATURE-----