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

Update datetime.utcnow() calls to datetime.now(tz=timezone.utc) to remove deprecation warning #8552

Conversation

jeffschaller
Copy link
Contributor

This PR changes all the datetime.utcnow() calls to datetime.now(tz=timezone.utc) to remove deprecation warnings that come up during the builds.

I tested with pytest, with Github's actions (e.g. https://github.com/jeffschaller/SmokeDetector/actions/runs/6764244470), and with a local SD instance; some indirect evidence of the instance is visible here: https://chat.stackexchange.com/transcript/message/64681032#64681032

Resolves #8510

replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
replace datetime.utcnow() with datetime.now(datetime.UTC)
@jeffschaller
Copy link
Contributor Author

From chat: two versions of the fix made it into the commit list, based on my simplistic usage of git. The initial one(s) change it to datetime.UTC (not good), followed by the eventual good fix of tz=timezone.utc

Copy link
Contributor

@makyen makyen left a comment

Choose a reason for hiding this comment

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

Thank you for creating this. I'm sorry for the long delay in my taking a look at it. Unfortunately, there is one additional change that's needed, which I've detailed in another comment I just posted.

For future commits, it would be beneficial to have the primary comment in the commit messages be a very brief summary of what the change was, rather than "Update ", as that's the primary thing that's seen when looking at changes and just "Update " really doesn't give good information. Don't worry about changing those now (I'll change them when merging), but having something more descriptive as the primary comment is quite helpful when looking at the current state of the code and looking back at commits or the list of commits.

helpers.py Show resolved Hide resolved
@makyen makyen changed the base branch from master to Merge-PR-8552-jeffschaller--fixdatetime.datetime.utcnow March 23, 2024 17:39
@makyen makyen merged commit ff5460d into Charcoal-SE:Merge-PR-8552-jeffschaller--fixdatetime.datetime.utcnow Mar 23, 2024
3 checks passed
makyen pushed a commit that referenced this pull request Mar 23, 2024
…zone.utc)`

All commits from PR#8552, plus a commit
changing the `log_str` in helpers.py as described in
#8552 (comment)
have been squashed into this commit in order to
provide a commit message describing the change.

autopull
@makyen
Copy link
Contributor

makyen commented Mar 23, 2024

In order to have a commit message that describes the change and not have the confusion of additional commits from merging the master branch into the development branch (sometimes merging in the master branch is necessary, but wasn't here), I squashed all the commits here, along with one more with the log_str change described here, into a single commit on the master branch. Basically, in this case, that provides a cleaner commit history, while preserving authorship. Normally, I prefer not to squash commits.

teward added a commit that referenced this pull request Mar 25, 2024
…th `datetime.now(tz=timezone.utc)`"""

This reverts commit 7de2948.

The changes done in the original merge request #8552 introduce hard breakages to SmokeDetector.

Introducing this change makes datetimes timezone-aware, and we also have datetimes coming via
websocket that are NOT tz-aware. This introduced a hard ValueError during SmokeDetector initialization.

This is a hard revert.  DO NOT revert this revert without checking with @teward first!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix DeprecationWarning: datetime.datetime.utcnow()
2 participants