Added test for DateTimeField validation when server has timezone with… #4987
Conversation
… DST and input is a native time in a DST shift interval. Added pytz to requirements-testing.txt to reproduce the case.
… conversion in timezone with DST. Ref: encode#4986
rest_framework/fields.py
Outdated
| return timezone.make_aware(value, field_timezone) | ||
| try: | ||
| return timezone.make_aware(value, field_timezone) | ||
| except Exception: |
There was a problem hiding this comment.
Could we have a more specific exception here ?
There was a problem hiding this comment.
Here pytz raises AmbiguousTimeError or NonExistentTimeError both have parent pytz.exceptions.InvalidTimeError(Exception).
django-rest-framework doesn't have pytz in requirements (except -testing added in this PR) and I think it's not worth to depend on it here only to catch specific exceptions. You can have no pytz but still can use drf.
There was a problem hiding this comment.
What sort of exception would there be without pytz ?
If the issue is just the non requirement of pytz, we have a compat module that will deal with that.
There was a problem hiding this comment.
Without pytz I can't get timezone pytz.timezone('America/New_York') object, so if pytz is not installed I got: AttributeError: 'NoneType' object has no attribute 'timezone'.
Can you please point me on that compact module, I'll give it a try.
There was a problem hiding this comment.
compat module is in the rest_framework directory.
It should either import the pytz exceptions if available or mock them to avoid breaking code.
And I don't think you already have the AttributeError when pytz is not installed do you ?
rest_framework/fields.py
Outdated
| @@ -1085,6 +1085,7 @@ class DateTimeField(Field): | |||
| default_error_messages = { | |||
| 'invalid': _('Datetime has wrong format. Use one of these formats instead: {format}.'), | |||
| 'date': _('Expected a datetime but got a date.'), | |||
| 'make_aware': _('Datetime can not be represented in timezone "{timezone}".') | |||
There was a problem hiding this comment.
What about 'Invalid datetime for the timezone "{timezone}".' ?
Mock pytz.timezone localize in tests. Ref: encode#4986
|
Hope this time I get it right, compat module is neat |
|
Nice! |
… DST and input is a native time in a DST shift interval.
Added pytz to requirements-testing.txt to reproduce the case.
Note: Before submitting this pull request, please review our contributing guidelines.
Description
PR reproduces an issue with DateTimeField in case when server TIME_ZONE has Day Light Saving Time (DST) shift and field given native datetime in DST shift.
In my case server has
pytzinstalled, I don't know how to reproduce it without it.Corresponding issue #4986