-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Add timezone information #19056
Conversation
|
||
To strive for data consistency (regardless of the timezone of the client) the Apache Superset backend tries to ensure that any timestamp sent to the client has an explicit (or semi-explicit as in the case with [Epoch time](https://en.wikipedia.org/wiki/Unix_time) which is always in reference to UTC) timezone encoded within. | ||
|
||
The challenge however lies with the slew of [database engines](/docs/databases/installing-database-drivers#install-database-drivers) which Apache Superset supports and various inconsistencies between their [Python Database API (DB API)](databases/installing-database-drivers#install-database-drivers) implementations combined with the fact that we use [Pandas](https://pandas.pydata.org/) to read SQL into a DataFrame prior to serializing to JSON. Regrettably Pandas ignores the DB API [type_code](https://www.python.org/dev/peps/pep-0249/#type-objects) relying by default on the underlying Python type returned by the DB API. Currently only a subset of the supported database engines work correctly with Pandas, i.e., ensuring timestamps without an explicit timestamp are serializd to JSON with the server timezone, thus guaranteeing the client will display timestamps in a consistent manner irrespective of the client's timezone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph and following example likely doesn't live here, i.e., it might best be suited for CONTRIBUTING.md
or similar. Additionally I'm really not sure what the best fix is. I'm concerned about adding yet more logic to Superset to special handle these nuances especially when the underlying issue resides with Pandas and/or the underlying DB APIs. Per trinodb/trino-python-client#160 it seems like Trino is working on this.
Codecov Report
@@ Coverage Diff @@
## master #19056 +/- ##
==========================================
- Coverage 69.09% 68.47% -0.63%
==========================================
Files 1909 1909
Lines 74918 74918
Branches 8297 8297
==========================================
- Hits 51766 51298 -468
- Misses 21010 21478 +468
Partials 2142 2142
Flags with carried forward coverage won't be shown. Click here to find out more. see 38 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all makes a lot of sense. I appreciate the info, and I'll bet others will too!!!
This seems to need a close/reopen to kick-start ye olde CI run. |
Ugh... CI is really unhappy with this PR. |
Gonna try close/open to restart CI - it's been a while, maybe something will change? |
Hm the failed checks will not re-run -- maybe they know nothing changed -- and their logs are too old to view. |
6d2641d
to
2738f11
Compare
Co-authored-by: Evan Rusackas <evan@preset.io>
Co-authored-by: Evan Rusackas <evan@preset.io>
SUMMARY
After debugging—with a heathy side of yak shaving—an issue with timestamps using the client timezone, I felt there was merit in at least documenting the topic of timezones (and the various contexts) which Superset likely doesn't do a great job with in terms of ensuring consistency.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI.
ADDITIONAL INFORMATION