Hi Knowles, Thanks for the reply! That all sounds reasonable to me, and that's a good catch regarding the SourceRecord.
Thanks, -John On Wed, 2021-10-27 at 15:32 -0400, Knowles Atchison Jr wrote: > John, > > Thank you for the response and feedback! > > I originally started my first pass with the ProducerRecord<byte[], byte[]>. > For our connector, we need some of the information out of the SourceRecord > to ack our source system. If I had the actual ProducerRecord, I would have > to convert it back before I would be able to do anything useful with it. I > think there is merit in providing both records as parameters to this > callback. Then connector writers can decide which of the representations of > the data is most useful to them. I also noticed that in my PR I was sending > the SourceRecord post transformation, when we really should be sending the > preTransformRecord. > > The Streams solution to this is very interesting. Given the nature of a > connector, to me it makes the most sense for the api call to be part of > that task rather than an external class that is configurable. This allows > the connector to use state it may have at the time to inform decisions on > what to do with these producer exceptions. > > I have updated the KIP and PR. > > Knowles > > On Wed, Oct 27, 2021 at 1:03 PM John Roesler <vvcep...@apache.org> wrote: > > > Good morning, Knowles, > > > > Thanks for the KIP! > > > > To address your latest questions, it is fine to call for a > > vote if a KIP doesn't generate much discussion. Either the > > KIP was just not controversial enough for anyone to comment, > > in which case a vote is appropriate; or no one had time to > > review it, in which case, calling for a vote might be more > > provacative and elicit a response. > > > > As far as pinging people directly, one idea would be to look > > at the git history (git blame/praise) for the files you're > > changing to see which committers have recently been > > involved. Those are the folks who are most likely to have > > valuable feedback on your proposal. It might not be > > appropriate to directly email them, but I have seen KIP > > discussions before that requested feedback from people by > > name. It's probably not best to lead with that, but since no > > one has responded so far, it might not hurt. I'm sure that > > the reason they haven't noticed your KIP is just that they > > are so busy it slipped their radar. They might actually > > appreciate a more direct ping at this point. > > > > I'm happy to review, but as a caveat, I don't have much > > experience with using or maintaining Connect, so caveat > > emptor as far as my review goes. > > > > First of all, thanks for the well written KIP. Without much > > context, I was able to understand the motivation and > > proposal easily just by reading your document. > > > > I think your proposal is a good one. It seems like it would > > be pretty obvious as a user what (if anything) to do with > > the proposed method. > > > > For your reference, this proposal reminds me of these > > capabilities in Streams: > > > > https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/errors/DeserializationExceptionHandler.java > > and > > > > https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/errors/ProductionExceptionHandler.java > > . > > > > I'm not sure if there's value in bringing your proposed > > interface closer to that pattern or not. Streams and Connect > > are quite different domains after all. At least, I wanted > > you to be aware of them so you could consider the > > alternative API strategy they present. > > > > Regardless, I do wonder if it would be helpful to also > > include the actual ProducerRecord we tried to send, since > > there's a non-trivial transformation that takes place to > > convert the SourceRecord into a ProducerRecord. I'm not sure > > what people would do with it, exactly, but it might be > > helpful in deciding what to do about the exception, or maybe > > even in understanding the exception. > > > > Those are the only thoughts that come to my mind! Thanks > > again, > > -John > > > > On Wed, 2021-10-27 at 09:16 -0400, Knowles Atchison Jr > > wrote: > > > Good morning, > > > > > > Bumping this thread. Is there someone specific on the Connect framework > > > team that I should ping? Is it appropriate to just call a vote? All > > source > > > connectors are dead in the water without a way to handle producer write > > > exceptions. Thank you. > > > > > > Knowles > > > > > > On Mon, Oct 18, 2021 at 8:33 AM Christopher Shannon < > > > christopher.l.shan...@gmail.com> wrote: > > > > > > > I also would find this feature useful to handle errors better, does > > anyone > > > > have any comments or feedback? > > > > > > > > > > > > On Mon, Oct 11, 2021 at 8:52 AM Knowles Atchison Jr < > > katchiso...@gmail.com > > > > > > > > > wrote: > > > > > > > > > Good morning, > > > > > > > > > > Bumping this for visibility. I would like this to go into the next > > > > release. > > > > > KIP freeze is Friday. > > > > > > > > > > Any comments and feedback are welcome. > > > > > > > > > > Knowles > > > > > > > > > > On Tue, Oct 5, 2021 at 4:24 PM Knowles Atchison Jr < > > > > katchiso...@gmail.com> > > > > > wrote: > > > > > > > > > > > Hello all, > > > > > > > > > > > > I would like to discuss the following KIP: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-779%3A+Allow+Source+Tasks+to+Handle+Producer+Exceptions > > > > > > > > > > > > The main purpose is to allow Source Tasks the ability to see > > underlying > > > > > > Producer Exceptions and decide what to do rather than being > > killed. In > > > > > our > > > > > > use cases we would want to log/write off some information and > > continue > > > > > > processing. > > > > > > > > > > > > PR is here: > > > > > > > > > > > > https://github.com/apache/kafka/pull/11382 > > > > > > > > > > > > Any comments and feedback are welcome. > > > > > > > > > > > > > > > > > > Knowles > > > > > > > > > > > > > > > > > > > > >
