-
Notifications
You must be signed in to change notification settings - Fork 11
Clarify the time zone in the chart (fixes DEV-133) #550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,19 +6,18 @@ | |
| from io import StringIO | ||
| from typing import Any | ||
| from wsgiref.util import FileWrapper | ||
| from zoneinfo import ZoneInfo | ||
|
|
||
| from django.db import IntegrityError | ||
| from django.db.models import Prefetch, QuerySet | ||
| from django.http import Http404, HttpResponse | ||
| from django.shortcuts import get_object_or_404 | ||
| from django.utils.timezone import is_aware | ||
| from rest_framework import status | ||
| from rest_framework.decorators import action | ||
| from rest_framework.request import Request | ||
| from rest_framework.response import Response | ||
| from rest_framework.viewsets import ModelViewSet, ReadOnlyModelViewSet | ||
|
|
||
| import iso8601 | ||
| import numpy as np | ||
| import pandas as pd | ||
| from htimeseries import HTimeseries | ||
|
|
@@ -282,9 +281,16 @@ def chart( | |
| ): | ||
| timeseries = get_object_or_404(models.Timeseries, pk=int(pk)) | ||
| self.check_object_permissions(request, timeseries) | ||
| serializer = serializers.TimeseriesRecordChartSerializer( | ||
| self._get_chart_data(request, timeseries), many=True | ||
| ) | ||
| try: | ||
| serializer = serializers.TimeseriesRecordChartSerializer( | ||
| self._get_chart_data(request, timeseries), many=True | ||
| ) | ||
| except ValueError as e: | ||
| return HttpResponse( | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
| content=str(e), | ||
| content_type="text/plain", | ||
| ) | ||
| return Response(serializer.data) # type: ignore | ||
|
|
||
| def _get_chart_data(self, request: Request, timeseries: models.Timeseries): | ||
|
|
@@ -336,17 +342,25 @@ def _get_stats_for_interval( | |
| } | ||
|
|
||
| def _get_date_bounds(self, request: Request, timeseries: models.Timeseries): | ||
| tz = ZoneInfo(timeseries.timeseries_group.gentity.display_timezone) | ||
| start_date = request.GET.get("start_date") or "" | ||
| end_date = request.GET.get("end_date") or "" | ||
| start_date = self._get_date_from_string(start_date, tz) # type: ignore | ||
| end_date = self._get_date_from_string(end_date, tz) # type: ignore | ||
| start_date = request.GET.get("start_date") or None | ||
| end_date = request.GET.get("end_date") or None | ||
| if start_date is not None: | ||
| start_date = self._get_date_from_string(start_date) # type: ignore | ||
| if end_date is not None: | ||
| end_date = self._get_date_from_string(end_date) # type: ignore | ||
| return start_date, end_date | ||
|
|
||
| def _get_data(self, request: Request, pk: int, format: str | None = None): | ||
| timeseries = self.get_object() | ||
| self.check_object_permissions(request, timeseries) | ||
| start_date, end_date = self._get_date_bounds(request, timeseries) | ||
| try: | ||
| start_date, end_date = self._get_date_bounds(request, timeseries) | ||
| except ValueError as e: | ||
| return HttpResponse( | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
| content=str(e), | ||
| content_type="text/plain", | ||
| ) | ||
| fmt_param = request.GET.get("fmt", "csv").lower() | ||
| timezone_param = request.GET.get("timezone", None) | ||
|
|
||
|
|
@@ -393,28 +407,25 @@ def _post_data(self, request: Request, pk: int, format: str | None = None): | |
| append_only=(mode == "append"), | ||
| ) | ||
| return HttpResponse(status=status.HTTP_204_NO_CONTENT) | ||
| except (IntegrityError, iso8601.ParseError, ValueError, KeyError) as e: | ||
| except (IntegrityError, ValueError, KeyError) as e: | ||
| return HttpResponse( | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
| content=str(e), | ||
| content_type="text/plain", | ||
| ) | ||
|
|
||
| def _get_date_from_string(self, adate: str, tz: dt.timezone): | ||
| date = self._parse_date(adate, tz) | ||
| if not date: | ||
| return None | ||
| def _get_date_from_string(self, adate: str): | ||
| date = self._parse_date(adate) | ||
| if not date or not is_aware(date): | ||
| raise ValueError(f"Invalid date: '{adate}' (must contain timezone)") | ||
| return self._bring_date_within_system_limits(date) | ||
|
|
||
| def _parse_date(self, adate: str, tz: dt.timezone): | ||
| try: | ||
| return iso8601.parse_date(adate, default_timezone=tz) | ||
| except iso8601.ParseError: | ||
| return None | ||
| def _parse_date(self, adate: str): | ||
| return dt.datetime.fromisoformat(adate) | ||
|
Comment on lines
+423
to
+424
|
||
|
|
||
| def _bring_date_within_system_limits(self, date: dt.datetime): | ||
| if date.isoformat() < "1680-01-01T00:00": | ||
| date = dt.datetime(1680, 1, 1, 0, 0, tzinfo=date.tzinfo) | ||
| if date.isoformat() > "2260-01-01T00:00": | ||
| date = dt.datetime(2260, 1, 1, 0, 0, tzinfo=date.tzinfo) | ||
| if date < dt.datetime(1680, 1, 1, 0, 0, tzinfo=dt.timezone.utc): | ||
| date = dt.datetime(1680, 1, 1, 0, 0, tzinfo=dt.timezone.utc) | ||
| if date > dt.datetime(2260, 1, 1, 0, 0, tzinfo=dt.timezone.utc): | ||
| date = dt.datetime(2260, 1, 1, 0, 0, tzinfo=dt.timezone.utc) | ||
| return date | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| Django>=5.2,<6 | ||
| djangorestframework>=3.9,<4 | ||
| iso8601 | ||
| pytz | ||
| rules>=2.0 | ||
| dj-rest-auth==2.1.7 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Z-suffixed timestamps in_parse_date_parse_datenow usesdatetime.fromisoformat, but this parser rejects ISO strings ending inZon Python 3.9/3.10 (which are still documented as supported indoc/general/install.rst). In this same commit, the chart frontend switched totoISOString(), which always sends...Z, so zoom/pan refetches (and any API client following the updated docs) will get a 400 on those supported runtimes instead of returning data.Useful? React with 👍 / 👎.