#22567 closed Bug (invalid)
FileSystemStorage.modified_time() is not timezone aware
Reported by: | James Aylett | Owned by: | nobody |
---|---|---|---|
Component: | File uploads/storage | Version: | 1.6 |
Severity: | Normal | Keywords: | |
Cc: | denis.cornehl@… | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I'm using the S3BotoStorage backend from django_storages, and collecstatic does not always upload new copies of static files. S3BotoStorage modified times come back in UTC (verified against Amazon's S3 console):
from django.contrib.staticfiles import finders, storage f = list(finders.get_finders()) pss = list(f[0].list([])) ps = pss[-1] from django.contrib.staticfiles.storage import staticfiles_storage as sf sf._setup() tlm = sf.modified_time(ps[0]) slm = ps[1].modified_time(ps[0]) print tlm print slm
gives:
2014-05-02 10:25:33 2014-05-02 09:57:57
However the local machine is in PDT (it's a Heroku dyno on an Amazon west coast instance somewhere):
>>> import os >>> os.system('ls -l static/css') total 8 -rw------- 1 u29977 29977 5838 2014-05-02 09:57 base.css 0 >>> os.system('date') Fri May 2 11:39:28 PDT 2014 0
So the correct modified time in UTC for the local static/css/base.css
is 2014-05-02 16:57:57, or several hours *after* the target file on S3 was last modified.
Because FileSystemStorage
isn't timezone aware, the comparison for "newer than target" in collectstatic
fails, and only updates that are more than seven hours (currently) after the previous successful update actually get deployed.
I'm pretty sure the actual problem is that datetime.datetime.fromtimestamp
converts to localtime but returns a naive object. I *believe* that forcing it into UTC by using …fromtimestamp(since_epoch, django.utils.timezone.utc)
would be appropriate (in the methods at the end of django/core/files/storage.py
), but timezones are fiddly things and I don't even have the brainpower right now to figure out how to write a simple test for this that can work no matter which underlying TZ the machine is configured for.
Change History (7)
follow-up: 3 comment:1 by , 11 years ago
comment:2 by , 11 years ago
Cc: | added |
---|
comment:3 by , 11 years ago
Replying to syphar:
Actually Heroku's dynos are set to UTC by default. You can change it by setting the environment, inside Django you see the timezone you define in your settings (even when you call
os.system
).
Ah, then that's it. Apologies for the confusion — Django's TIME_ZONE
is set to Pacific for this app, so that's where the discrepancy is coming from.
I just dug into it and I think django-storages doesn't even use the original modified-time of the file, it's just the upload-time in UTC (and it returns a naive datetime).
S3 probably doesn't have a concept of modification time (because you don't modify objects in S3, you replace them), so creation time is broadly equivalent. It's naive, which isn't great, but it's in UTC when Django's ones honour TZ. At the least I feel it should be made explicit what the "correct" (current) behaviour is in the [custom storage backend docs](https://docs.djangoproject.com/en/dev/howto/custom-file-storage/) — probably in the [storage backend API docs](https://docs.djangoproject.com/en/dev/ref/files/storage/), I'm guessing).
Timezone-awareness of the returned timestamps (change tz_info to the currently used timezone) would lead to collectstatic somehow having to compare an aware and naive timestamp (or breaking). Just expecting naive=UTC would be just wrong.
Yeah, that's a problem and I think answers your final question — it would appear to be a BC break to make them timezone-aware, so probably needs new API methods and a deprecation policy?
Even when
modified_time
would be timezone-aware,django-storages
would have to return an aware datetime too, or has to convert it to the current timezone.
I think this has convinced me that at least right now the more pressing issue is that django-storages' S3Boto
backend (and likely others) doesn't honour settings.TIME_ZONE
, and so this should be considered a bug against that and not Django.
comment:4 by , 11 years ago
There's [a bug filed against django-storages](https://bitbucket.org/david/django-storages/pull-request/83/converting-s3-last-modified-time-to-local) already, which references [a post to django-users](https://groups.google.com/forum/#!topic/django-users/pEIKxsHYN74) asking about how this all "should" work. The analysis there appears to be that the system timezone is in play, which doesn't appear to be the case.
comment:5 by , 11 years ago
And, erm, apologies for just remembering that this is trac, and doesn't use Markdown :-(
follow-up: 7 comment:6 by , 11 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
Thanks jaylett for figuring this out.
I think when one of the authors stated in the PR that this is a valid bug to django-storages we can see this ticket as closed.
The question why these datetimes are naive still is open and still valid, but is a general question.
comment:7 by , 11 years ago
Replying to syphar:
I think when one of the authors stated in the PR that this is a valid bug to django-storages we can see this ticket as closed.
+1
The question why these datetimes are naive still is open and still valid, but is a general question.
I think it'd be good to see current behaviour documented, still (since I can't see changing to TZ-aware being a fast process) and everyone has to integrate with FileSystemStorage
for the purposes of collectstatic
if nothing else.
Actually Heroku's dynos are set to UTC by default. You can change it by setting the environment, inside Django you see the timezone you define in your settings (even when you call
os.system
).I just dug into it and I think django-storages doesn't even use the original modified-time of the file, it's just the upload-time in UTC (and it returns a naive datetime).
Timezone-awareness of the returned timestamps (change tz_info to the currently used timezone) would lead to collectstatic somehow having to compare an aware and naive timestamp (or breaking). Just expecting naive=UTC would be just wrong. Even when
modified_time
would be timezone-aware,django-storages
would have to return an aware datetime too, or has to convert it to the current timezone.But this still raised the question why the returned modified-times aren't timezone aware. I'll try to figure out if there is a valid reason.