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

[Adi Roiban]

On Mon, 19 Oct 2020 at 18:18, Glyph <glyph at twistedmatrix.com> wrote:

> I just glanced at https://github.com/twisted/twisted/pull/1417 after it
> was merged and noticed a few problems:
>
> - It exposes a new top-level name rather than setting the protocol version
> as a parameter
> - The NEWS files are malformed which is going to lead to some confusing
> duplication in the changelog
> - AMPv2 itself does not appear to be "standardized" - the page at
> https://amp-protocol.net/conversations_v2.html appears to be a
> preliminary suggestion for how long values might be handled rather than
> something broadly implemented; for one thing, I'd never heard of it before
> and for another the site itself doesn't link the document. This would be
> fine as a first draft, but I think it needs to be given some more
> revisions, given that (for example) there are a number of
> backwards-compatible ways to do this.
> - The layering is wrong because it puts the protocol-parsing into a leaf
> class in the hierarchy, when the parsing logic was deliberately isolated to
> a lower level to facilitate different framing mechanisms.
> - As JP pointed out, the tests have a potential bug where they can leak
> errors between cases.
>
> I don't normally like to revert folks' work once it's been reviewed and
> accepted, particularly when there's no process violation (broken CI, lack
> of code coverage etc), but I'm particularly concerned about lending
> Twisted's imprimatur to this protocol extension as a whole new version
> without much more context on who is implementing it and what other options
> were considered, particularly when I personally (nominally the inventor of
> AMP) don't like the direction it has taken :).
>
> What do other folks think?  Anyone else have more of a finger on the pulse
> of where the "v2" conversation has been happening?
>
>
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).

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.

----

BTW this is also my suggestion for the PR trying to introduce support for
SMB.... have twisted SMB implementation in a separate project.

-- 
Adi Roiban
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20201020/088b9d1f/attachment.htm>