Hi Chia-Ping, > be we can add public helpers to Header to offer “last header”? The helper can cast the Iterable to List (if ok) to get last header effectively
Are you referring to extracting the RecordHeaders#lastHeader() <https://github.com/apache/kafka/blob/0123bdcf06d2554ea62ba3a75af7c4b17eb4d23a/clients/src/main/java/org/apache/kafka/common/header/internals/RecordHeaders.java#L84-L94> implementation to a static helper method like Header.lastHeader(Iterable<Header>)? That’s an interesting idea, but I think it falls outside the scope of this KIP. If you’re interested, perhaps you could start another discussion thread or raise a separate KIP for it. > should we pass a mutable interface to users in those callback function? This is a great question. Passing read-only Headers to these callbacks would maintain a unified experience and also address your concern. In fact, KafkaProducer already setReadOnly <https://github.com/apache/kafka/blob/0123bdcf06d2554ea62ba3a75af7c4b17eb4d23a/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java#L1058C13-L1058C24> during `doSend`. It is more of an implementation detail. Overall, I believe the interface of `Headers` should be kept as is. What do you think? ``` onAcknowledgement(RecordMetadata metadata, Exception exception, Headers headers) onCompletion(RecordMetadata metadata, Exception exception, Headers headers) ``` Regards, Rich On Fri, Aug 23, 2024 at 2:12 AM Chia-Ping Tsai <chia7...@gmail.com> wrote: > hi Rich, > > Thanks for sharing the case. Maybe we can add public helpers to Header to > offer “last header”? The helper can cast the Iterable to List (if ok) to > get last header effectively > > Thanks, > Chia-Ping > > > > > Rich C. <chenjy.r...@gmail.com> 於 2024年8月23日 中午12:00 寫道： > > > > Hi Chia-Ping, > > > > To address your concern, how about we clone the headers as RecordHeaders > > and then use setReadOnly? This approach will pass immitable interface to > > callback and won’t affect the developer experience. > > > > Regards, > > Rich > > > > > >> On Thu, Aug 22, 2024 at 23:29 Rich C. <chenjy.r...@gmail.com> wrote: > >> > >> Hi Chia-Ping, > >> > >> I agree that `Headers > >> < > https://kafka.apache.org/38/javadoc/org/apache/kafka/common/header/Headers.html > >` > >> extending `Iterable<Header>` includes unnecessary mutable methods like > >> add() and remove(). However, Headers does offer a useful `lastHeader()` > >> method. > >> While I understand your point that it's not overly complex to iterate > >> through and get the last header, I'm concerned that using > >> `Iterable<Header>` might create a *non-unified developer experience* > >> between these callbacks and the existing ProducerRecord.headers() > >> < > https://kafka.apache.org/38/javadoc/org/apache/kafka/clients/producer/ProducerRecord.html#headers() > > > >> and ConsumerRecord.headers() > >> < > https://kafka.apache.org/38/javadoc/org/apache/kafka/clients/consumer/ConsumerRecord.html#headers() > > > >> . > >> > >> Could this be an issue? > >> > >> Regards, > >> Rich > >> > >> > >> On Thu, Aug 22, 2024 at 10:05 PM Chia-Ping Tsai <chia7...@gmail.com> > >> wrote: > >> > >>> hi Rich > >>> > >>> Headers is a sub class of Iterable<Header> already, so changing the > type > >>> from Headers to Iterable<Header> does not make code complicated :) > >>> > >>> The point is “should we pass a mutable interface to users in those > >>> callback function?” > >>> > >>> IMHO, the answer is NO. > >>> > >>> Best, > >>> Chia-Ping > >>> > >>>> > >>>> Rich C. <chenjy.r...@gmail.com> 於 2024年8月23日 上午8:26 寫道： > >>>> > >>>> Hi Chia-Ping, > >>>> > >>>> I initially considered `Iterable<Header>`, but eventually went with > >>>> `Headers`. Because ProducerRecord.headers() > >>>> < > >>> > https://kafka.apache.org/38/javadoc/org/apache/kafka/clients/producer/ProducerRecord.html#headers() > >>>> > >>>> type is `Headers`. > >>>> > >>>> Regards, > >>>> Rich > >>>> > >>>> > >>>>> On Thu, Aug 22, 2024 at 4:50 PM Chia-Ping Tsai <chia7...@apache.org> > >>> wrote: > >>>>> > >>>>> hi Rich > >>>>> > >>>>> Sorry for late response. I have just one comment: > >>>>> > >>>>> Have we consider replacing `Headers` by `Iterable<Header>`? There are > >>> some > >>>>> disadvantages of using `Headers`: > >>>>> > >>>>> 1. `Headers` have many setters and they are meaningless to users. > >>>>> 2. If users do want to modify `Headers`, they can get inconsistent > >>> results > >>>>> as `Headers` can be either readonly of modifiable. > >>>>> > >>>>> Best, > >>>>> Chia-Ping > >>>>> > >>>>>> On 2024/07/23 03:13:59 "Rich C." wrote: > >>>>>> Hi Everyone, > >>>>>> > >>>>>> I hope this email finds you well. > >>>>>> > >>>>>> I would like to start a discussion on KIP-512. The initial version > of > >>>>>> KIP-512 was created in 2019, and I have resurrected it in 2024 with > >>> more > >>>>>> details about the motivation behind it. > >>>>>> > >>>>>> You can view the current version of the KIP here: KIP-512: Make > Record > >>>>>> Headers Available in onAcknowledgement. > >>>>>> < > >>>>> > >>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-512%3A+make+Record+Headers+available+in+onAcknowledgement > >>>>>> > >>>>>> > >>>>>> Let's focus on discussing the necessity of this feature first. If we > >>>>> agree > >>>>>> on its importance, we can then move on to discussing the proposed > >>>>> changes. > >>>>>> > >>>>>> Looking forward to your feedback. > >>>>>> > >>>>>> Best regards, > >>>>>> Rich > >>>>>> > >>>>> > >>> > >> >
