[PATCH] Avoid timezone confusion
Veronika Kabatova
vkabatov at redhat.com
Tue Jan 16 01:26:58 AEDT 2018
Hi!
----- Original Message -----
> From: "Daniel Axtens" <dja at axtens.net>
> To: vkabatov at redhat.com, patchwork at lists.ozlabs.org
> Sent: Monday, January 15, 2018 1:52:33 AM
> Subject: Re: [PATCH] Avoid timezone confusion
>
> 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.
>
That would work too. Then we could only document "hey, all times are
in UTC" and things would be definitely more consistent and easier to
work with.
> >
> > 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.
>
Hm, let me think about it... We can set the default TIME_ZONE setting to
UTC and either
a) document that if people override it they will get inconsistent times
and it's their problem to deal with, or
b) accept if people want to override it and use that timezone when parsing
email submissions instead of the UTC, but then we would still need to
report used timezone somehow because noone except instance admin would
know what the timezone actually is (which is exactly what we want to
avoid)
option a) is IMHO cleaner and really easy to implement too. I can send an
updated patch if you agree with it.
Thanks,
Veronika
> 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