Force timestamp datetimes to be offset-naive or offset-aware

[miohtama]

Python has a troublematic history with timezones. You can have two kinds of datetime objects - with or without a timezone. You cannot compare between these twos. Your code will break funnily when two datetimes clashes.

I recommend that Feedly takes the approach where all timestamps are either offset-naive or offset-aware (e.g. datetime.datetime.now() vs. datetime.datetime.utcnow()) and validates this assumption for all inputs which can persist datetimes.

Below is an example when I entered activity.time as timezone-aware datetime. The activity is stored fine in the feed, but when you try to add another activity it breaks down.

  File "/Users/moo/code/foobar/venv/lib/python2.7/site-packages/feedly/feeds/aggregated_feed/base.py", line 83, in add_many
    current_activities, activities)
  File "/Users/moo/code/foobar/venv/lib/python2.7/site-packages/feedly/aggregators/base.py", line 81, in merge
    new_aggregated.append(activity)
  File "/Users/moo/code/foobar/venv/lib/python2.7/site-packages/feedly/activity.py", line 286, in append
    if self.updated_at is None or activity.time > self.updated_at:
TypeError: can't compare offset-naive and offset-aware datetimes

When update_at (Feedly internal?) is instated it does not have timezone information, thus leading to this error.

[intellisense]

You need to make your datetimes naive (taking care of the timezone) before assigning it to activity, as feedly internally use naive datetimes. Django has make_naive helper function for that.

from django.utils.timezone import make_naive
import pytz

nv_dt = make_naive(obj.timestamp, pytz.utc)

[miohtama]

Cool. I am doing this. However I suggest adding a validation to Activity init() or similar, so that timezone-aware datetimes cannot slip in (as they are persistent, causing issues later).

https://github.com/tschellenbach/Feedly/blob/master/feedly/activity.py#L59

If this is ok with the authors I can provide a patch.

[tbarbugli]

I agree, the way we handle datetime is too loose and can lead to bugs or worse (eg. read failures, write failures...). Any contribution is always very welcome so if you have already a patch for this feel free to create a pull request on github.

Tommaso

2014-03-19 12:25 GMT+01:00 Mikko Ohtamaa notifications@github.com:

Cool. I am doing this. However I suggest adding a validation to Activity init() or similar, so that timezone-aware datetimes cannot slip in (as they are persistent, causing issues later).

https://github.com/tschellenbach/Feedly/blob/master/feedly/activity.py#L59

If this is ok with the authors I can provide a patch.

Reply to this email directly or view it on GitHubhttps://github.com/tschellenbach/Feedly/issues/49#issuecomment-38040298 .