Skip to content
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

Change datetime objects to timezone-aware ones #909

Merged
merged 1 commit into from Jun 20, 2018

Conversation

lucassz
Copy link
Contributor

@lucassz lucassz commented Jun 18, 2018

Change naive datetime objects to timezone-aware ones. This has the advantage of fixing 200+ warnings when running the tests, and everything seems to work with this change.

@hdoupe
Copy link
Collaborator

hdoupe commented Jun 18, 2018

@lucassz This is extremely helpful. Thanks for knocking this out. Just so I understand, timezone.now() is the correct way to get the date when the time zone support is enabled and make_aware is a helper function for compatibility?

@lucassz
Copy link
Contributor Author

lucassz commented Jun 20, 2018

@hdoupe Right, yes. We just need to use make_aware in order to adapt the hardcoded times that are used as defaults (not sure whether those are absolutely needed either).

@hdoupe
Copy link
Collaborator

hdoupe commented Jun 20, 2018

Ok, so right now, TZ is set to UTC. It seems like the old times were set to some other timezone and time_aware converts them to UTC.

I'm reading the django timezone docs now. I'm having trouble figuring out which timezone we were using before when we had USE_TZ set to True, TIME_ZONE set to UTC, but were not using timezone aware datetimes. Do you have any ideas on this?

@lucassz
Copy link
Contributor Author

lucassz commented Jun 20, 2018

@hdoupe According to my understanding of the Django documentation, it would indeed default to the TIME_ZONE in settings.py, in this case UTC. So the behavior is not changing -- wrapping it in make_aware is just doing explicitly what Django was doing implicitly (with a warning) in a way that's deprecated but supported to preserve backward compatibility.

@hdoupe
Copy link
Collaborator

hdoupe commented Jun 20, 2018

Ok, that makes sense. Let's just leve the make_aware wrapper to be on the safe side. Is this ready to be merged?

@lucassz
Copy link
Contributor Author

lucassz commented Jun 20, 2018

@hdoupe Yes. Plus it gets rid of the warnings since we're doing it explicitly, so the Travis output will be cleaner.

@hdoupe
Copy link
Collaborator

hdoupe commented Jun 20, 2018

Very nice. Thanks for the contribution.

@hdoupe hdoupe merged commit b0a3485 into ospc-org:master Jun 20, 2018
@lucassz lucassz deleted the timezonefix branch June 21, 2018 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants