On Sep 30, 2013, at 12:09 PM, Laurens Van Houtven <_@lvh.io> wrote:

Hi Glyph,

Thanks for your response!

On Mon, Sep 30, 2013 at 8:41 PM, Glyph <glyph@twistedmatrix.com> wrote:

On Sep 30, 2013, at 2:45 AM, Laurens Van Houtven <_@lvh.io> wrote:

What am I doing wrong? Is this a bug?

I think it's pretty clearly a bug. Calling the argument "proto" in the first place indicates the nature of the confusion.

There are parts of the flow here from bytes to method execution and back (like _wrapWithSerialization) which are nice for composition, but the fact that they're private sort of ruins their utility for extensibility.

Looking at the code you're trying to write in txampext though, the problem appears to be simply that you're writing functionality close enough to AMP's core that you should be making the changes to AMP directly, and fixing the issue by making changes to AMP itself rather than trying to work around it externally. The way I was going to recommend fixing it before I clicked on your link was by writing something like ExposingArgument and accessing the locator/receiver/sender via that new API rather than via the 'proto' argument at all :)

I'm a little confused why that would help; you're saying there should be a new API that gives arguments access to the locator, receiver, sender? What would that look like? Something along the lines of fromBox/toBox, or are you thinking of a more direct approach where the locator has a reference to the other components? (Given your suggestion of not going through the proto argument, I imagine something closer to the latter.)

For someone confused about why it would help, you are pretty close to the mark :).

I am not trying to propose a specific new implementation mechanism, but rather to say that fromBox/toBox are broken, in that the contract of the 'proto' argument is incompletely specified. Most of the code I can think of that wants to use that really wants the transport rather than the "protocol", but nothing within AMP itself actually uses those arguments; in fact, searching the usual suspects (epsilon, vertex) I can't even find any Arguments that use the 'proto' argument for anything useful.

If I recall, I believe the idea behind it was to allow an AMP responder within Vertex to return the peer's IP address back to the peer, from within an authenticated AMP route that (because it was a route) wasn't necessarily connected directly to the transport (and therefore couldn't just do self.transport.getPeer()). Ironically I don't think it'll actually work for that now :-).

When we pull the authentication logic in from <http://bazaar.launchpad.net/~divmod-dev/divmod.org/trunk/view/head:/Epsilon/epsilon/ampauth.py>, you might write a responder that's interested in authentication information that lives in some other relation to the protocol.

So in order to fix fromBox/toBox, we need to do a fix that firms up that contract and perhaps exposes more than a Protocol object. The *recommended* API should be more or less like what ExposingArgument is doing - specify an Argument that asks for a particular attribute of the transport or the protocol or the authentication context or whatever, the implementation details may involve other lower-level public APIs.

My contributions to AMP have been more of the defect-findy kind, but I could certainly turn them more into the code-contributy kind. I imagine I'm not the first person to want tests for command classes (https://github.com/lvh/txampext/blob/master/txampext/commandtests.py) or a nested AMP box (https://github.com/lvh/txampext/blob/master/txampext/nested.py).

That would be cool. And, you know, that auth thing I said :-).

I look forward to being in the same locality as you, I presume it will make me more productive ;)