Skip to content

refactor: replace datetime.now and datetime.today with timezone.now#4211

Merged
jennifer-richards merged 11 commits intoietf-tools:feat/tzawarefrom
painless-security:jennifer/timezone-now
Aug 25, 2022
Merged

refactor: replace datetime.now and datetime.today with timezone.now#4211
jennifer-richards merged 11 commits intoietf-tools:feat/tzawarefrom
painless-security:jennifer/timezone-now

Conversation

@jennifer-richards
Copy link
Copy Markdown
Member

Prepare for switching to USE_TZ=True by globally replacing datetime.datetime.now() with django.utils.timezone.now() through the project. Calls to datetime.datetime.today() have also been replaced. Despite the misleading name, this method is a synonym for datetime.datetime.now() when the latter is used without an argument.

Adds migrations to update model fields whose default value was set using datetime.now. Two old migrations used datetime.datetime.today() to find sessions in the future. After initially updating these to use timezone.now(), I reverted the change because I think it makes sense to leave these as they were. Migrating back to that point without setting USE_TZ=False is going to break timestamps anyway. If we really want to support that we could add a migration step to interrupt migrations and prompt the user to modify settings.USE_TZ at the appropriate time. I don't think that complexity is warranted, however.

datetime.datetime.today() is equivalent to datetime.datetime.now(); both
return a naive datetime with the current local time.
This is effectively the same, but is less likely to encourage accidental
use of naive datetimes.
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 12, 2022

Codecov Report

❗ No coverage uploaded for pull request base (feat/tzaware@a94a87f). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head a9a2220 differs from pull request most recent head 4db99a7. Consider uploading reports for the commit 4db99a7 to get more accurate results

@@               Coverage Diff               @@
##             feat/tzaware    #4211   +/-   ##
===============================================
  Coverage                ?   88.41%           
===============================================
  Files                   ?      294           
  Lines                   ?    39237           
  Branches                ?        0           
===============================================
  Hits                    ?    34693           
  Misses                  ?     4544           
  Partials                ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jennifer-richards jennifer-richards changed the base branch from main to feat/tzaware August 25, 2022 15:08
@jennifer-richards jennifer-richards marked this pull request as draft August 25, 2022 15:08
@jennifer-richards
Copy link
Copy Markdown
Member Author

Converting to draft while I check that it's still compatible with recent updates

@jennifer-richards jennifer-richards self-assigned this Aug 25, 2022
@jennifer-richards jennifer-richards marked this pull request as ready for review August 25, 2022 15:55
Copy link
Copy Markdown
Member

@rjsparks rjsparks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this fairly quickly - I don't think there's much opportunity for simple errors in what the PR aims for, and we'll have plenty of time to check for subtle as we move to the next phase of this project.

Comment thread ietf/utils/test_runner.py
# remember the value so ietf.utils.mail.send_smtp() will use the same
ietf.utils.mail.SMTP_ADDR['port'] = base + offset
self.smtpd_driver = SMTPTestServerDriver((ietf.utils.mail.SMTP_ADDR['ip4'],ietf.utils.mail.SMTP_ADDR['port']),None)
ietf.utils.mail.SMTP_ADDR['port'] = base + offset
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, ok, but it makes reviewing a really big PR even longer...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why these were changed - my editor is configured only to update whitespace on lines that changed in other ways.

@jennifer-richards jennifer-richards merged commit ebebdbe into ietf-tools:feat/tzaware Aug 25, 2022
@jennifer-richards jennifer-richards deleted the jennifer/timezone-now branch August 25, 2022 16:59
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants