> On Jan. 27, 2015, 7:37 p.m., Rafael Schloming wrote: > > proton-c/bindings/python/proton/__init__.py, line 2701 > > <https://reviews.apache.org/r/30318/diff/1/?file=836290#file836290line2701> > > > > This seems to change the semantics of send in a subtle but odd way. If > > you pass in something that it knows how to encode into bytes, it will > > assume that constitutes an entire message and call advance and > > automatically generate a tag for you. Yet if you pass in raw bytes, it > > doesn't do this. I think these two things should really be controlled > > orthogonally, e.g. I might want to be able to configure the sender to > > assume every call to send is a single message even if I'm just passing in > > raw bytes, and I might also want to be able to stream large messages out by > > passing in higher level message chunks that the sender automatically > > encodes into bytes for me. > > Gordon Sim wrote: > WHat I want is a simple, single call to send a message. I'd like that to > be called send(), since that seems a clear and intuitive name for it. However > that clashes with the existing definition. I would argue that send() isn't > such a great name for the current semantics in that it isn't (in my view) > clear that you first need to create a delivery and then need to call > advance() when done. To me the current semantic is more like an append_data(). > > However, I didn't want to impose a breaking change. Hence the somewhat > odd combination of semantics for the same name. > > There are (at least) three ways to deal with this: > > (1) use send_message() for the new simple, single call semantic and leave > send() as is. I could live with that but it seems a shame to me to use the > obvious name for the less obvious (and I believe less commonly required) > semantic > > (2) use send() exclusively for the new simple, single call semantic, and > have send_bytes() - or better still perhaps, append_data() - as the old > semantic. This is obviously a breaking API change though, not sure how > acceptable that would be? > > (3) add in some flag(s)/option(s) to control the single-shot v. > part-of-sequence-of-calls semantics. However the defaults need to be decided. > So agin, we can default it to the current bheaviour, but that in my view is > less intuitive and less commonly what users want. Or we can default it to the > new behaviour, but that then is another breaking change. > > To me, if a breaking change is acceptable, I'd probably go for something > like (2), because I think it will be clearer. Perhaps I'm just not > imaginative enough yet on what the flags/options might be like in 3. > > If a breaking change isn't acceptable, I'd probably rather just stick > with a send_message() rather than have a send() where a non-default option > was required to trigger the single-shot behaviour.
I agree with you that send(Message) should do the obvious thing, so I'm not keen on (1). I don't agree that send is a bad name for the streaming behaviour though. I think in the scenario where you are streaming a large message out over the wire, you are still very much sending, it's just that you are sending "Message Chunks" rather than entire messages, but you are still sending a unit of work that you expect to make it all the way over the wire to the receiver. I also think that automatically rendering objects into amqp bytes shouldn't be only associated with the non streaming case, in other words I want to be able to do something like this: send(Header); send(Chunk); ...; send(Footer) Where each of those things are actual objects that know how to render themselves into the appropriate AMQP section. One possibility here might be to delegate back to the object itself again for a bit more flexibility, e.g. have send check if the object being sent has some sort of "send_yourself" method and then delegate back to it. That would allow the object to then set up whatever delivery tags it wanted, encode itself however it wanted, and then choose whether or not to advance the link. Presumably in the above example the Header would set up the delivery tag, the Chunk would just send more data, and the Footer would advance the link. That of course does leave the question of defaults when you pass in raw bytes. To keep backwards compatibility we would need to say that passing in raw bytes is equivalent to passing in Chunk(bytes), assuming Chunk is defined to be roughly what I described above. I'm not opposed to breaking compatibility here though, I think that's partly the point of this release. - Rafael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30318/#review69863 ----------------------------------------------------------- On Jan. 27, 2015, 2 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30318/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2015, 2 p.m.) > > > Review request for qpid and Rafael Schloming. > > > Repository: qpid-proton-git > > > Description > ------- > > Sending a message requires several steps: create a delivery with a unique > tag, encode the message, cal send(), call advance. While this gives a lot of > flexibility and power (e.g. streaming messages in chunks without ever holding > the entire thing in memory), for many, many use cases, a simpler solution > would be adequate and would avoid users having to learn what is in my view > one of the more esoteric aspects of the API. > > The attached patch adds send support for a Message object, such that a > message can be sent with a single call. It also allows the send() method to > be 'dual purpose'. It remains backwards compatible as a way of appending > bytes to the current delivery, but can also be used as a one-step call to > send a single message. > > At present the engine examples in python make use of a similar > simplification, but one that is added dynamiccaly and is therefore less clean > and obvious. It currently also doesn't work as well for sending on links > established by the remote peer. This addition would in my view allow for > simpler use where needed, without breaking any existing usage. > > > Diffs > ----- > > proton-c/bindings/python/proton/__init__.py 17cef30 > > Diff: https://reviews.apache.org/r/30318/diff/ > > > Testing > ------- > > > Thanks, > > Gordon Sim > >
