Sent to early:
Hi Radai: RE: Header header(String key) - returns JUST ONE (the very last) value given a key Iterable<Header> headers(String key) - returns ALL under a key Iterable<Header> headers() - returns all, period. maybe allow null as key to prev method instead? void add(Header header) - appends a header (key inside). void remove(String key) - removes ALL HEADERS under a key. I don't think this one is needed: "Iterable<Header> headers() - returns all, period. maybe allow null as key to prev method instead" The class Headers implements Iterable<Header> anyhow. On another note of the response for the add, remove. I note you put void. Any particular reason? for add, i would normally either expect a boolean response if it succeeded or not (classical case) or the instance object returned like the "with" pattern. This avoids in the case of when its in read only, needing to throw exception, simply return false (in classical case). Like wise on remove normally i would expect a boolean or previous object returned, so again operation didn't succeed either i get a false, or a null object. any objections if i make the response's for these to booleans to denote if the operation succeeded? Cheers Mike ________________________________________ From: Michael Pearce Sent: Wednesday, March 1, 2017 5:55 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-82 - Add Record Headers Hi Radai: RE: Header header(String key) - returns JUST ONE (the very last) value given a key Iterable<Header> headers(String key) - returns ALL under a key void add(Header header) - appends a header (key inside). void remove(String key) - removes ALL HEADERS under a key. I don't think this one is needed: ________________________________________ From: Becket Qin <becket....@gmail.com> Sent: Wednesday, March 1, 2017 2:54 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-82 - Add Record Headers Hi Ismael, Yes, there is a difference between Batch and Headers. I was just trying to see if that would work. Good point about sending the same ProducerRecord twice, but in fact in that case any reuse of objects would cause problem. As you can imagine if the ProducerRecord has a value as a List and the Interceptor.onSend() can actually add an element to the List. If the producer.send() is called on the same ProducerRecord again, the value list would have one more element than the previously sent ProducerRecord even though the ProducerRecord itself is not mutable, right? Same thing can apply to any modifiable class type. >From this standpoint allowing headers to be mutable doesn't really weaken the mutability we already have. Admittedly a mutable header is kind of guiding user towards to change the headers in the existing object instead of creating a new one. But I think reusing an object while it is possible to be modified by user code is a risk that users themselves are willing to take. And we do not really protect against that. But there still seems value to allow the users to not pay the overhead of creating tons of objects if they do not reuse an object to send it twice, which is probably a more common case. Thanks, Jiangjie (Becket) Qin On Tue, Feb 28, 2017 at 12:43 PM, radai <radai.rosenbl...@gmail.com> wrote: > I will settle for any API really, but just wanted to point out that as it > stands right now the API targets the most "advanced" (hence obscure and > rare) use cases, at the expense of the simple and common ones. i'd suggest > (as the minimal set): > > Header header(String key) - returns JUST ONE (the very last) value given a > key > Iterable<Header> headers(String key) - returns ALL under a key > Iterable<Header> headers() - returns all, period. maybe allow null as key > to prev method instead? > void add(Header header) - appends a header (key inside). > void remove(String key) - removes ALL HEADERS under a key. > > this way naive get/set semantics map to header(key)/add(Header) cleanly and > simply while preserving the ability to handle more advanced use cases. > we can always add more convenience methods (like those dealing with lists - > addAll etc) but i think the 5 (potentially 4) above are sufficient for > basically everything. > > On Tue, Feb 28, 2017 at 4:08 AM, Ismael Juma <ism...@juma.me.uk> wrote: > > > Hi Becket, > > > > Comments inline. > > > > On Sat, Feb 25, 2017 at 10:33 PM, Becket Qin <becket....@gmail.com> > wrote: > > > > > > 1. Regarding the mutability. > > > > > > I think it would be a big convenience to have headers mutable during > > > certain stage in the message life cycle for the use cases you > mentioned. > > I > > > agree there is a material benefit especially given that we may have to > > > modify the headers for each message. > > > > > > That said, I also think it is fair to say that in the producer, in > order > > to > > > guarantee the correctness of the entire logic, it is necessary that at > > some > > > point we need to make producer record immutable. For example we > probably > > > don't want to see that users accidentally updated the headers when the > > > producer is doing the serialization or compression. > > > > > > Given that, would it be possible to make Headers to be able to switch > > from > > > mutable to immutable? We have done this for the Batch in the producer. > > For > > > example, initially the headers are mutable, but after it has gone > through > > > all the interceptors, we can call Headers.close() to make it immutable > > > afterwards. > > > > > > > The difference is that the batch is an internal class that is not exposed > > to users. Can you please explain what happens if a user tries to send the > > same ProducerRecord twice? Would an interceptor fail when trying to > mutate > > the header that is now closed? Or did you have something else in mind? > > > > Thanks, > > Ismael > > > The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
