Hi Bruno, Ha I see what you were talking about the extract method in UsePreviousTimeInvalidTimeStamp. Please ignore my last mail and I will update the KIP accordingly.
With Best Regards, Rabi Kumar K C Sent from Mail for Windows 10 From: Rabi Kumar K C Sent: Monday, October 7, 2019 4:50 PM To: dev@kafka.apache.org Subject: RE: [DISCUSS] KIP-530: Considerrenaming'UsePreviousTimeOnInvalidTimeStamp' classto'UsePartitionTimeOnInvalidTimeStamp' Hi Bruno, Thank You for your suggestions. I have made necessary changes in KIP and hopefully it’s fine now and if not then please do let me know. To answer your question 4) right now in trunk we can see that extract method is not present in UsePreviousTimeOnInvalidTimestamp instead it implements onInvalidTimestamp which is abstract method of super class ExtractRecordMetadataTimestamp. I have only seen extract() method in ExtractRecordMetadataTimestamp. Please do correct me if I am wrong. And yes I do agree with you on 5) the deprecation part for compatibility, deprecation and migration plan With Best Regards, Rabi Kumar K C Sent from Mail for Windows 10 From: Bruno Cadonna Sent: Monday, October 7, 2019 3:47 PM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-530: Consider renaming'UsePreviousTimeOnInvalidTimeStamp' class to'UsePartitionTimeOnInvalidTimeStamp' Hi Rabi, Thank you for the KIP! 1.) Could you please improve the formatting of the KIP? For instance, use appropriate formatting for code to differentiate it from the text. Also, we usually do not use italics to write KIPs. Look at other KIPs to get an idea of the formatting. 2.) "Public Interfaces" does not directly refer to interfaces in Java. It rather refers to the APIs that are visible from the outside. Thus, you should specify the class `UsePartitionOnInvalidTimeStamp` with its method signatures but without implementation. 3.) Under "Public Interfaces", you should also mention whether ` UsePreviousTimeOnInvalidTimestamp` should be deprecated or not. 4.) What do you mean with "now extract has been removed from 'UsePreviousTimeOnInvalidTimestamp'"? Without `extract()`, `UsePreviousTimeOnInvalidTimestamp` would not implement the `TimestampExtractor` interface anymore. 5.) Regarding "Compatibility, Deprecation, and Migration Plan", I do not think that we can simply remove `UsePreviousTimeOnInvalidTimestamp` in the next minor release. It needs to be deprecated beforehand. Best, Bruno On Wed, Oct 2, 2019 at 4:49 PM RABI K.C. <ravow...@gmail.com> wrote: > > Hello All, > > This is KIP for the change of Class name from > UsePreviousTimeOnInvalidTimeStamp to UsePartitionTimeOnInvalidTimeStamp. > Link and Jira ticket is mentioned below: > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=130028807 > https://issues.apache.org/jira/browse/KAFKA-8953 > > Would be pleased to get your feedback on this. > > With Best Regards, > Rabi Kumar K C
