Save timezone #4386

dannyroberts · 2014-09-24T18:08:09Z

I'm opening this for preliminary review. (Let me know if you think there's a better workflow than opening a PR that we don't intend to merge. I like to do this sometimes to get feedback on a direction I'm going when I'm about half-way.)

Might want to scan the diff first and then review commit by commit.

Here's my progress so far:

Make jsonobject support replacing default handlers Type configs jsonobject#97
Write an UTCDateTimeProperty that accepts any valid ISO date/datetime and translates it to UTC, and saves up to microseconds. (Lenient input, strict output.)
Make sure UTCDateTimeProperty saves timezone information (see commits in Save timezone utc date time #4499)
Make XFormInstance use the UTCDateTimeProperty exclusively (both for explicit properties and dynamic properties)
Mark XFormInstances processed the new way with a is_timezone_aware flag set to true
Make CommCareCases use the new property exclusively
Write script to reprocess form xml (using the changes feed or a view that iterates through all forms)
Write a script that reprocesses all cases with affected forms (might as well be all cases). Verify that this has the expected behavior of updating the case actions to include the timezone-modified dates
...make sure all reports take the timezone change into account

czue · 2014-09-25T13:36:50Z

looks good so far to me!

snopoke · 2014-09-30T11:35:51Z

looks good, just wondering why you're introducing ios8601 dependency? Can't you get the same functionality from date_util and pytz?

millerdev · 2014-09-30T12:17:28Z

corehq/ext/jsonobject.py

+
+    def wrap(self, obj):
+        dt = iso8601.parse_date(obj)
+        return dt.astimezone(iso8601.iso8601.UTC).replace(tzinfo=None)


Would it be very hard to make this preserve the original time zone so it is not lost on wrap/unwrap?

I don't think it'd be hard, just not sure if it's desired. We rely pretty heavily on the fact that these datetime strings sort correctly with a simple lexical sort; if you have the timezone info there the way it was originally, they won't sort correctly anymore.

There are other schemes we could invent to solve this, like

YYYY-MM-DDThh:mm:ss.uuuuuuZ+05:30

where Z means it's a UTC time and "+05:30" means that it was originally given in India time. Baking a made up datetime format into this property feels weird to me, so I opted for just switching to UTC and calling a day, as the smallest change that fixes our problem.

I'm actually very willing to be convinced of a better way that's also feasible in the short term, or be convinced that one of the options I scrapped is actually better than what I'm doing.

I handn't thought about lexical sorting, and that's a very good point.

I guess a question we should answer first is do we ever need the original timezone? Would it make some parts of the app more usable to display in the original timezone? Or are we always wanting to use- and able to determine the local timezone of the user who is viewing the data?

As data scientist I think it is ideal not to discard data, even if we don't immediately have a use for it. But sometimes practicality beats purity too. Other than needing a custom parsing function, what are the problems with the custom format you proposed above?

dannyroberts · 2014-09-30T13:31:27Z

@snopoke I'm not sure and you might be correct, but I was looking for something that narrowly accepts ISO8601 date/datetime strings and nothing else, and I believe date_util fights to the death to try to accept anything that looks remotely like a date or datetime. Does that make sense?

snopoke · 2014-09-30T14:10:58Z

Yea that makes sense. Does this need to be normalized at all (see not in
introduction here: http://pytz.sourceforge.net/)?

dannyroberts · 2014-09-30T18:03:25Z

@snopoke I'm not sure how that problem would bite us. As long as the phone time is set correctly, we're always given timezone offset ("+05:30") not a location ("India time"), so daylight savings etc should be baked into that timezone offset already and not something we need to worry about.

Does that sound right to you, or am I missing something?

snopoke · 2014-10-01T06:49:20Z

True, good point.

dimagimon · 2014-10-22T18:01:38Z

corehq/ex-submodules/casexml/apps/case/tests/test_v2_parsing.py

@@ -127,7 +127,7 @@ def testParseWithIndices(self):
        # quick test for ota restore
        v2response = phone_views.xml_for_case(HttpRequest(), case.get_id, version="2.0")
        expected_v2_response = """
-        <case case_id="foo-case-id" date_modified="2011-12-07T13:42:50Z" user_id="bar-user-id" xmlns="http://commcarehq.org/case/transaction/v2">
+        <case case_id="foo-case-id" date_modified="2011-12-07T18:42:50Z" user_id="bar-user-id" xmlns="http://commcarehq.org/case/transaction/v2">


E501 line too long (145 > 115 characters)

to keep track of whether an XFormInstance has been migrated to process timezones correctly

after timezone changes

and transforms them to utc datetimes with time zeroed out

…tetime (we use timezone-unaware datetimes throughout to mean UTC)

needs more tests

for succinctly preforming many simple input/output matching tests

while otherwise acting like a tz-naive datetime that is implicitly in UTC also rename ISO8601Property to UTCDateTimeProperty

- split UTCDateTime into separate corehq.ext.datetime module - minor cleanup to UTCDateTime - give option to pass in tz string instead of timedelta for UTCDateTime original_offset - change UTCDateTime repr to use tz string notation for succinctness

for converting back to a normal timezone-aware datetime

when no timzone is given datetimes are still treated as UTC, but the original_offset is given as None and the timezone hint (+00:00) is excluded from the output format

make certain tests use exact_equals for comparison instead also improve corpus test docstrings slightly

exposed an issue with ISODocument

and add tests fixes commtrack tests

snopoke · 2014-10-30T08:47:58Z

corehq/ext/datetime.py

+
+    @staticmethod
+    def _validate_tz_offset(hours, minutes, seconds):
+        if not (0 <= hours <= 15):


comparison does not match error message: 0 <= hours <= 15 != between 0 and 14 (inclusive)

snopoke · 2014-10-30T09:06:21Z

Test failure real

dannyroberts added Open for review: do not merge A work in progress awaiting QA QA in progress. Do not merge labels Sep 24, 2014

millerdev reviewed Sep 30, 2014
View reviewed changes

dannyroberts mentioned this pull request Oct 8, 2014

Save timezone utc date time #4499

Closed

2 tasks

dimagimon reviewed Oct 22, 2014
View reviewed changes

dannyroberts force-pushed the save-timezone branch from c32dd5b to d511f10 Compare October 22, 2014 18:42

dannyroberts added 18 commits October 29, 2014 10:46

make XFormInstance object's datetimes respect timezones

c298a91

add is_timezone_aware property to XFormInstance

414eddf

to keep track of whether an XFormInstance has been migrated to process timezones correctly

fix receiverwrapper tests

7c36a03

after timezone changes

move couchforms jsonobject extensions into corehq.ext

6547a8e

add test confirming ISO8601Property handles dates

d3e3529

and transforms them to utc datetimes with time zeroed out

fix test by making sure get_received_on returns a timezone-unaware da…

08f069b

…tetime (we use timezone-unaware datetimes throughout to mean UTC)

fix pact tests to respect timezone info

e63d860

cleanup that can go into master

d1e44b7

preliminary use of timezones in CommCareCase

37a4851

needs more tests

add Corpus & CorpusMeta to ext.unittest

65dc147

for succinctly preforming many simple input/output matching tests

add UTCDateTime that preserves original offset

4dc37d4

while otherwise acting like a tz-naive datetime that is implicitly in UTC also rename ISO8601Property to UTCDateTimeProperty

add UTCDateTime.to_datetime function

4741849

for converting back to a normal timezone-aware datetime

use pytz instead of iso8601 where possible

a4814e7

add docstrings UTCDateTime and UTCDateTimeProperty

4939cc7

document Corpus helpers

9584fc6

add UTCDateTime tests for < > <= >=

2447557

fix and test bug in ISOMeta

c9f3fbc

dannyroberts added 7 commits October 29, 2014 11:04

carefully exclude timezone hint in UTCDateTimeProperty

a3eb14a

when no timzone is given datetimes are still treated as UTC, but the original_offset is given as None and the timezone hint (+00:00) is excluded from the output format

remove Z from UTCDateTimeProperty output for naive datetimes

a6228ba

use __name__ instead of func_name

14e150e

make UTCDateTime.__eq__ follow datetime standard

d17b1f1

make certain tests use exact_equals for comparison instead also improve corpus test docstrings slightly

clean up corehq.ext.jsonobject round trip tests slightly

88f68b1

fix couchforms tests, given new UTCDateTimeProperty

52f84da

fix phone tests

5742dbb

dannyroberts force-pushed the save-timezone branch from d511f10 to 5742dbb Compare October 29, 2014 15:15

dannyroberts added 3 commits October 29, 2014 11:57

fix pact tests

2f9948f

exposed an issue with ISODocument

add UTCDateTime.replace, copy, and deepcopy

8bfd10f

and add tests fixes commtrack tests

fix receiverwrapper tests

9757780

snopoke reviewed Oct 30, 2014
View reviewed changes

dannyroberts closed this Apr 13, 2015

proteusvacuum deleted the save-timezone branch June 28, 2016 21:44

snopoke restored the save-timezone branch March 19, 2017 00:06

dannyroberts deleted the save-timezone branch March 29, 2019 20:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Save timezone #4386

Save timezone #4386

dannyroberts commented Sep 24, 2014

czue commented Sep 25, 2014

snopoke commented Sep 30, 2014

millerdev Sep 30, 2014

dannyroberts Sep 30, 2014

millerdev Sep 30, 2014

dannyroberts commented Sep 30, 2014

snopoke commented Sep 30, 2014

dannyroberts commented Sep 30, 2014

snopoke commented Oct 1, 2014

dimagimon Oct 22, 2014

snopoke Oct 30, 2014

snopoke commented Oct 30, 2014

Save timezone #4386

Save timezone #4386

Conversation

dannyroberts commented Sep 24, 2014

czue commented Sep 25, 2014

snopoke commented Sep 30, 2014

millerdev Sep 30, 2014

Choose a reason for hiding this comment

dannyroberts Sep 30, 2014

Choose a reason for hiding this comment

millerdev Sep 30, 2014

Choose a reason for hiding this comment

dannyroberts commented Sep 30, 2014

snopoke commented Sep 30, 2014

dannyroberts commented Sep 30, 2014

snopoke commented Oct 1, 2014

dimagimon Oct 22, 2014

Choose a reason for hiding this comment

snopoke Oct 30, 2014

Choose a reason for hiding this comment

snopoke commented Oct 30, 2014