remove dependency on pytz #8984
Conversation
|
OK. Maybe we could remove pytz dependencies but keep the error handling code without having pytz imports , by catching the upper-level exception |
|
@max-muoto what do you think |
I'll take a look at this PR. Seems probable to me that it should work. |
rest_framework/fields.py
Outdated
| self.fail('make_aware', timezone=field_timezone) | ||
| return dt | ||
| except InvalidTimeError: | ||
| dt = timezone.make_aware(value, field_timezone) |
There was a problem hiding this comment.
I think we should move this back to a try catch, and check that the exception's name using type(e).__name__, confirming it's equal to "InvalidTimeError". This way, we can still be specific, while removing the pytz dependency.
There was a problem hiding this comment.
hum, actually it only uses derived classes of InvalidTimeError. Should we iterate over the inheritance chain (__mro__) ?
There was a problem hiding this comment.
I came out with something I find pretty good. The things I am annoyed with is: since we support both presence and absence of pytz in tests, the CI should test both and I doubt it does (and I yet don't see how without rerunning the test suite in a virtualenv with a different install)
This reverts commit 393609d.
| try: | ||
| import pytz | ||
| except ImportError: | ||
| pytz = None |
There was a problem hiding this comment.
I added this, so it's possible the @pytest.mark.skipif(pytz is None) on line 1598 never worked
|
thanks a lot for handling this! |
No description provided.