django allows invalid TIME_ZONE in settings.py

[forest@…]

django didn't complain at all when my TIME_ZONE was "America/New York" instead of "America/New_York". It'd be nice if django at least printed a warning somewhere. It would have saved me some time tracking down the issue (which appeared to be a bug, since datetime.datetime.now() always returned the wrong time, but only in manage.py shell, not in a normal python shell).

-Forest

[Michael Radziej <mir@…>]

Can you propose how to check the TIME_ZONE for correctness?

[forest@…]

Well, the most obvious solution is to hard code a list of allowed values.

A more simple solution:

It seems that datetime.datetime.now() returns a UTC time when TIME_ZONE is set incorrectly (at least on my system). So, if the difference between datetime.datetime.now() and datetime.datetime.utcnow() is negligible, it's a safe bet that something is wrong with TIME_ZONE. I would just print a warning, not bring down the server, but a warning would be nice.

-Forest

[forest@…]

Regarding comparing datetime.datetime.now() with datetime.datetime.utcnow():

Obviously, if TIME_ZONE == 'UTC' or an equivalent, you wouldn't print the warning. It does vastly reduce the number of TIME_ZONE values you have to explicitly check for (and thereby reduces the maintenance overhead that comes with maintaining a list of valid TIME_ZONE values).

Might be easier to just maintain the list of valid values -- I'll leave that to the django folks.

-Forest

[Malcolm Tredinnick]

-1 on maintaining a list of values, because it would repeat information that is already known to the OS (and the list would be different for different operating systems).

Doing a sanity check against UTC for cases when we are not expecting to be in UTC is a reasonable idea. However, it may be fiddly because you can't raise errors for London and related time zones when they are not on summer time.

The "design decision" is not to keep the list, but if sanity checking is possible, then that seems reasonable. We may discover that sanity checking is really hard to get right, though.

[Winston Lee <lee.winston@…>]

I think an easier method would be to keep a list of Timezones that would return ('UTC', 'UTC') when time.tzname is called.

I'm thinking something like this in Settings.init

if hasattr(time, 'tzset'):
    # Move the time zone info into os.environ. See ticket #2315 for why
    # we don't do this unconditionally (breaks Windows).
    os.environ['TZ'] = self.TIME_ZONE
    time.tzset()
    if time.tzname == ('UTC', 'UTC') and not self.UTC_TZ.__contains__(self.TIME_ZONE):
        # time.tzname may have fallen back to UTC. 
        # If the TIME_ZONE is indeed a UTC TZ then it should be added to self.UTC_TZ tuple.
        warnings.warn("Your TIME_ZONE of %s may have been incorrectly set. UTC will be used.' % (self.TIME_ZONE))

Define in Settings class (or somewhere more appropriate)

self.UTC_TZ = ('UTC', 'ETC/UTC')

Note that I'm not running Django on this Mac so I can't check/test the code. I have no idea what would go into self.UTC_TZ either. :)

Test:

>>> os.environ['TZ'] = 'Australia/Melbourne'
>>> time.tzset()
>>> time.tzname == ('UTC', 'UTC') and not UTC_TZ.__contains__('Australia/Melbourne')
False
>>> os.environ['TZ'] = 'UTC'
>>> time.tzset()
>>> time.tzname == ('UTC', 'UTC') and not UTC_TZ.__contains__('UTC')                
False
>>> os.environ['TZ'] = 'abc'
>>> time.tzset()
>>> time.tzname == ('UTC', 'UTC') and not UTC_TZ.__contains__('abc')   
True

[tsal <tsal@…>]

Is the version really relative to this? I haven't tested this in SVN, but I will when I get a chance. If it's screwy in SVN, I'll see about turning Winston's suggestions into a patch of some sort.

[Malcolm Tredinnick]

Winston's idea is not a bad one. However, the implementation should be a bit more careful. Check the values before trying to change the timezone and then again afterwards. Basically, I would not want a warning emitted for whose machine was set to a London timezone (which isn't UTC, because UTC doesn't have daylight savings), or any equivalent locale.

Also, emitting a warning is not correct. If we cannot effectively change the timezone, it should be an error. The current behaviour is like emitting a warning that the aircraft is missing a wing and then letting the pilot try to take off anyway.

This patch will need a lot of testing on different platforms in different configurations, so it may take a while to get right, but Winston's approach is the best idea I've seen yet for trying to get this correct without having to maintain a massive whitelist.

[Leo Shklovskii]

Is including pytz as a library with Django completely out of the question?

[Malcolm Tredinnick]

Yes. It's way overkill to fix this patch.

[Malcolm Tredinnick]

s/patch/bug/

[Dennis Kaarsemaker]

Variant one: check time.tzname

[Dennis Kaarsemaker]

Variant 2: check /usr/share/zoneinfo

[Dennis Kaarsemaker]

Attached are two patched: variant one implements Winston Lee's idea, which is nice but cannot be used to check the value before mangling the environment. Patch two checks whether the timezone is available in /usr/share/zoneinfo. This can be done before mangling the environment. Both patches do not work on windows, where the whole environment mangling is skipped anyway.

I have no idea how to create unittests/doctests for this, as there is afaik no way to test settings handling in django's test framework (correct me if I'm wrong please, pointers to existing tests would be helpful too). I have tested both variants manually. The timezones 'UTC' and 'America/Chicago' are seen as valid, whereas 'America/Chicagoops' was rejected.

[Dennis Kaarsemaker]

Of course, since the patches don't overlap/conflict it is also possible to apply both.

[Dennis Kaarsemaker]

Setting stage back to accepted as it now has a patch.

[Adam Nelson]

Has a patch and should be ready for checkin.

[Malcolm Tredinnick]

(In [13722]) When possible, sanity check the timezone setting to see if the Unix-like
box it's running on supports that setting. If checking isn't possible,
we skip it. Patch from seveas. Thanks.

Fixed #3415.