Skip to content

Commit

Permalink
Merge pull request #442 from EverythingMe/fix/timezone
Browse files Browse the repository at this point in the history
Fix: when the server has non UTC timezone, timestamps were wrong
  • Loading branch information
arikfr committed Jun 7, 2015
2 parents 276ee7c + a60b168 commit c9da4be
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 17 deletions.
2 changes: 1 addition & 1 deletion redash/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ def outdated_queries(cls):
.switch(Query).join(DataSource)\
.where(cls.schedule != None)

now = datetime.datetime.utcnow().replace(tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=0, name=None))
now = utils.utcnow()
outdated_queries = {}
for query in queries:
if should_schedule_next(query.latest_query_data.retrieved_at, now, query.schedule):
Expand Down
5 changes: 2 additions & 3 deletions redash/tasks.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import time
import datetime
import logging
import redis
from celery import Task
from celery.result import AsyncResult
from celery.utils.log import get_task_logger
from redash import redis_connection, models, statsd_client, settings
from redash import redis_connection, models, statsd_client, settings, utils
from redash.utils import gen_query_hash
from redash.worker import celery
from redash.query_runner import get_query_runner
Expand Down Expand Up @@ -272,7 +271,7 @@ def execute_query(self, query, data_source_id, metadata):
redis_connection.delete(QueryTask._job_lock_id(query_hash, data_source.id))

if not error:
query_result = models.QueryResult.store_result(data_source.id, query_hash, query, data, run_time, datetime.datetime.utcnow())
query_result = models.QueryResult.store_result(data_source.id, query_hash, query, data, run_time, utils.utcnow())
else:
raise Exception(error)

Expand Down
9 changes: 9 additions & 0 deletions redash/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import re
import hashlib
import sqlparse
import pytz

COMMENTS_REGEX = re.compile("/\*.*?\*/")

Expand Down Expand Up @@ -62,6 +63,14 @@ def _find_dml_statements(self):
return False


def utcnow():
"""Return datetime.now value with timezone specified.
Without the timezone data, when the timestamp stored to the database it gets the current timezone of the server,
which leads to errors in calculations.
"""
return datetime.datetime.now(pytz.utc)

def slugify(s):
return re.sub('[^a-z0-9_\-]+', '-', s.lower())

Expand Down
5 changes: 2 additions & 3 deletions tests/factories.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import datetime
import redash.models
from redash.utils import gen_query_hash
from redash.utils import gen_query_hash, utcnow


class ModelFactory(object):
Expand Down Expand Up @@ -66,7 +65,7 @@ def __call__(self):
query_result_factory = ModelFactory(redash.models.QueryResult,
data='{"columns":{}, "rows":[]}',
runtime=1,
retrieved_at=datetime.datetime.utcnow,
retrieved_at=utcnow,
query="SELECT 1",
query_hash=gen_query_hash('SELECT 1'),
data_source=data_source_factory.create)
Expand Down
7 changes: 3 additions & 4 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
from tests import BaseTestCase
from redash import models
from factories import dashboard_factory, query_factory, data_source_factory, query_result_factory, user_factory, widget_factory
from redash.utils import gen_query_hash
from redash import query_runner
from redash.utils import gen_query_hash, utcnow


class DashboardTest(BaseTestCase):
Expand Down Expand Up @@ -141,7 +140,7 @@ def test_skips_fresh_queries(self):
self.assertNotIn(query, queries)

def test_outdated_queries_works_with_specific_time_schedule(self):
half_an_hour_ago = datetime.datetime.utcnow() - datetime.timedelta(minutes=30)
half_an_hour_ago = utcnow() - datetime.timedelta(minutes=30)
query = query_factory.create(schedule=half_an_hour_ago.strftime('%H:%M'))
query_result = query_result_factory.create(query=query, retrieved_at=half_an_hour_ago-datetime.timedelta(days=1))
query.latest_query_data = query_result
Expand Down Expand Up @@ -326,7 +325,7 @@ def setUp(self):
self.query = "SELECT 1"
self.query_hash = gen_query_hash(self.query)
self.runtime = 123
self.utcnow = datetime.datetime.utcnow()
self.utcnow = utcnow()
self.data = "data"

