-
Notifications
You must be signed in to change notification settings - Fork 182
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
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)
UTC fix
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 |
There was a problem hiding this 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.
ff5460d
into
Charcoal-SE:Merge-PR-8552-jeffschaller--fixdatetime.datetime.utcnow
…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
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 |
…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!
This PR changes all the
datetime.utcnow()
calls todatetime.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