[PATCH 2 of 7 iterbatch] wireproto: document quirk of _callstream between http and ssh
pierre-yves.david at ens-lyon.org
Thu Mar 10 09:30:01 EST 2016
On 03/09/2016 01:02 AM, Augie Fackler wrote:
>> On Mar 8, 2016, at 6:54 PM, Martin von Zweigbergk <martinvonz at google.com> wrote:
>> On Mon, Mar 7, 2016 at 8:25 PM, Augie Fackler <raf at durin42.com> wrote:
>>> # HG changeset patch
>>> # User Augie Fackler <augie at google.com>
>>> # Date 1456946323 18000
>>> # Wed Mar 02 14:18:43 2016 -0500
>>> # Node ID 071d05cce9156434dee4696287119f284631701b
>>> # Parent 5fd58c8c55d052c671a4725d01f27fb1c14353a9
>>> # EXP-Topic batch
>>> wireproto: document quirk of _callstream between http and ssh
>>> This tripped me up when trying to use it, so it feels like we should
>>> document this to avoid future pain.
>>> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
>>> --- a/mercurial/wireproto.py
>>> +++ b/mercurial/wireproto.py
>>> @@ -396,9 +396,12 @@ class wirepeer(peer.peerrepository):
>>> def _callstream(self, cmd, **args):
>>> """execute <cmd> on the server
>>> - The command is expected to return a stream.
>>> + The command is expected to return a stream. Note that if the
>>> + command doesn't return a stream, _callstream behaves
>>> + differently for ssh and http peers.
>>> - returns the server reply as a file like object."""
>>> + returns the server reply as a file like object.
>>> + """
>> I find the documentation very confusing, but that's mostly unrelated
>> to your patch. So the method is expected to return a stream, but it
>> doesn't have to? What does that mean? And is a "file like object" a
> httppeer returns a file-like object which hits EOF when the specific response ends. This is the same wether or not the called method is defined to be a streaming method.
> sshpeer returns a file-like object, which is the stdout of the ssh subprocess. As a result, it *does not* hit EOF at the end of a given response. If the method is defined to be a streaming method, the response body must be self-describing at least as far as content length. If the method is *not* a streaming method (as is the case for the batch() method today), then the sshpeer first emits “%d\n” % len(response), and then the client knows to read that many bytes after the newline.
> A good future cleanup would probably be to introduce a streambatch() wireproto method and then deprecate batch() so that the batching behavior can be the same on both ssh and http.
Patches 1 and 2 are on the clowncopter.
It would probably be useful to follow up with a documentation update
mentionning this ssh/http business (small layer violation, but I think
it is useful)
More information about the Mercurial-devel