Hi, Michael, The KIP looks good to me overall. Just one comment. The wiki says "This will be done by calling "close()" method". However, there is no close() in Headers.
Thanks, Jun On Thu, Mar 23, 2017 at 9:34 AM, Michael Pearce <michael.pea...@ig.com> wrote: > Thanks all for the +1 votes so far, just one more committer vote needed ( > > Please note: > > I have made one small adjustment to the kip based on Ismael’s comment in > discussion thread, and further requested by Jason in the vote thread. > > Please note the below method is changed based on this feedback. > > Headers.header(key)` to be `Headers.lastHeader(key) > > Thanks > Mike > > On 22/03/2017, 16:39, "Joel Koshy" <jjkosh...@gmail.com> wrote: > > +1 > > On Tue, Mar 21, 2017 at 5:01 PM, Jason Gustafson <ja...@confluent.io> > wrote: > > > Thanks for the KIP! +1 (binding) from me. Just one nit: can we change > > `Headers.header(key)` to be `Headers.lastHeader(key)`? It's not a > > deal-breaker, but I think it's better to let the name reflect the > actual > > behavior as clearly as possible. > > > > -Jason > > > > On Wed, Feb 15, 2017 at 6:10 AM, Jeroen van Disseldorp < > jer...@axual.io> > > wrote: > > > > > +1 on introducing the concept of headers, neutral on specific > > > implementation. > > > > > > > > > > > > On 14/02/2017 22:34, Jay Kreps wrote: > > > > > >> Couple of things I think we still need to work out: > > >> > > >> 1. I think we agree about the key, but I think we haven't > talked > > about > > >> the value yet. I think if our goal is an open ecosystem of > these > > >> header > > >> spread across many plugins from many systems we should > consider > > >> making this > > >> a string as well so it can be printed, set via a UI, set in > config, > > >> etc. > > >> Basically encouraging pluggable serialization formats here > will lead > > >> to a > > >> bit of a tower of babel. > > >> 2. This proposal still includes a pretty big change to our > > >> serialization > > >> and protocol definition layer. Essentially it is introducing > an > > >> optional > > >> type, where the format is data dependent. I think this is > actually a > > >> big > > >> change though it doesn't seem like it. It means you can no > longer > > >> specify > > >> this type with our type definition DSL, and likewise it > requires > > >> custom > > >> handling in client libs. This isn't a huge thing, since the > Record > > >> definition is custom anyway, but I think this kind of protocol > > >> inconsistency is very non-desirable and ties you to > hand-coding > > >> things. I > > >> think the type should instead by [Key Value] in our BNF, > where key > > and > > >> value are both short strings as used elsewhere. This brings > it in > > >> line with > > >> the rest of the protocol. > > >> 3. Could we get more specific about the exact Java API change > to > > >> ProducerRecord, ConsumerRecord, Record, etc? > > >> > > >> -Jay > > >> > > >> On Tue, Feb 14, 2017 at 9:42 AM, Michael Pearce < > michael.pea...@ig.com> > > >> wrote: > > >> > > >> Hi all, > > >>> > > >>> We would like to start the voting process for KIP-82 – Add record > > >>> headers. > > >>> The KIP can be found > > >>> at > > >>> > > >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > >>> 82+-+Add+Record+Headers > > >>> > > >>> Discussion thread(s) can be found here: > > >>> > > >>> http://search-hadoop.com/m/Kafka/uyzND1nSTOHTvj81?subj= > > >>> Re+DISCUSS+KIP+82+Add+Record+Headers > > >>> http://search-hadoop.com/m/Kafka/uyzND1Arxt22Tvj81?subj= > > >>> Re+DISCUSS+KIP+82+Add+Record+Headers > > >>> http://search-hadoop.com/?project=Kafka&q=KIP-82 > > >>> > > >>> > > >>> > > >>> Thanks, > > >>> Mike > > >>> > > >>> 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. > > >>> > > >>> > > > > > > > > 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. >
