[MERGE] chunked sftp readv

Martin Pool mbp at canonical.com
Thu Sep 4 06:38:40 BST 2008


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

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

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

For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C48BF5041.5020906%40arbash-meinel.com%3E
Project: Bazaar



More information about the bazaar mailing list