Re: [PATCH v4 11/12] convert: add filter.<driver>.process option

[Lars Schneider <larsxschneider@gmail.com>]

> On 06 Aug 2016, at 00:27, Jeff King <peff@peff.net> wrote:
> 
> On Fri, Aug 05, 2016 at 03:06:28PM -0700, Junio C Hamano wrote:
> 
>> Torsten Bögershausen <tboegi@web.de> writes:
>> 
>>> On 2016-08-03 18.42, larsxschneider@gmail.com wrote:
>>>> The filter is expected to respond with the result content in zero
>>>> or more pkt-line packets and a flush packet at the end. Finally, a
>>>> "result=success" packet is expected if everything went well.
>>>> ------------------------
>>>> packet:          git< SMUDGED_CONTENT
>>>> packet:          git< 0000
>>>> packet:          git< result=success\n
>>>> ------------------------
>>> I would really send the diagnostics/return codes before the content.
>> 
>> I smell the assumption "by the time the filter starts output, it
>> must have finished everything and knows both size and the status".
>> 
>> I'd prefer to have a protocol that allows us to do streaming I/O on
>> both ends when possible, even if the initial version of the filters
>> (and the code that sits on the Git side) hold everything in-core
>> before starting to talk.
> 
> I think you really want to handle both cases:
> 
>  - the server says "no, I can't fulfill your request" (e.g., HTTP 404)

You can do this with the current protocol:

packet:          git< 0000
packet:          git< result=reject\n

Admittedly the flush packet could be consider overhead but I think
that is neglectable.


>  - the server can abort an in-progress response to indicate that it
>    could not be fulfilled completely (in HTTP chunked encoding, this
>    requires hanging up before sending the final EOF chunk)

Also already supported with the following sequence:

packet:          git< HALF_WRITTEN_ERRONEOUS_CONTENT
packet:          git< 0000
packet:          git< result=error\n


> If we expect the second case to be rare, then hanging up before sending
> the flush packet is probably OK. But we could also have a trailing error
> code after the data to say "ignore that, we saw an error, but I can
> still handle more requests".
> 
> It is true that you don't need the up-front status code in that case
> (you can send an empty body and say "ignore that, we saw an error") but
> that feels a little weird.

I understand your argument. However, I think "0000" indicates 
"I have nothing for you" and therefore I think it would be OK in the
reject case.


> And I expect it makes the lives of the client
> easier to get a code up front, before it starts taking steps to handle
> what it _thinks_ is probably a valid response.

I am not sure I can follow you here. Which actor are you referring to when
you write "client" -- Git, right? If the response is rejected right away
then Git just needs to read a single flush. If the response experiences
an error only later, then the filter wouldn't know about the error when
it starts sending. Therefore I don't see how an error code up front could
make it easier for Git.

- Lars