Skip to content

Commit

Permalink
Fix wrong time zone handling in telemetry
Browse files Browse the repository at this point in the history
  • Loading branch information
aptiko committed Aug 3, 2023
1 parent 7fab6c8 commit 86c46c0
Show file tree
Hide file tree
Showing 6 changed files with 702 additions and 68 deletions.
2 changes: 1 addition & 1 deletion doc/dev/telemetry.rst
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ scanning goes to :data:`enhydris.telemetry.drivers`.
format`_.

``enhydris_timeseries_end_date`` is either None (meaning get all
measurements since the beginning) or a datetime.
measurements since the beginning) or an aware datetime.

In order to avoid loading the server too much, this should not
return more than a reasonable number of records, such as half a
Expand Down
636 changes: 636 additions & 0 deletions enhydris/telemetry/migrations/0007_alter_telemetry_data_timezone.py

Large diffs are not rendered by default.

11 changes: 2 additions & 9 deletions enhydris/telemetry/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,8 @@ class Telemetry(models.Model):
max_length=35,
blank=True,
choices=timezone_choices,
verbose_name=_("Time zone of the timestamps (useful only for DST switches)"),
help_text=_(
"If the station switches to Daylight Saving Time, enter the time zone "
"here. This is only used in order to know when the DST switches occur. "
"The timestamp, after converting to winter time, is entered as is. If "
"the station does not switch to DST, leave this field empty."
),
verbose_name=_("Time zone of the timestamps"),
help_text=_('The time zone of the data, like "Europe/Athens" or "Etc/GMT".'),
)
fetch_interval_minutes = models.PositiveSmallIntegerField(
validators=[MinValueValidator(10), MaxValueValidator(1440)],
Expand Down Expand Up @@ -108,8 +103,6 @@ def _fetch_sensor(self, sensor):
timeseries_group_id=sensor.timeseries_group_id, type=Timeseries.INITIAL
)
timeseries_end_date = timeseries.end_date
if timeseries_end_date is not None:
timeseries_end_date = timeseries_end_date.replace(tzinfo=None)
measurements = self.api_client.get_measurements(
sensor_id=sensor.sensor_id, timeseries_end_date=timeseries_end_date
)
Expand Down
57 changes: 0 additions & 57 deletions enhydris/telemetry/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,63 +227,6 @@ def test_appends_data_to_database(self):
self.assertEqual(data.getvalue().strip(), "1990-01-01 00:00,42.0,")


class TelemetryFetchIgnoresTimeZoneTestCase(TelemetryFetchTestCaseBase):
"""Test that timeseries_end_date is always naive
When records already exist in the timeseries before calling fetch(), the
timeseries_end_date specified in get_measurement() is determined from the
latest of these records. However, storage of timestamps in the database is
always aware, so we need to convert it to naive before calling
get_measurement(). This TestCases checks that this is done.
Unfortunately, at the time of this writing, if this test fails, the error
message is not easy to understand, because fetch() catches all exceptions and
records them in the TelemetryLog. So while debugging things related to this
test it's a good idea to temporarily modify fetch() to raise exceptions.
"""

def setUp(self):
timeseries = Timeseries(
timeseries_group_id=self.timeseries_group.id, type=Timeseries.INITIAL
)
timeseries.save()
timeseries.append_data(
StringIO("2022-06-14 08:00,42.1,\n"), default_timezone="Etc/GMT-2"
)

@patch("enhydris.telemetry.types.meteoview2.requests.request")
def test_ignores_timezone(self, mock_request):
self._set_mock_request_return_values(mock_request)
self.telemetry.fetch()
self.assertEqual(
mock_request.call_args.kwargs,
{
"headers": {
"content-type": "application/json",
"Authorization": "Bearer topsecretapitoken",
},
"data": json.dumps(
{
"sensor": ["257"],
"datefrom": "2022-06-14",
"timefrom": "08:01",
"dateto": "2022-12-11",
}
),
},
)

def _set_mock_request_return_values(self, mock_request):
mock_request.side_effect = [
MagicMock( # Response for login
**{"json.return_value": {"code": "200", "token": "topsecretapitoken"}}
),
MagicMock( # Response for measurements
**{"json.return_value": "irrelevant"}
),
]


@patch("enhydris.telemetry.types.meteoview2.requests.request")
class TelemetryFetchDealsWithTooCloseTimestampsTestCase(TelemetryFetchTestCaseBase):
"""Test successive timestamps less than one minute apart
Expand Down
63 changes: 62 additions & 1 deletion enhydris/telemetry/tests/test_types/test_meteoview2.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import datetime as dt
import json
from io import StringIO
from unittest import TestCase
from unittest.mock import patch
from unittest.mock import MagicMock, patch

from freezegun import freeze_time
from requests import Timeout

from enhydris.models import Timeseries
from enhydris.telemetry import TelemetryError
from enhydris.telemetry.models import Telemetry
from enhydris.telemetry.tests.test_models import TelemetryFetchTestCaseBase
from enhydris.telemetry.types.meteoview2 import TelemetryAPIClient


Expand Down Expand Up @@ -143,6 +146,64 @@ def _set_successful_request_result(self, mock_request):
}


class TelemetryFetchIgnoresTimeZoneTestCase(TelemetryFetchTestCaseBase):
"""Test that timeseries_end_date is always naive
When records already exist in the timeseries before calling fetch(), the
timeseries_end_date specified in get_measurements() is determined from the
latest of these records. meteoview2 converts the timeseries_end_date to naive.
This TestCase checks that this is done.
Unfortunately, at the time of this writing, if this test fails, the error
message is not easy to understand, because fetch() catches all exceptions and
records them in the TelemetryLog. So while debugging things related to this
test it's a good idea to temporarily modify fetch() to raise exceptions.
"""

def setUp(self):
timeseries = Timeseries(
timeseries_group_id=self.timeseries_group.id,
type=Timeseries.INITIAL,
)
timeseries.save()
timeseries.append_data(
StringIO("2022-06-14 08:00,42.1,\n"), default_timezone="Etc/GMT"
)
self.telemetry.data_timezone = "Etc/GMT-2"

@patch("enhydris.telemetry.types.meteoview2.requests.request")
def test_ignores_timezone(self, mock_request):
self._set_mock_request_return_values(mock_request)
self.telemetry.fetch()
self.assertEqual(
mock_request.call_args.kwargs,
{
"headers": {
"content-type": "application/json",
"Authorization": "Bearer topsecretapitoken",
},
"data": json.dumps(
{
"sensor": ["257"],
"datefrom": "2022-06-14",
"timefrom": "10:01",
"dateto": "2022-12-11",
}
),
},
)

def _set_mock_request_return_values(self, mock_request):
mock_request.side_effect = [
MagicMock( # Response for login
**{"json.return_value": {"code": "200", "token": "topsecretapitoken"}}
),
MagicMock( # Response for measurements
**{"json.return_value": "irrelevant"}
),
]


@patch("enhydris.telemetry.types.meteoview2.requests.request")
@freeze_time("2022-06-14 08:35")
class GetMeasurementsTestCase(LoggedOnTestCaseBase):
Expand Down
1 change: 1 addition & 0 deletions enhydris/telemetry/types/meteoview2.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def get_measurements(self, sensor_id, timeseries_end_date):

def _get_start_date(self, sensor_id, timeseries_end_date):
if timeseries_end_date is not None:
timeseries_end_date = timeseries_end_date.replace(tzinfo=None)
start_date = timeseries_end_date + dt.timedelta(minutes=1)
else:
start_date = dt.datetime(1990, 1, 1)
Expand Down

0 comments on commit 86c46c0

Please sign in to comment.