john-bodley opened a new pull request #7702: [sqla] Enforcing ISO 8601 date/timestamp formats URL: https://github.com/apache/incubator-superset/pull/7702 ### CATEGORY Choose one - [ ] Bug Fix - [x] Enhancement (new features, refinement) - [ ] Refactor - [ ] Add tests - [ ] Build / Development Environment - [ ] Documentation ### SUMMARY Per [SIP-15A](https://github.com/apache/incubator-superset/issues/7656) this PR helps to ensure that the when specifying a string like temporal column the date/timestamp `strftime` format adheres to the ISO 8601 format. The TL;DR is if we're going to filter (and thus order) dates we need to use a representation where the lexicographical order coincides with the chronological order (see the ISO 8601 section in the SIP for details). This PR updates the CRUD and inline datasource editor (and adds validation where appropriate). Regrettable it seems in FAB we can't couple multiple fields, i.e., if the "Is temporal" field is unchecked then disable the "Datetime Format" field (@dpgaspar may know of how to do this). Additionally the messaging seems to be omitted if the form field is invalid (I was able to make this work for the default "Detail" tab (per [here](https://github.com/apache/incubator-superset/pull/5449)) but not for the "Columns" tab. I originally when down the rabbit hole of replacing the `strftime` formatting to using the more restrictive ISO 8601 constructs, i.e., `YYYY-MM-DD hh:mm:ss.SSSSSS` including providing a migration. I finally opted against this as the `datetime` object doesn't support formatting using the ISO 8601 tokens (packages like `arrow` and `pendulumn` do however) and it seems like we heavily depend of the `strftime` format for transforming Pandas dataframe. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF   ### TEST PLAN CI and tested the regex validator. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Changes UI - [ ] Requires DB Migration. - [ ] Confirm DB Migration upgrade and downgrade tested. - [ ] Introduces new feature or API - [ ] Removes existing feature or API ### REVIEWERS to: @betodealmeida @michellethomas @mistercrunch @villebro
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