def test_stores_the_result(self):
Expand Down
13 changes: 7 additions & 6 deletions tests/test_refresh_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from mock import patch, call, ANY
from tests import BaseTestCase
from tests.factories import query_factory, query_result_factory
from redash.utils import utcnow
from redash.tasks import refresh_queries


Expand All @@ -11,7 +12,7 @@
class TestRefreshQueries(BaseTestCase):
def test_enqueues_outdated_queries(self):
query = query_factory.create(schedule="60")
retrieved_at = datetime.datetime.utcnow() - datetime.timedelta(minutes=10)
retrieved_at = utcnow() - datetime.timedelta(minutes=10)
query_result = query_result_factory.create(retrieved_at=retrieved_at, query=query.query,
query_hash=query.query_hash)
query.latest_query_data = query_result
Expand All @@ -23,7 +24,7 @@ def test_enqueues_outdated_queries(self):

def test_skips_fresh_queries(self):
query = query_factory.create(schedule="1200")
retrieved_at = datetime.datetime.utcnow() - datetime.timedelta(minutes=10)
retrieved_at = utcnow() - datetime.timedelta(minutes=10)
query_result = query_result_factory.create(retrieved_at=retrieved_at, query=query.query,
query_hash=query.query_hash)

Expand All @@ -33,7 +34,7 @@ def test_skips_fresh_queries(self):

def test_skips_queries_with_no_ttl(self):
query = query_factory.create(schedule=None)
retrieved_at = datetime.datetime.utcnow() - datetime.timedelta(minutes=10)
retrieved_at = utcnow() - datetime.timedelta(minutes=10)
query_result = query_result_factory.create(retrieved_at=retrieved_at, query=query.query,
query_hash=query.query_hash)

Expand All @@ -45,7 +46,7 @@ def test_enqueues_query_only_once(self):
query = query_factory.create(schedule="60")
query2 = query_factory.create(schedule="60", query=query.query, query_hash=query.query_hash,
data_source=query.data_source)
retrieved_at = datetime.datetime.utcnow() - datetime.timedelta(minutes=10)
retrieved_at = utcnow() - datetime.timedelta(minutes=10)
query_result = query_result_factory.create(retrieved_at=retrieved_at, query=query.query,
query_hash=query.query_hash)
query.latest_query_data = query_result
Expand All @@ -60,7 +61,7 @@ def test_enqueues_query_only_once(self):
def test_enqueues_query_with_correct_data_source(self):
query = query_factory.create(schedule="60")
query2 = query_factory.create(schedule="60", query=query.query, query_hash=query.query_hash)
retrieved_at = datetime.datetime.utcnow() - datetime.timedelta(minutes=10)
retrieved_at = utcnow() - datetime.timedelta(minutes=10)
query_result = query_result_factory.create(retrieved_at=retrieved_at, query=query.query,
query_hash=query.query_hash)
query.latest_query_data = query_result
Expand All @@ -79,7 +80,7 @@ def test_enqueues_only_for_relevant_data_source(self):
query = query_factory.create(schedule="60")
query2 = query_factory.create(schedule="3600", query=query.query, query_hash=query.query_hash)
import psycopg2
retrieved_at = datetime.datetime.utcnow().replace(tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=0, name=None)) - datetime.timedelta(minutes=10)
retrieved_at = utcnow().replace(tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=0, name=None)) - datetime.timedelta(minutes=10)
query_result = query_result_factory.create(retrieved_at=retrieved_at, query=query.query,
query_hash=query.query_hash)
query.latest_query_data = query_result
Expand Down

0 comments on commit c9da4be

Please sign in to comment.