[SIP-15] Making client time use UTC as the local time

[john-bodley]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

Whilst playing with the time range picker I discovered that for relative times ("Last Week" etc.) the reference time (either shown in default or via a tooltip) would use the client timezone rather than the server timezone. This was problematic say at 2019-10-24:18:00:00 PDT the UI would reference 2019-10-24 00:00:00 in various tooltips however when the query was executed it would use 2019-10-25 00:00:00.

Initially I was unsure whether we should parse the server timezone to the client but it seems in other places there's the assumption that times are expressed in UTC (irrespective of the server timezone).

Note the time range picker does not explicitly set the timezone by default for absolute dates, i.e., the query will actually run in reference to the timezone of the executing database engine. However one can specify the timezone via the ISO 8601 format. I’m not certain whether Superset should be more opinionated with regards to timezones.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

I added a tooltip to explicitly define where/how times are evaluated.

TEST PLAN

CI.

ADDITIONAL INFORMATION

• Has associated issue: [SIP-15] Transparent and Consistent Time Intervals #6360
• 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 @etr2460 @mistercrunch
cc: @vylc

[codecov-io]

Codecov Report

Merging #8450 into master will not change coverage.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##           master    #8450   +/-   ##
=======================================
  Coverage   66.58%   66.58%           
=======================================
  Files         448      448           
  Lines       22492    22492           
  Branches     2364     2364           
=======================================
  Hits        14976    14976           
  Misses       7378     7378           
  Partials      138      138

Impacted Files	Coverage Δ
superset/assets/src/explore/controls.jsx	41.26% <ø> (ø)	⬆️
.../explore/components/controls/DateFilterControl.jsx	37.9% <66.66%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eaeed0c...121502a. Read the comment docs.

[villebro]

While I agree UTC is the natural starting point for now, there will be many people relying on the current behaviour. So there should at least be a mention in UPDATING.md informing users of the change. An added bonus would be making this optional, i.e. being able to choose whether or not the reference point is local or UTC time, but that would require more backend -> frontend plumbing, so might cause unnecessary clutter in the codebase.

[john-bodley]

@villebro I agree regarding adding a section to UPDATING.md. The timezone logic is possibly more nuances especially if we’re talking about non-explicit timezone formats, i.e., not epoch which is in reference to UTC.

There are three different clocks:

1. The database engine
2. The Superset server
3. The Superset client

in addition to possible timezone encoding in the data. Unless ISO 8601 times are encoded with the timezone (which can be specified in the time range picker) the time logic can become quite complex especially if all three clocks are not using the same timezone.

Currently for the time range picker absolute times are specified using the client clock (or per this PR use UTC as the local time) without a designated timezone. Similarly relative times are evaluated on the Superset server using the server clock without a designated timezone. When the query is run the times are in reference to the local time of the database engine. This can get quite messy if the timezones differ as it’s not apparent (especially for relative times) what the reference point is.

[etr2460]

wording nit: I prefer The time range for the visualization. Times are in UTC. Gets the same point across but is much more succinct

[john-bodley]

@villebro I addressed your comments and updated the tooltip to explicitly mentioned where times were evaluated and how timezones were used.

[etr2460]

lgtm with one nit

[etr2460]

spelling nit: sans