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

[master] refactor: issue #65604 #65605

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

abismor
Copy link

@abismor abismor commented Nov 23, 2023

What does this PR do?

replace deprecated datetime.utcnow() with datetime.now(tz=timezone.utc)

What issues does this PR fix or reference?

Fixes: #65604

Commits signed with GPG?

No

Self comment:

This is my first issue working on someone else's repo. Althou trivial I hope I've followed correct procedure.

@abismor abismor requested a review from a team as a code owner November 23, 2023 21:57
@abismor abismor requested review from MKLeb and removed request for a team November 23, 2023 21:57
Copy link

welcome bot commented Nov 23, 2023

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title refactor: issue #65604 [master] refactor: issue #65604 Nov 23, 2023
@lkubb
Copy link
Contributor

lkubb commented Nov 24, 2023

There's many more instances that should be replaced:

$ rg -l datetime.utcnow
setup.py
salt/state.py
salt/client/ssh/__init__.py
salt/modules/win_timezone.py
salt/modules/vsphere.py
salt/beacons/status.py
salt/modules/x509_v2.py
salt/modules/purefb.py
salt/modules/tls.py
salt/modules/x509.py
salt/modules/inspectlib/fsdb.py
salt/modules/system.py
salt/modules/purefa.py
salt/grains/core.py
salt/states/boto_iot.py
salt/states/boto_dynamodb.py
salt/states/x509_v2.py
salt/states/github.py
salt/cloud/clouds/ec2.py
salt/cloud/clouds/joyent.py
salt/utils/aws.py
salt/utils/versions.py
salt/utils/jid.py
salt/utils/x509.py
salt/utils/timeutil.py
salt/utils/event.py
salt/spm/pkgdb/sqlite3.py
tests/pytests/unit/utils/test_aws.py
tests/pytests/functional/modules/test_system.py
tests/unit/modules/test_x509.py
tests/unit/test_fileserver.py
tools/pkg/repo/create.py
tools/changelog.py
tools/utils/repo.py
tools/vm.py

…() with datetime.now(tz=timezone.utc)

Update imports to accomodate this change

Resolves issue saltstack#65604
…e.utcnow() with datetime.now(tz=timezone.utc)"

This reverts commit 92940db.
@abismor
Copy link
Author

abismor commented Nov 24, 2023

My apologies, I've realised flaw of my approach - hence the revert. I will update soon.

…() with datetime.now(tz=timezone.utc)

Update imports to accomodate this change

Resolves issue saltstack#65604
@abismor
Copy link
Author

abismor commented Nov 24, 2023

I'm confident this is now done correctly. Thank You for Your time to review my commit.

@dwoz
Copy link
Contributor

dwoz commented Dec 18, 2023

@azuro0092 Looks like there are conflicts again. Pre-commit and lint also failed.

@leeclemens
Copy link
Contributor

It seems a lot of the Pre-Commit failures are not related to the changes:

-                data = __salt__["status.{}".format(func)]()
+                data = __salt__[f"status.{func}"]()

Oddly enough, the style-guides that I've found (SaltStack and linked Google guides) show .format() is acceptable.

@@ -1374,7 +1374,7 @@ def expires(certificate, days=0):
"""
cert = x509util.load_cert(certificate)
# dates are encoded in UTC/GMT, they are returned as a naive datetime object
return cert.not_valid_after <= datetime.datetime.utcnow() + datetime.timedelta(
return cert.not_valid_after <= datetime.now(tz=timezone.utc) + timedelta(
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail because cryptography returns naive datetimes. They should be converted to UTC. Once there is a release with pyca/cryptography#9186, this should be updated to try to use the new endpoints, otherwise fallback to the old one with the mentioned fix.

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.

4 participants