Andrew, I agree it's a better commitRecord, but with the slightly different semantics you mentioned. I suppose we could document that well enough that reusing the same name would be fine.
I'll resend the discussion email. Maybe it got lost somehow. Ryanne On Mon, Jan 21, 2019, 4:37 AM Andrew Schofield <andrew_schofi...@live.com wrote: > Hi, > I'm not quite sure about the etiquette here but I wonder whether the KIP > could be improved. I think I missed the DISCUSS thread. > > I think that really your recordLogged(SourceRecord, RecordMetadata) method > is actually a better version of commitRecord() and perhaps it ought to be > an overload. This is similar to the situation in which the Serializer > interface was enhanced when record headers were added. > > public abstract class SourceTask implements Task { > public void commitRecord(SourceRecord sourceRecord, RecordMetadata > recordMetadata) { > this.commitRecord(); > } > > public void commitRecord() { > } > } > > Or something like that. I do understand that the KIP mentions that > recordLogged() is only called for records that are actually ACKed, but it's > very similar in intent to commitRecord() in my view. > > Just my 2 cents. > > Andrew Schofield > IBM Event Streams > > > On 17/01/2019, 23:54, "Ryanne Dolan" <ryannedo...@gmail.com> wrote: > > Hey y'all, please vote for KIP-416 by replying +1 to this thread. > > Right now, there is no way for a SourceConnector/Task to know: > > - whether a record was successfully sent to Kafka, vs filtered out or > skipped. > - the downstream offsets and metadata of sent records > > KIP-416 proposes adding a recordLogged() callback for this purpose. > > > https://nam05.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FKAFKA%2FKIP-416%253A%2BNotify%2BSourceTask%2Bof%2BACK%2527d%2Boffsets%252C%2Bmetadata&data=02%7C01%7C%7C9fa617754cce4bab7ba508d67cd7128f%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636833660500817715&sdata=udEP27%2FrshuP5sWthvZmUIdt13whM5XqKMoia1wE93c%3D&reserved=0 > > Thanks! > Ryanne > > >