Code review comment for lp:~seif/zeitgeist/fixing-payloads

On 21 December 2010 11:58, Seif Lotfy <email address hidden> wrote:
> Ofcourse it wont work. If you send a payload from the client to the engine
> it will be an array of bytes. It is translated that way by dbus.
> This means u wont get ur "Hello World" string on the other side.
>
> The problem in the first place was that remotely we couldnt get events with
> payloads. So this is where this all starts.
> Try the the testEventWithStringPayload. Since it fails to send it back to
> the client it means we are facing an internal issue.
> I narrowed it down and I think my approach is actually right. I mean if u
> send a binary u need to decode it to start something with it.
> The test cases work.
>
> The only other solution imho would be to change payload to a string instead
> of an array of bytes.

Technically a string is *exactly identical* to an array of bytes. "The
difference is the same" as we say in Denmark :-) It's just the Python
logic here that messes it up, in C, fx., it's just a matter of how you
cast the pointers.

The contract that makes sense is twofold:

 1) In the Python client API: event_before.payload == event_after.payload, and
 2) On the DBus wire level the same invariant: event_before[2] == event_after[2]

So this is what the unit tests should assert. It seems we have a bug
here and that needs fixing of course - and a non-invasive fix like
Seif's patch here seems ok by me, even if there may theoretically
cleaner ways we can do this. Given no concrete alternatives we should
merge Seif's branch given that it implements the contract as I
outline.

That's my take on this at least.