hi Rich > 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.
The APIs included by this KIP is public, and so we can't change it easily after it gets release even though I filed another KIP to add helper. Also, that is why I stick on the APIs. > 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 if the callback is invoked on the error path ( https://github.com/apache/kafka/blob/0123bdcf06d2554ea62ba3a75af7c4b17eb4d23a/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java#L1111)? the simple solution is to call `setReadOnly` to make it immutable, so it is fine. In fact, I'm not a fan of `Headers` as it uses "exception" to prevent the modification. IMHO, the better design is to define a read-only interface. At any rate, that is not related to this KIP. thanks for having discussion with me. I will +1 to this KIP and file a Jira to discuss the interface changes of `Headers` Best, Chia-Ping Rich C. <chenjy.r...@gmail.com> 於 2024年8月24日 週六 上午9:30寫道： > 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 > > >>>>>> > > >>>>> > > >>> > > >> > > >
