[MERGE] Add call_with_body_stream to _SmartClient; add support for streamed request bodies to the server.
Andrew Bennetts
andrew.bennetts at canonical.com
Tue Jan 27 05:56:00 GMT 2009
John Arbash Meinel wrote:
> John Arbash Meinel has voted tweak.
> Status is now: Conditionally approved
> Comment:
> +
> + Possible states:
> + * expecting args
> + * expecting body (terminated by receiving a post-body status)
> + * finished
> """
>
>
> ^- It seems that this is incomplete. I'm not sure what other states are
> available, but at the very least I saw:
Good point. I've expanded it to read:
Possible states:
* args: expecting args
* body: expecting body (terminated by receiving a post-body status)
* error: expecting post-body error
* end: expecting end of message
* nothing: finished
> I find it a little bit odd that the "accept_body" function is now
> calling
> "do_chunk" rather than calling "do_body".
This is intentional, although maybe the name of accept_body could be clearer.
accept_body functions in the smart code have always taken incomplete body parts.
Previously this accept_body was just accumulating the body in a string; now it's
up to the SmartServerRequest to decide how to deal with these parts (by default
it accumulates as before).
> v- Having run into this elsewhere:
[...]
> + self._body_bytes += chunk_bytes
[...]
>
> ^- It would be a lot better if "self._body_bytes" was a list that we
> appended
> to, and then used ''.join() if we had to. Remember the terrible scaling
> of
> 'readv()' when it was reading 100MB, 64k chunks at a time.
Ok, done. The only reason I hadn't already done this was just to try keep this
patch somewhat minimal (this is the current behaviour shifted to a different
class).
> === modified file 'bzrlib/util/bencode.py'
> --- bzrlib/util/bencode.py 2007-07-13 04:22:17 +0000
> +++ bzrlib/util/bencode.py 2009-01-09 05:52:37 +0000
> @@ -52,7 +52,7 @@
> while x[f] != 'e':
> v, f = decode_func[x[f]](x, f)
> r.append(v)
> - return (r, f + 1)
> + return (tuple(r), f + 1)
>
>
> ^- Also, these changes seem specifically incorrect now, as you don't
> really
> want to universally change the api to return tuples instead of lists. It
> probably just means that this can't land until some form of your other
> patch
> lands.
Right, I even mentioned it in my preamble to this patch.
<http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C20090107022415.GA2394%40steerpike.home.puzzling.org%3E>
is still waiting for a review :)
Actually, I'll probably just merge the bencode patch in a day or so if it stays
unreviewed; I'm quite sure it's reasonable and safe, and I don't want it to
block this call_with_body_stream patch.
-Andrew.
More information about the bazaar
mailing list