[Twisted-Python] revert / rework of "AMPv2" change?

[Glyph]

> On Oct 21, 2020, at 10:34 AM, Jonathan Bastien-Filiatrault <joe at x2a.org> wrote:
> 
> On 2020-10-20 04 h 57, adi at roiban.ro (Adi Roiban) wrote:
>> Just my 2 cents
>> I think that whether this should be reverted or reworked should be between
>> the person raising this issue (Glyph) and the author of the PR (Jonathan).
> Glyph has asked me to submit the type annotation work on AMP, I will do
> this. I also plan on resubmitting a reworked patch that addresses the
> issues that glyph raised.
>> 
>> I see that Jonathan has responded to the feedback inside the merged PR.I
>> hope it can be reworked.
>> 
>> Since AMP v2 is not standardized maybe is ok to have it as a separate
>> experimental top level name.
>> 
>> I have never used AMP ...I tried to use it via ampoule but ended up using
>> the stdlib process pool as I was not able see any performance difference.
>> 
>> Since `amp` is used by `trial -j`  we need to keep it inside twisted.
>> 
>> But since AMP v2 is experimental, I think that is best to have it as a
>> separate txamp2 project.
>> In this way, the twisted AMP v2 project can follow its own rules.
>> Once AMP v2 is "mature" it can be moved to core twisted
>> 
>> So my take is to revert it and move it to twisted/txamp2.
> 
> I have no issue with changing the name or if this patch lives as a
> project besides twisted. I might even rework it into something more
> forward-compatible as suggested by glyph. I think netstrings are a
> decent option as it would make long value handling easier since values
> would always be contiguous without "continuation markers" interspersed.
> The main thing I dislike about netstrings are the variable-length ASCII
> numbers, but that is not a big complexity.

I originally didn't like this either, but since I designed AMP I've come around:

The thing about variable-length ASCII numbers is that they give you a lot of leeway to error out in case the protocol is not being respected.  With packed binary values, any 2-byte sequence is a valid length, so if something opens a socket and says "EHLO", it starts waiting for 17734 more bytes which it will never receive.  By contrast, netstrings have built-in redudancy; for a very low overhead (most length prefixes end up being 2-3 bytes rather than 2) you get the ability to immediately error out with a clear exception as soon as you see "E".  Anyone implementing this in a static context where they don't want heap allocations for the length prefix itself should not allocate arbitrary data, but rather, allocate a very small fixed buffer (6 bytes will get you considerably beyond the built-in limitations of AMP already) and again, immediately error out if that is exceeded.  Similarly the trailing comma provides an in-band sanity check where buggy applications will promptly crash rather than bleeding into the next message.

> Could the next AMP version
> use values framed using 32 bit or 64 bit integers ?

I'd really rather go with netstrings for all the reasons listed above.

> Transmitting long-ish values is a real use case and whatever extension
> we do must retain the dead-simple, easy to implement nature of the
> current AMP protocol. I now realise that I somewhat hijacked the
> "streaming values" bug (2529). IMHO, streaming should be handled at the
> application level, but I digress.

Yeah, that bug is more about making it easy to do the application-level stuff than implementing something implicit within the protocol.

>> ----
>> 
>> BTW this is also my suggestion for the PR trying to introduce support for
>> SMB.... have twisted SMB implementation in a separate project.
>> 
> All the best,
> 
> Jonathan

Same to you.  Thanks for your contributions, and sorry for the slow pace of communication here; 2020 is a heck of a year.

-g
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20201128/8025b4dd/attachment.htm>