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

fix: Replace deprecated datetime.utcnow() with timezone-aware alterna… #652

Merged
merged 7 commits into from
Mar 25, 2025

Conversation

AnsahMohammad
Copy link
Contributor

Replaced instances of datetime.datetime.utcnow() with datetime.datetime.now(datetime.UTC) to address deprecation warnings while running pytest.

AnsahMohammad and others added 2 commits February 21, 2025 21:31
…tive

Replaced instances of datetime.datetime.utcnow() with datetime.datetime.now(datetime.UTC) to address deprecation warnings in Python 3.12+.

Signed-off-by: AnsahMohammad <[email protected]>
Copy link
Contributor

@jcristau jcristau left a comment

Choose a reason for hiding this comment

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

Thanks for the patch.

You'll need to use datetime.timezone.utc, as datetime.UTC was only added in python 3.11.

@AnsahMohammad
Copy link
Contributor Author

Thanks for the patch.

You'll need to use datetime.timezone.utc, as datetime.UTC was only added in python 3.11.

Just did that, please check now

@AnsahMohammad
Copy link
Contributor Author

@jcristau Hey, I noticed that the firefoxci-taskcluster / Decision Task failed due to some time-formatting issues. However, the tests didn't catch any errors. Would you suggest extending the test coverage to include this as well?

@jcristau
Copy link
Contributor

@jcristau Hey, I noticed that the firefoxci-taskcluster / Decision Task failed due to some time-formatting issues. However, the tests didn't catch any errors. Would you suggest extending the test coverage to include this as well?

Sounds like a good idea, yes :)

@AnsahMohammad
Copy link
Contributor Author

@jcristau Hey, I noticed that the firefoxci-taskcluster / Decision Task failed due to some time-formatting issues. However, the tests didn't catch any errors. Would you suggest extending the test coverage to include this as well?

Sounds like a good idea, yes :)

I went through the codebase and found this :

            except KeyError:
                # go on to the next index path
                pass

This made the test case miss the time-format issue on tests. I am not sure if it'll be a good idea to replace the "pass" statement here as any error might break the flow. I am confused on how to proceed.

@jcristau
Copy link
Contributor

jcristau commented Mar 3, 2025

@AnsahMohammad I've pushed a commit to your branch to add a test and fix the issue that was breaking the decision task.

@AnsahMohammad
Copy link
Contributor Author

@AnsahMohammad I've pushed a commit to your branch to add a test and fix the issue that was breaking the decision task.

Thanks for the commit 👍.
I thought it was another file causing the error.

@jcristau
Copy link
Contributor

jcristau commented Mar 4, 2025

Updated to pull in #659 and trigger the required complete-pr task.

@AnsahMohammad AnsahMohammad requested a review from jcristau March 4, 2025 17:18
@jcristau jcristau added the BREAKING CHANGE Backwards incompatible request that will require major version bump label Mar 4, 2025
@jcristau
Copy link
Contributor

jcristau commented Mar 4, 2025

Added the "breaking change" label because both json_time_from_now and current_json_time with datetime_format=True now return a timezone-aware object.

@AnsahMohammad AnsahMohammad deleted the fpr branch March 9, 2025 09:50
@AnsahMohammad AnsahMohammad restored the fpr branch March 9, 2025 09:50
@AnsahMohammad AnsahMohammad reopened this Mar 10, 2025
@ahal ahal merged commit 4d63571 into taskcluster:main Mar 25, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE Backwards incompatible request that will require major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants