Hi all, Great to see the progress that has been achieved on this one. :) A few comments regarding the APIs (I'm still reviewing the message format changes):
1. Nit: `getHeaders` in `ProducerRecord` and `ConsumerRecord` should be named `headers` (we avoid the `get` prefix in Kafka) 2. The `Headers` class is mutable (there's an `add` method). Does it need to be? If so, it would be good to explain why. Related to that, we should also explain the thinking around thread-safety. If we keep the `add` method, it may make sense for it to take a `Header` (that way we can add things to `Header` without changing the interface). 3. Do we need the `Headers.get()` method? People usually assume that `get` would be efficient, but depending on the implementation (the current proposal states that an array would be used), it may not be. If we expect the number of headers to be small, it doesn't matter though. Ismael On Tue, Feb 21, 2017 at 6:38 PM, Michael Pearce <michael.pea...@ig.com> wrote: > Hi Jason, > > Have converted the interface/api bullets into interface code snippets. > > Agreed implementation won’t take too long. We have early versions already. > Maybe a week before you think about merging I would assume it would be more > stabilised? I was thinking then we could fork from your confluent branch, > making and then holding KIP-82 changes in a patch file, that we can then > re-fork from apache once KIP98 final is merged, and apply patch with last > minute changes. > > Cheers > Mike > > > On 22/02/2017, 00:56, "Jason Gustafson" <ja...@confluent.io> wrote: > > Hey Michael, > > Awesome. I have a minor request. The APIs are currently documented as a > wiki list. Would you mind adding a code snippet instead? It's a bit > easier > to process. > > How will be best to manage this, as we will obviously build off your > KIP’s > > protocol changes, to avoid a merge hell, should we branch from your > branch > > in the confluent repo or is it worth having a KIP-98 special branch > in the > > apache git, that we can branch/fork from? > > > I was thinking about this also. Ideally we'd like to get the changes > in as > close together as possible since we only want one magic bump and some > users > deploy trunk. The level of effort to change the format for headers > seems > not too high. Do you agree? My guess is that the KIP-98 message format > patch will take 2-3 weeks to review before we merge to trunk, so you > could > hold off implementing until that patch has somewhat stabilized. That > would > save some potential rebase pain. > > -Jason > > > 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. >
