Hi all, Wanted to bump this and see if it looks good enough for a third vote. Yash, any thoughts?
Cheers, Chris On Mon, Jan 29, 2024 at 2:55 AM Ashwin <apan...@confluent.io.invalid> wrote: > Thanks for reviewing this KIP, Yash. > > Could you please elaborate on the cleanup steps? For instance, if we > > encounter an error after wiping existing offsets but before writing the > new > > offsets, there's not really any good way to "revert" the wiped offsets. > > It's definitely extremely unlikely that a user would expect the previous > > offsets for a connector to still be present (by creating a new connector > > with the same name but without initial offsets for instance) after such a > > failed operation, but it would still be good to call this out > explicitly. I > > presume that we'd want to wipe the newly written initial offsets if we > fail > > while writing the connector's config however? > > > Agree - I have clarified the cleanup here - > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-995%3A+Allow+users+to+specify+initial+offsets+while+creating+connectors#KIP995:Allowuserstospecifyinitialoffsetswhilecreatingconnectors-ProposedChanges > . > > The `PATCH /connectors/{connector}/offsets` and `DELETE > > /connectors/{connector}/offsets` endpoints have two possible success > > messages in the response depending on whether or not the connector plugin > > has implemented the `alterOffsets` connector method. Since we're > proposing > > to utilize the same offset validation during connector creation if > initial > > offsets are specified, I think it would be valuable to surface similar > > information to users here as well > > > Thanks for pointing this out. I have updated the response to include a new > field “initial_offsets_response” which will contain the response based on > whether the connector implements alterOffsets or not. This also means that > if initial_offsets is set in the ConnectorCreate request, we will return a > new REST entity (ConnectorInfoWithInitialOffsetsResponse ?) which will be a > child class of ConnectorInfo. > > ( > > https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/entities/ConnectorInfo.java#L28-L28 > ) > > Thanks, > Ashwin > > On Wed, Jan 17, 2024 at 4:48 PM Yash Mayya <yash.ma...@gmail.com> wrote: > > > Hi Ashwin, > > > > Thanks for the KIP. > > > > > If Connect runtime encounters an error in any of these steps, > > > it will cleanup (if required) and return an error response > > > > Could you please elaborate on the cleanup steps? For instance, if we > > encounter an error after wiping existing offsets but before writing the > new > > offsets, there's not really any good way to "revert" the wiped offsets. > > It's definitely extremely unlikely that a user would expect the previous > > offsets for a connector to still be present (by creating a new connector > > with the same name but without initial offsets for instance) after such a > > failed operation, but it would still be good to call this out > explicitly. I > > presume that we'd want to wipe the newly written initial offsets if we > fail > > while writing the connector's config however? > > > > > Validate the offset using the same checks performed while > > > altering connector offsets (PATCH /$connector/offsets ) as > > > specified in KIP-875 > > > > The `PATCH /connectors/{connector}/offsets` and `DELETE > > /connectors/{connector}/offsets` endpoints have two possible success > > messages in the response depending on whether or not the connector plugin > > has implemented the `alterOffsets` connector method. Since we're > proposing > > to utilize the same offset validation during connector creation if > initial > > offsets are specified, I think it would be valuable to surface similar > > information to users here as well. Thoughts? > > > > Thanks, > > Yash > > > > On Wed, Jan 17, 2024 at 3:31 PM Ashwin <apan...@confluent.io.invalid> > > wrote: > > > > > Hi All , > > > > > > Can I please get one more binding vote, so that the KIP is approved ? > > > Thanks for the votes Chris and Mickael ! > > > > > > > > > - Ashwin > > > > > > > > > On Thu, Jan 11, 2024 at 3:55 PM Mickael Maison < > mickael.mai...@gmail.com > > > > > > wrote: > > > > > > > Hi Ashwin, > > > > > > > > +1 (binding), thanks for the KIP > > > > > > > > Mickael > > > > > > > > On Tue, Jan 9, 2024 at 4:54 PM Chris Egerton <chr...@aiven.io.invalid > > > > > > wrote: > > > > > > > > > > Thanks for the KIP! +1 (binding) > > > > > > > > > > On Mon, Jan 8, 2024 at 9:35 AM Ashwin <apan...@confluent.io.invalid > > > > > > wrote: > > > > > > > > > > > Hi All, > > > > > > > > > > > > I would like to start a vote on KIP-995. > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-995%3A+Allow+users+to+specify+initial+offsets+while+creating+connectors > > > > > > > > > > > > Discussion thread - > > > > > > https://lists.apache.org/thread/msorbr63scglf4484yq764v7klsj7c4j > > > > > > > > > > > > Thanks! > > > > > > > > > > > > Ashwin > > > > > > > > > > > > > > > >
