-
-
Notifications
You must be signed in to change notification settings - Fork 272
Replace use of utcnow() with now() for Python 3.13 compatibility, Fixes #1643 #1644
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
Conversation
It seems that the tests are mocking something that needs updating |
Ah ok, I’ll take a look. I didn’t find a CONTRIBUTING file so this initial change was a bit of a shot in the dark 🤓 |
no problem, it's what the ci is for! |
c0372b1
to
1ddb3ca
Compare
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.
OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 1ddb3ca of this pull request into gerrit so we can run tests and reviews and stuff
New Gerrit review created for change 1ddb3ca: https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5892 |
@@ -629,8 +629,7 @@ def _generate_create_date(self) -> datetime.datetime: | |||
"Can't locate timezone: %s" % self.timezone | |||
) from None | |||
create_date = ( | |||
datetime.datetime.utcnow() | |||
.replace(tzinfo=datetime.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.
the test fails without this replace. i dont remember the reason for the replace, and it perhaps is local to the fact that the test is using mocks. not sure yet
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.
sorry I'm just going to restore that replace, I know if I had more mental capacity right now I could change the mock on the other end but I just need to get this merged and I dont want to screw it up right now
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.
Maybe I'm being dumb, but doesn't datetime.datetime.now(tzinfo)
achieve the desired result (the current datetime in the prescribed timezone)?
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.
I can't get the mock in the tests to match it. i dont have time to understand why the mocks were set up the way they were and dont want to change them
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.
diff --git a/alembic/script/base.py b/alembic/script/base.py
index 972a9bd..552dc05 100644
--- a/alembic/script/base.py
+++ b/alembic/script/base.py
@@ -605,7 +605,6 @@ class ScriptDirectory:
create_date = (
datetime.datetime.now(tz=datetime.timezone.utc)
- .replace(tzinfo=datetime.timezone.utc)
.astimezone(tzinfo)
)
else:
diff --git a/tests/test_script_production.py b/tests/test_script_production.py
index 4263c1e..1049357 100644
--- a/tests/test_script_production.py
+++ b/tests/test_script_production.py
@@ -235,7 +235,13 @@ class ScriptNamingTest(TestBase):
with mock.patch(
"alembic.script.base.datetime",
mock.Mock(
- datetime=mock.Mock(now=lambda tz=None: given),
+ datetime=mock.Mock(
+ now=lambda tz=None: (
+ given.replace(tzinfo=datetime.timezone.utc)
+ if timezone_arg
+ else given
+ )
+ ),
timezone=datetime.timezone,
),
):
also note no tests covered this, all of them mocked utcnow(). adding one now |
Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5892 has been merged. Congratulations! :) |
Description
Fixes: #1643
Existing tests should cover this change.
Checklist
This pull request is:
must include a complete example of the issue. one line code fixes without an
issue and demonstration will not be accepted.
Fixes: #<issue number>
in the commit messageinclude a complete example of how the feature would look.
Fixes: #<issue number>
in the commit message