[PATCH] Avoid timezone confusion
Daniel Axtens
dja at axtens.net
Mon Jan 15 11:52:33 AEDT 2018
vkabatov at redhat.com writes:
> From: Veronika Kabatova <vkabatov at redhat.com>
>
> Patchwork saves patches, comments etc with UTC timezone and reports
> this time when opening the patch details. However, internally generated
> processes such as events are reported with the instance's local time.
> There's nothing wrong with that and making PW timezone-aware would add
> useless complexity, but in a world-wide collaboration a lot of confusion
> may arise as the timezone is not reported at all. Instance's local time
> might be very different from the local time of CI integrating with PW,
> which is different from the local time of person dealing with it etc.
>
> For submission views, just add 'UTC' strings. For API responses, change
> the serializers to still use ISO 8601 format but with timezone
> information suffix [Z+-HHMM] - UTC for submissions and PW's local
> timezone for internal events.
Thanks for your patch! It looks very comprehensive - thanks especially
for including documentation!!
Would you prefer if we made internal events UTC? I think that might be
preferable, but I haven't thought about it in great detail just yet.
>
> Responses are returned as JSON. Blank fields are returned as ``null``, rather
> -than being omitted. Timestamps use the ISO 8601 format::
> +than being omitted. Returned timestamps use the ISO 8601 format including the
> +timezone information::
>
> - YYYY-MM-DDTHH:MM:SSZ
> + YYYY-MM-DDTHH:MM:SS[Z+-HH:MM]
> +
> +.. note::
> +
> + Timezone suffix is only provided in responses to avoid users' confusion and
> + to allow proper coordination with CI tooling. Underlying data are
> + timezone-naive and can't be filtered based on timezones, therefore requests
> + made to the API should not use the timezone suffix.
>
In particular, I wonder if using UTC everywhere would make things like
this simpler?
As I said I haven't put an enormous deal of thought into it, and I
haven't done a lot of work with TZ in python before so I don't know if
it would be crazy to implement. I just wanted your thoughts on it.
Regards,
Daniel
> Requests should use either query parameters or form-data, depending on the
> method. Further information is provided `below <rest_parameters>`__.
> diff --git a/docs/deployment/installation.rst b/docs/deployment/installation.rst
> index a570dd8..38e12d1 100644
> --- a/docs/deployment/installation.rst
> +++ b/docs/deployment/installation.rst
> @@ -173,7 +173,8 @@ this also:
> .. code-block:: shell
>
> $ sudo apt-get install -y python3-django python3-psycopg2 \
> - python3-djangorestframework python3-django-filters
> + python3-djangorestframework python3-django-filters \
> + python3-tz
>
> .. tip::
>
> diff --git a/patchwork/api/check.py b/patchwork/api/check.py
> index b37d6e0..4854a82 100644
> --- a/patchwork/api/check.py
> +++ b/patchwork/api/check.py
> @@ -17,6 +17,7 @@
> # along with Patchwork; if not, write to the Free Software
> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>
> +from django.utils import timezone
> from rest_framework.exceptions import PermissionDenied
> from rest_framework.generics import ListCreateAPIView
> from rest_framework.generics import RetrieveAPIView
> @@ -63,6 +64,7 @@ class CheckSerializer(HyperlinkedModelSerializer):
> return super(CheckSerializer, self).run_validation(data)
>
> def to_representation(self, instance):
> + instance.date = timezone.make_aware(instance.date)
> data = super(CheckSerializer, self).to_representation(instance)
> data['state'] = instance.get_state_display()
> return data
> diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
> index 1064504..2de4ea2 100644
> --- a/patchwork/api/cover.py
> +++ b/patchwork/api/cover.py
> @@ -19,6 +19,7 @@
>
> import email.parser
>
> +from pytz import UTC
> from rest_framework.generics import ListAPIView
> from rest_framework.generics import RetrieveAPIView
> from rest_framework.serializers import HyperlinkedModelSerializer
> @@ -37,6 +38,10 @@ class CoverLetterListSerializer(HyperlinkedModelSerializer):
> submitter = PersonSerializer(read_only=True)
> mbox = SerializerMethodField()
> series = SeriesSerializer(many=True, read_only=True)
> + date = SerializerMethodField()
> +
> + def get_date(self, instance):
> + return instance.date.replace(tzinfo=UTC)
>
> def get_mbox(self, instance):
> request = self.context.get('request')
> diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py
> index 7b5090a..bc915fe 100644
> --- a/patchwork/api/embedded.py
> +++ b/patchwork/api/embedded.py
> @@ -23,6 +23,8 @@ A collection of serializers. None of the serializers here should reference
> nested fields.
> """
>
> +from django.utils import timezone
> +from pytz import UTC
> from rest_framework.serializers import CharField
> from rest_framework.serializers import HyperlinkedModelSerializer
> from rest_framework.serializers import SerializerMethodField
> @@ -61,6 +63,7 @@ class CheckSerializer(HyperlinkedModelSerializer):
> url = CheckHyperlinkedIdentityField('api-check-detail')
>
> def to_representation(self, instance):
> + instance.date = timezone.make_aware(instance.date)
> data = super(CheckSerializer, self).to_representation(instance)
> data['state'] = instance.get_state_display()
> return data
> @@ -77,6 +80,10 @@ class CheckSerializer(HyperlinkedModelSerializer):
>
> class CoverLetterSerializer(MboxMixin, HyperlinkedModelSerializer):
>
> + def to_representation(self, instance):
> + instance.date = instance.date.replace(tzinfo=UTC)
> + return super(CoverLetterSerializer, self).to_representation(instance)
> +
> class Meta:
> model = models.CoverLetter
> fields = ('id', 'url', 'msgid', 'date', 'name', 'mbox')
> @@ -88,6 +95,10 @@ class CoverLetterSerializer(MboxMixin, HyperlinkedModelSerializer):
>
> class PatchSerializer(MboxMixin, HyperlinkedModelSerializer):
>
> + def to_representation(self, instance):
> + instance.date = instance.date.replace(tzinfo=UTC)
> + return super(PatchSerializer, self).to_representation(instance)
> +
> class Meta:
> model = models.Patch
> fields = ('id', 'url', 'msgid', 'date', 'name', 'mbox')
> @@ -126,6 +137,10 @@ class ProjectSerializer(HyperlinkedModelSerializer):
>
> class SeriesSerializer(MboxMixin, HyperlinkedModelSerializer):
>
> + def to_representation(self, instance):
> + instance.date = instance.date.replace(tzinfo=UTC)
> + return super(SeriesSerializer, self).to_representation(instance)
> +
> class Meta:
> model = models.Series
> fields = ('id', 'url', 'date', 'name', 'version', 'mbox')
> diff --git a/patchwork/api/event.py b/patchwork/api/event.py
> index 0d97af2..22c747e 100644
> --- a/patchwork/api/event.py
> +++ b/patchwork/api/event.py
> @@ -19,6 +19,7 @@
>
> from collections import OrderedDict
>
> +from django.utils import timezone
> from rest_framework.generics import ListAPIView
> from rest_framework.serializers import ModelSerializer
> from rest_framework.serializers import SerializerMethodField
> @@ -61,6 +62,7 @@ class EventSerializer(ModelSerializer):
> }
>
> def to_representation(self, instance):
> + instance.date = timezone.make_aware(instance.date)
> data = super(EventSerializer, self).to_representation(instance)
> payload = OrderedDict()
> kept_fields = self._category_map[instance.category] + [
> diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> index 115feff..57aa97d 100644
> --- a/patchwork/api/patch.py
> +++ b/patchwork/api/patch.py
> @@ -20,6 +20,7 @@
> import email.parser
>
> from django.utils.translation import ugettext_lazy as _
> +from pytz import UTC
> from rest_framework.generics import ListAPIView
> from rest_framework.generics import RetrieveUpdateAPIView
> from rest_framework.relations import RelatedField
> @@ -86,6 +87,10 @@ class PatchListSerializer(HyperlinkedModelSerializer):
> check = SerializerMethodField()
> checks = SerializerMethodField()
> tags = SerializerMethodField()
> + date = SerializerMethodField()
> +
> + def get_date(self, instance):
> + return instance.date.replace(tzinfo=UTC)
>
> def get_mbox(self, instance):
> request = self.context.get('request')
> diff --git a/patchwork/api/series.py b/patchwork/api/series.py
> index b5f4450..562bf45 100644
> --- a/patchwork/api/series.py
> +++ b/patchwork/api/series.py
> @@ -17,6 +17,8 @@
> # along with Patchwork; if not, write to the Free Software
> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>
> +
> +from pytz import UTC
> from rest_framework.generics import ListAPIView
> from rest_framework.generics import RetrieveAPIView
> from rest_framework.serializers import HyperlinkedModelSerializer
> @@ -38,6 +40,10 @@ class SeriesSerializer(HyperlinkedModelSerializer):
> mbox = SerializerMethodField()
> cover_letter = CoverLetterSerializer(read_only=True)
> patches = PatchSerializer(read_only=True, many=True)
> + date = SerializerMethodField()
> +
> + def get_date(self, instance):
> + return instance.date.replace(tzinfo=UTC)
>
> def get_mbox(self, instance):
> request = self.context.get('request')
> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
> index 6ed20c3..e817713 100644
> --- a/patchwork/templates/patchwork/submission.html
> +++ b/patchwork/templates/patchwork/submission.html
> @@ -255,7 +255,7 @@ function toggle_div(link_id, headers_id)
> <div class="comment">
> <div class="meta">
> <span>{{ submission.submitter|personify:project }}</span>
> - <span class="pull-right">{{ submission.date }}</span>
> + <span class="pull-right">{{ submission.date }} UTC</span>
> </div>
> <pre class="content">
> {{ submission|commentsyntax }}
> @@ -271,7 +271,7 @@ function toggle_div(link_id, headers_id)
> <div class="comment">
> <div class="meta">
> <span>{{ item.submitter|personify:project }}</span>
> - <span class="pull-right">{{ item.date }} | <a href="{% url 'comment-redirect' comment_id=item.id %}"
> + <span class="pull-right">{{ item.date }} UTC | <a href="{% url 'comment-redirect' comment_id=item.id %}"
> >#{{ forloop.counter }}</a></span>
> </div>
> <pre class="content">
> diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py
> index 93f6035..d59779d 100644
> --- a/patchwork/views/xmlrpc.py
> +++ b/patchwork/views/xmlrpc.py
> @@ -38,6 +38,7 @@ from django.views.decorators.csrf import csrf_exempt
> from django.utils import six
> from django.utils.six.moves import xmlrpc_client
> from django.utils.six.moves.xmlrpc_server import SimpleXMLRPCDispatcher
> +from pytz import UTC
>
> from patchwork.compat import reverse
> from patchwork.models import Check
> @@ -248,7 +249,7 @@ def patch_to_dict(obj):
>
> {
> 'id': 1
> - 'date': '2000-12-31 00:11:22',
> + 'date': '2000-12-31 00:11:22+00:00',
> 'filename': 'Fix-all-the-bugs.patch',
> 'msgid': '<BLU438-SMTP36690BBDD2CE71A7138B082511A at phx.gbl>',
> 'name': "Fix all the bugs",
> @@ -273,7 +274,7 @@ def patch_to_dict(obj):
> """
> return {
> 'id': obj.id,
> - 'date': six.text_type(obj.date).encode('utf-8'),
> + 'date': six.text_type(obj.date.replace(tzinfo=UTC)).encode('utf-8'),
> 'filename': obj.filename,
> 'msgid': obj.msgid,
> 'name': obj.name,
> @@ -319,7 +320,7 @@ def check_to_dict(obj):
> object which is OK to send to the client."""
> return {
> 'id': obj.id,
> - 'date': six.text_type(obj.date).encode('utf-8'),
> + 'date': six.text_type(obj.date.replace(tzinfo=UTC)).encode('utf-8'),
> 'patch': six.text_type(obj.patch).encode('utf-8'),
> 'patch_id': obj.patch_id,
> 'user': six.text_type(obj.user).encode('utf-8'),
> diff --git a/releasenotes/notes/report-timezones-47e2095efcb37a84.yaml b/releasenotes/notes/report-timezones-47e2095efcb37a84.yaml
> new file mode 100644
> index 0000000..ad1dae4
> --- /dev/null
> +++ b/releasenotes/notes/report-timezones-47e2095efcb37a84.yaml
> @@ -0,0 +1,4 @@
> +---
> +other:
> + - |
> + Report timezones in web UI and APIs.
> diff --git a/requirements-prod.txt b/requirements-prod.txt
> index dd2040d..1e5207e 100644
> --- a/requirements-prod.txt
> +++ b/requirements-prod.txt
> @@ -3,3 +3,4 @@ djangorestframework>=3.4,<3.7
> django-filter>=1.0,<1.1
> psycopg2>=2.7,<2.8
> sqlparse==0.2.3
> +pytz>=2016.10
> diff --git a/requirements-test.txt b/requirements-test.txt
> index 141cf66..632eb4c 100644
> --- a/requirements-test.txt
> +++ b/requirements-test.txt
> @@ -3,3 +3,4 @@ psycopg2>=2.7,<2.8
> django-debug-toolbar==1.8
> python-dateutil>2.0,<3.0
> selenium>=3.0,<4.0
> +pytz>=2016.10
> --
> 2.13.6
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list