Re: [DISCUSS] KIP-808: Add support for unix epoch precision in TimestampConverter SMT
Hi Tom, Thanks for your review. My original intent was to use the naming available in TimeUnit doc : https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/TimeUnit.html This led me to follow their logic : "toMicros", "toNanos", etc. By looking around the kafka code to see if there was some sort of consistency, ms/millis/milliseconds are present at multiple places but millis seems to be the least present wording. I've modified the KIP accordingly to use the full naming (seconds, milliseconds, ...) as per your suggestion. I've also added a note on precision loss but I'm not sure my phrasing is clear enough. Please let me know, Julien Le ven. 21 janv. 2022 à 13:20, Tom Bentley a écrit : > Hi Julien, > > Thanks! A couple of other points sprang to mind: > > 1. seconds is a unit, but millis etc are really just prefixes. I can > imagine what my old physics teacher would have to say about mixing these > concepts :-). I would have preferred to use the abbreviations s, ms, µs and > ns (ms in particular would be consistent with the unit names used in > configs), but I'm quite sure many people would struggle to know how to type > µ. Maybe 'us' is an acceptable alternative (it's a pretty common > convention), or perhaps we should use 'seconds', 'milliseconds', > 'microseconds' and 'nanoseconds' as the allowed values, even though they're > a bit long. > > 2. I think the truncation and sub-milliseconds behaviour when converting > to/from java.util.Date etc. should be part of the documentation of the > config. > > Apart from these minor points this is looking good to me. > > Kind regards, > > Tom > > On Wed, 19 Jan 2022 at 22:06, Julien Chanaud > wrote: > > > Hi Tom and thanks for your review, > > > > I agree and I have renamed the field name to unix.precision. > > I've replaced all references to the word "epoch" which I mistakenly used > to > > describe measurements throughout the KIP. > > I have also modified the KIP name as well. > > Before : Add support for unix epoch precision in TimestampConverter SMT > > Now : Add support for different unix precisions in TimestampConverter SMT > > > > Let me know what you think, > > > > Julien > > > > > > Le mer. 19 janv. 2022 à 19:27, Tom Bentley a > écrit : > > > > > Hi Julien, > > > > > > I wonder if the name epoch.precision is a little confusing. An epoch > is a > > > point in time chosen as a origin for a particular calendar system. As > > such > > > it doesn't have a precision. It's only measurements from this point in > > time > > > which have a precision. In the unix case, precisions of seconds, ms, µs > > and > > > ns seem to make the most sense. So I wonder if the name should be > > > unix.precision instead. That also makes it clearer that it only applies > > in > > > the type=unix case. Wdyt? > > > > > > Kind regards, > > > > > > Tom > > > > > > On Thu, 13 Jan 2022 at 13:42, Julien Chanaud > > > > wrote: > > > > > > > Hi Mickael, > > > > > > > > Thank you very much for your feedback and direction. > > > > > > > > I have added the documentation to the "Public Interfaces" chapter > > > > (formatted in a table as I've seen in other KIPs) and I'll put this > > > > KIP to a vote next week as per your suggestion. > > > > > > > > Regards, > > > > > > > > Julien > > > > > > > > Le mer. 12 janv. 2022 à 18:19, Mickael Maison > > > > a écrit : > > > > > > > > > > Hi Julien, > > > > > > > > > > Thanks for the KIP. I looks like a useful improvement to the > > > > > TimestampConverter SMT. > > > > > > > > > > I'd suggest adding the documentation for the new setting to the > KIP. > > > > > I've had to go check your PR to fully understand how you want to > use > > > > > it, both for input and output. > > > > > Apart from that, if you don't get any further feedback, feel free > to > > > > > start a vote. > > > > > > > > > > Thanks, > > > > > Mickael > > > > > > > > > > On Tue, Dec 21, 2021 at 2:19 PM Julien Chanaud < > > > chanaud.jul...@gmail.com> > > > > wrote: > > > > > > > > > > > > Hi everyone, > > > > > > > > > > > > Bumping this KIP discussion. > > > > > > It's a small change, entirely backward compatible and I'd love > your > > > > > > feedback on it. > > > > > > Thanks, > > > > > > Julien > > > > > > > > > > > > > > > > > > Le jeu. 9 déc. 2021 à 21:56, Julien Chanaud < > > > chanaud.jul...@gmail.com> > > > > a écrit : > > > > > > > > > > > > > > Hi everyone, > > > > > > > > > > > > > > I would like to start a discussion for KIP-808 > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-808%3A+Add+support+for+unix+epoch+precision+in+TimestampConverter+SMT > > > > > > > > > > > > > > This seems like a simple change but I suspect there are several > > > > things to consider, most notably regarding the java.util.Date object, > > > which > > > > is at the heart of the conversions. > > > > > > > > > > > > > > Let me know what you think. > > > > > > > > > > > > > > Julien > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSS] KIP-808: Add support for unix epoch precision in TimestampConverter SMT
Hi Julien, Thanks! A couple of other points sprang to mind: 1. seconds is a unit, but millis etc are really just prefixes. I can imagine what my old physics teacher would have to say about mixing these concepts :-). I would have preferred to use the abbreviations s, ms, µs and ns (ms in particular would be consistent with the unit names used in configs), but I'm quite sure many people would struggle to know how to type µ. Maybe 'us' is an acceptable alternative (it's a pretty common convention), or perhaps we should use 'seconds', 'milliseconds', 'microseconds' and 'nanoseconds' as the allowed values, even though they're a bit long. 2. I think the truncation and sub-milliseconds behaviour when converting to/from java.util.Date etc. should be part of the documentation of the config. Apart from these minor points this is looking good to me. Kind regards, Tom On Wed, 19 Jan 2022 at 22:06, Julien Chanaud wrote: > Hi Tom and thanks for your review, > > I agree and I have renamed the field name to unix.precision. > I've replaced all references to the word "epoch" which I mistakenly used to > describe measurements throughout the KIP. > I have also modified the KIP name as well. > Before : Add support for unix epoch precision in TimestampConverter SMT > Now : Add support for different unix precisions in TimestampConverter SMT > > Let me know what you think, > > Julien > > > Le mer. 19 janv. 2022 à 19:27, Tom Bentley a écrit : > > > Hi Julien, > > > > I wonder if the name epoch.precision is a little confusing. An epoch is a > > point in time chosen as a origin for a particular calendar system. As > such > > it doesn't have a precision. It's only measurements from this point in > time > > which have a precision. In the unix case, precisions of seconds, ms, µs > and > > ns seem to make the most sense. So I wonder if the name should be > > unix.precision instead. That also makes it clearer that it only applies > in > > the type=unix case. Wdyt? > > > > Kind regards, > > > > Tom > > > > On Thu, 13 Jan 2022 at 13:42, Julien Chanaud > > wrote: > > > > > Hi Mickael, > > > > > > Thank you very much for your feedback and direction. > > > > > > I have added the documentation to the "Public Interfaces" chapter > > > (formatted in a table as I've seen in other KIPs) and I'll put this > > > KIP to a vote next week as per your suggestion. > > > > > > Regards, > > > > > > Julien > > > > > > Le mer. 12 janv. 2022 à 18:19, Mickael Maison > > > a écrit : > > > > > > > > Hi Julien, > > > > > > > > Thanks for the KIP. I looks like a useful improvement to the > > > > TimestampConverter SMT. > > > > > > > > I'd suggest adding the documentation for the new setting to the KIP. > > > > I've had to go check your PR to fully understand how you want to use > > > > it, both for input and output. > > > > Apart from that, if you don't get any further feedback, feel free to > > > > start a vote. > > > > > > > > Thanks, > > > > Mickael > > > > > > > > On Tue, Dec 21, 2021 at 2:19 PM Julien Chanaud < > > chanaud.jul...@gmail.com> > > > wrote: > > > > > > > > > > Hi everyone, > > > > > > > > > > Bumping this KIP discussion. > > > > > It's a small change, entirely backward compatible and I'd love your > > > > > feedback on it. > > > > > Thanks, > > > > > Julien > > > > > > > > > > > > > > > Le jeu. 9 déc. 2021 à 21:56, Julien Chanaud < > > chanaud.jul...@gmail.com> > > > a écrit : > > > > > > > > > > > > Hi everyone, > > > > > > > > > > > > I would like to start a discussion for KIP-808 > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-808%3A+Add+support+for+unix+epoch+precision+in+TimestampConverter+SMT > > > > > > > > > > > > This seems like a simple change but I suspect there are several > > > things to consider, most notably regarding the java.util.Date object, > > which > > > is at the heart of the conversions. > > > > > > > > > > > > Let me know what you think. > > > > > > > > > > > > Julien > > > > > > > > > > > > > > >
Re: [DISCUSS] KIP-808: Add support for unix epoch precision in TimestampConverter SMT
Hi Tom and thanks for your review, I agree and I have renamed the field name to unix.precision. I've replaced all references to the word "epoch" which I mistakenly used to describe measurements throughout the KIP. I have also modified the KIP name as well. Before : Add support for unix epoch precision in TimestampConverter SMT Now : Add support for different unix precisions in TimestampConverter SMT Let me know what you think, Julien Le mer. 19 janv. 2022 à 19:27, Tom Bentley a écrit : > Hi Julien, > > I wonder if the name epoch.precision is a little confusing. An epoch is a > point in time chosen as a origin for a particular calendar system. As such > it doesn't have a precision. It's only measurements from this point in time > which have a precision. In the unix case, precisions of seconds, ms, µs and > ns seem to make the most sense. So I wonder if the name should be > unix.precision instead. That also makes it clearer that it only applies in > the type=unix case. Wdyt? > > Kind regards, > > Tom > > On Thu, 13 Jan 2022 at 13:42, Julien Chanaud > wrote: > > > Hi Mickael, > > > > Thank you very much for your feedback and direction. > > > > I have added the documentation to the "Public Interfaces" chapter > > (formatted in a table as I've seen in other KIPs) and I'll put this > > KIP to a vote next week as per your suggestion. > > > > Regards, > > > > Julien > > > > Le mer. 12 janv. 2022 à 18:19, Mickael Maison > > a écrit : > > > > > > Hi Julien, > > > > > > Thanks for the KIP. I looks like a useful improvement to the > > > TimestampConverter SMT. > > > > > > I'd suggest adding the documentation for the new setting to the KIP. > > > I've had to go check your PR to fully understand how you want to use > > > it, both for input and output. > > > Apart from that, if you don't get any further feedback, feel free to > > > start a vote. > > > > > > Thanks, > > > Mickael > > > > > > On Tue, Dec 21, 2021 at 2:19 PM Julien Chanaud < > chanaud.jul...@gmail.com> > > wrote: > > > > > > > > Hi everyone, > > > > > > > > Bumping this KIP discussion. > > > > It's a small change, entirely backward compatible and I'd love your > > > > feedback on it. > > > > Thanks, > > > > Julien > > > > > > > > > > > > Le jeu. 9 déc. 2021 à 21:56, Julien Chanaud < > chanaud.jul...@gmail.com> > > a écrit : > > > > > > > > > > Hi everyone, > > > > > > > > > > I would like to start a discussion for KIP-808 > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-808%3A+Add+support+for+unix+epoch+precision+in+TimestampConverter+SMT > > > > > > > > > > This seems like a simple change but I suspect there are several > > things to consider, most notably regarding the java.util.Date object, > which > > is at the heart of the conversions. > > > > > > > > > > Let me know what you think. > > > > > > > > > > Julien > > > > > > > > > >
Re: [DISCUSS] KIP-808: Add support for unix epoch precision in TimestampConverter SMT
Hi Julien, I wonder if the name epoch.precision is a little confusing. An epoch is a point in time chosen as a origin for a particular calendar system. As such it doesn't have a precision. It's only measurements from this point in time which have a precision. In the unix case, precisions of seconds, ms, µs and ns seem to make the most sense. So I wonder if the name should be unix.precision instead. That also makes it clearer that it only applies in the type=unix case. Wdyt? Kind regards, Tom On Thu, 13 Jan 2022 at 13:42, Julien Chanaud wrote: > Hi Mickael, > > Thank you very much for your feedback and direction. > > I have added the documentation to the "Public Interfaces" chapter > (formatted in a table as I've seen in other KIPs) and I'll put this > KIP to a vote next week as per your suggestion. > > Regards, > > Julien > > Le mer. 12 janv. 2022 à 18:19, Mickael Maison > a écrit : > > > > Hi Julien, > > > > Thanks for the KIP. I looks like a useful improvement to the > > TimestampConverter SMT. > > > > I'd suggest adding the documentation for the new setting to the KIP. > > I've had to go check your PR to fully understand how you want to use > > it, both for input and output. > > Apart from that, if you don't get any further feedback, feel free to > > start a vote. > > > > Thanks, > > Mickael > > > > On Tue, Dec 21, 2021 at 2:19 PM Julien Chanaud > wrote: > > > > > > Hi everyone, > > > > > > Bumping this KIP discussion. > > > It's a small change, entirely backward compatible and I'd love your > > > feedback on it. > > > Thanks, > > > Julien > > > > > > > > > Le jeu. 9 déc. 2021 à 21:56, Julien Chanaud > a écrit : > > > > > > > > Hi everyone, > > > > > > > > I would like to start a discussion for KIP-808 > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-808%3A+Add+support+for+unix+epoch+precision+in+TimestampConverter+SMT > > > > > > > > This seems like a simple change but I suspect there are several > things to consider, most notably regarding the java.util.Date object, which > is at the heart of the conversions. > > > > > > > > Let me know what you think. > > > > > > > > Julien > > > > > >
Re: [DISCUSS] KIP-808: Add support for unix epoch precision in TimestampConverter SMT
Hi Mickael, Thank you very much for your feedback and direction. I have added the documentation to the "Public Interfaces" chapter (formatted in a table as I've seen in other KIPs) and I'll put this KIP to a vote next week as per your suggestion. Regards, Julien Le mer. 12 janv. 2022 à 18:19, Mickael Maison a écrit : > > Hi Julien, > > Thanks for the KIP. I looks like a useful improvement to the > TimestampConverter SMT. > > I'd suggest adding the documentation for the new setting to the KIP. > I've had to go check your PR to fully understand how you want to use > it, both for input and output. > Apart from that, if you don't get any further feedback, feel free to > start a vote. > > Thanks, > Mickael > > On Tue, Dec 21, 2021 at 2:19 PM Julien Chanaud > wrote: > > > > Hi everyone, > > > > Bumping this KIP discussion. > > It's a small change, entirely backward compatible and I'd love your > > feedback on it. > > Thanks, > > Julien > > > > > > Le jeu. 9 déc. 2021 à 21:56, Julien Chanaud a > > écrit : > > > > > > Hi everyone, > > > > > > I would like to start a discussion for KIP-808 > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-808%3A+Add+support+for+unix+epoch+precision+in+TimestampConverter+SMT > > > > > > This seems like a simple change but I suspect there are several things to > > > consider, most notably regarding the java.util.Date object, which is at > > > the heart of the conversions. > > > > > > Let me know what you think. > > > > > > Julien > > >
Re: [DISCUSS] KIP-808: Add support for unix epoch precision in TimestampConverter SMT
Hi Julien, Thanks for the KIP. I looks like a useful improvement to the TimestampConverter SMT. I'd suggest adding the documentation for the new setting to the KIP. I've had to go check your PR to fully understand how you want to use it, both for input and output. Apart from that, if you don't get any further feedback, feel free to start a vote. Thanks, Mickael On Tue, Dec 21, 2021 at 2:19 PM Julien Chanaud wrote: > > Hi everyone, > > Bumping this KIP discussion. > It's a small change, entirely backward compatible and I'd love your > feedback on it. > Thanks, > Julien > > > Le jeu. 9 déc. 2021 à 21:56, Julien Chanaud a > écrit : > > > > Hi everyone, > > > > I would like to start a discussion for KIP-808 > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-808%3A+Add+support+for+unix+epoch+precision+in+TimestampConverter+SMT > > > > This seems like a simple change but I suspect there are several things to > > consider, most notably regarding the java.util.Date object, which is at the > > heart of the conversions. > > > > Let me know what you think. > > > > Julien > >
Re: [DISCUSS] KIP-808: Add support for unix epoch precision in TimestampConverter SMT
Hi everyone, Bumping this KIP discussion. It's a small change, entirely backward compatible and I'd love your feedback on it. Thanks, Julien Le jeu. 9 déc. 2021 à 21:56, Julien Chanaud a écrit : > > Hi everyone, > > I would like to start a discussion for KIP-808 > https://cwiki.apache.org/confluence/display/KAFKA/KIP-808%3A+Add+support+for+unix+epoch+precision+in+TimestampConverter+SMT > > This seems like a simple change but I suspect there are several things to > consider, most notably regarding the java.util.Date object, which is at the > heart of the conversions. > > Let me know what you think. > > Julien >
[DISCUSS] KIP-808: Add support for unix epoch precision in TimestampConverter SMT
Hi everyone, I would like to start a discussion for KIP-808 https://cwiki.apache.org/confluence/display/KAFKA/KIP-808%3A+Add+support+for+unix+epoch+precision+in+TimestampConverter+SMT This seems like a simple change but I suspect there are several things to consider, most notably regarding the java.util.Date object, which is at the heart of the conversions. Let me know what you think. Julien
