All timezones in UTC

[owocki]

There's a little bit of hackery in the repo in transforming UTC to MST (for example, subtraction of timedelta(hours=7) instead of true timezone transforms in order to get the correct local time in the UI. I'll button this up.

[rubik]

I wanted to work on this, but I think I'm missing something. In alert_fail_cases.py I see

# 7 hours thing is a hack for MST vs UTC timezone issues
is_trader_running = last_trade.created_on > (get_time() - datetime.timedelta(hours=int(7)) - datetime.timedelta(minutes=int(15)))

but get_time() is already MST, so why are we subtracting as if it were UTC?

[owocki]

Anytime you see datetime.timedelta(hours=int(7)) in the repo, it's a good chance you're looking at an attempted timezone mutation.

Honestly -- the code is a bit of a mess and I wouldn't be surprised if the logic for is_trader_running is completely borked. To your point, its possible that is_trader_running will return a True even if it's been 14 hours since the trader was last seen.

[rubik]

Ok, so I guess a somewhat big refactor is in order. The way I see it, is that the DB should only contain UTC datetimes, and this is how I've seen it done in the past (it's also what the Django docs suggest). The datetime is only converted for display.

The problem is that in model.py everything is saved in local time. The refactor would then include a migration. The logic like above would be much simpler because it wouldn't involve timezone conversions. Do you agree?

[owocki]

I'm not entirely sure. When I pull the data back from the database on the shell, it appears as UTC.

In [1]: from history.models import *

In [2]: ClassifierTest.objects.last()
Out[2]: <ClassifierTest: Nearest Neighbors on 2016-04-06 14:45:01.813674+00:00>

In [3]: ClassifierTest.objects.last().created_on
Out[3]: datetime.datetime(2016, 4, 6, 14, 45, 1, 813674, tzinfo=<UTC>)

To your point, the django UI shows the data in MST.

This is the same object as pulled from the shell above ^^

[rubik]

Tricky. I think that it could be due to the fact that Postgresql stores all the datetimes as UTC regardless (IIRC). But I'll have to check up on that.

[rubik]

Ok, I read a bit about this. From http://www.postgresql.org/docs/9.5/static/datatype-datetime.html#DATATYPE-TIMEZONES

All timezone-aware dates and times are stored internally in UTC. They are converted to local time in the zone specified by the TimeZone configuration parameter before being displayed to the client.

So if for some reason the connection set the tz to UTC then in the shell it's displayed as UTC.

Now, this makes it very easy to migrate the data, since it's already stored in UTC. We just have to alter the column types and change a couple things in the model. I question the usefulness of created_on_str. Why store it in the db when you can generate it at runtime for whatever timezone you want? I propose to remove it from the model.

[owocki]

So if for some reason the connection set the tz to UTC then in the shell it's displayed as UTC.

:+1:

I question the usefulness of created_on_str. Why store it in the db when you can generate it at runtime for whatever timezone you want? I propose to remove it from the model.

Yes, it's a real hack and an affront to DRY. The reason it was added is because django-chartit does not play well with GROUP BYs and the native DateTime model (or at least I could not get it to). See if you can get some of the code in the views ( ex: https://github.com/owocki/pytrader/blob/master/history/views.py#L192 ) to display in your native timezone using chartit. I could not.