[PATCH 2 of 7 iterbatch] wireproto: document quirk of _callstream between http and ssh

[Pierre-Yves David]

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
>> stream?
>
> 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)

Cheers.

-- 
Pierre-Yves David