[GitHub] [nifi] exceptionfactory commented on pull request #4773: NIFI-8161 NiFi EL: migration from SimpleDateFormat to DateTimeFormatter
exceptionfactory commented on PR #4773: URL: https://github.com/apache/nifi/pull/4773#issuecomment-1506047689 It looks like it is not possible to reopen this PR following the rebase. If you can open a new PR, you can reference this one for background. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4773: NIFI-8161 NiFi EL: migration from SimpleDateFormat to DateTimeFormatter
exceptionfactory commented on PR #4773: URL: https://github.com/apache/nifi/pull/4773#issuecomment-1503885247 Thanks for revisiting this pull request @arkadius! Yes, with the main branch version now set to 2.0.0-SNAPSHOT, this is a good time to rebase the pull request and move forward with these changes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4773: NIFI-8161 NiFi EL: migration from SimpleDateFormat to DateTimeFormatter
exceptionfactory commented on pull request #4773: URL: https://github.com/apache/nifi/pull/4773#issuecomment-811032497 Thanks for the helpful reference @jfrazee! After reviewing the API documentation for `SimpleDateFormat` and `DateTimeFormatter` again in light of the Spark issue, it looks like another key difference is that `SimpleDateFormat` supports lowercase `z` as a General time zone reference, whereas `DateTimeFormatter` does not support it. Implementing a unit test using the lowercase `z` as part of the parsing pattern should present the differences between the implementations. Thanks for highlighting the issues you have uncovered @arkadius. It would be great to find a way forward, unfortunately I'm not sure about the timeline for a new major release, or the best approach for now. One option is providing a way to use this `DateTimeFormatter` approach as an optional feature. This would require reworking the PR to implement new Evaluators, as opposed to changing the existing ones. As far as how this could work, one approach might be a new `toDateTime()` function, which would avoid conflicting with the existing `toDate()` function. This would allow users to migrate to the new implementation without breaking existing flows. What do you think @jfrazee and @turcsanyip? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
