Skip to content

Comments

tools/checkcerts.py: fix deprecated datetime.datetime.utcnow()#770

Draft
dmick wants to merge 1 commit intoceph:mainfrom
dmick:wip-checkcerts-fix-utcnow
Draft

tools/checkcerts.py: fix deprecated datetime.datetime.utcnow()#770
dmick wants to merge 1 commit intoceph:mainfrom
dmick:wip-checkcerts-fix-utcnow

Conversation

@dmick
Copy link
Member

@dmick dmick commented Feb 28, 2025

The replacement, datetime.datetime.now(datetime.UTC), returns a
tz-aware item, so the expiry time must also be timezone-aware.
datetime.strptime() does not handle that time format sanely.
I don't know why.

Luckily zoneinfo exists and can parse likely timezones into valid
datetime.tzinfo instances, as far as I can tell.  Why isn't this part
of datetime?  I don't know.  But this code seems be somewhat reliable
(assuming the timezone name is the last item in the date string,
delimited by spaces).

@dmick dmick requested a review from zmc February 28, 2025 03:16
The replacement, datetime.datetime.now(datetime.UTC), returns a
tz-aware item, so the expiry time must also be timezone-aware.
datetime.strptime() does not handle that time format sanely.
I don't know why.

Luckily zoneinfo exists and can parse likely timezones into valid
datetime.tzinfo instances, as far as I can tell.  Why isn't this part
of datetime?  I don't know.  But this code seems be somewhat reliable
(assuming the timezone name is the last item in the date string,
delimited by spaces).

Signed-off-by: Dan Mick <[email protected]>
@dmick dmick force-pushed the wip-checkcerts-fix-utcnow branch from a4a1a7e to e70f547 Compare February 28, 2025 18:51
@dmick
Copy link
Member Author

dmick commented Feb 28, 2025

replaced original change with a better idea using the new "zoneinfo". It will require Python 3.9, which is not currently on gw, so I'll find a better place to run this.

@dmick dmick marked this pull request as draft March 1, 2025 03:29
@dmick
Copy link
Member Author

dmick commented Mar 1, 2025

eh. this is required by and requires a pretty-recent python version (later even than 3.9). In retrospect I think I'll hold off on this until we decide if we even want to continue with checkcerts.py as we update our infra. Adam already has some grafana alerts set up for them and that may be a better plan, particularly if we can change the alerting scheme from the Slack-only verbose format it has now.

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.

1 participant