-
Notifications
You must be signed in to change notification settings - Fork 8
Upgrade biblib to Python 3.12 #189
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| from biblib.manage import DeleteObsoleteVersionsNumber, DeleteStaleUsers, DeleteObsoleteVersionsTime | ||
| from biblib.models import User, Library, Permissions, Notes | ||
| from sqlalchemy.orm.exc import NoResultFound | ||
| from sqlalchemy import text | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this strictly input sanitization?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sanitization. It's a SQLAlchemy requirement for 1.4+. https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#execute-method-more-strict-execution-options-are-more-prominent |
||
| from biblib.tests.base import TestCaseDatabase | ||
| import sqlalchemy_continuum | ||
| import freezegun | ||
|
|
@@ -53,8 +54,8 @@ def test_delete_stale_users(self): | |
|
|
||
| with self.app.session_scope() as session: | ||
| # We do not add user 1 to the API database | ||
| session.execute('create table users (id integer, random integer);') | ||
| session.execute('insert into users (id, random) values (2, 7);') | ||
| session.execute(text('create table users (id integer, random integer);')) | ||
| session.execute(text('insert into users (id, random) values (2, 7);')) | ||
| session.commit() | ||
|
|
||
| with self.app.session_scope() as session: | ||
|
|
@@ -171,7 +172,7 @@ def test_delete_stale_users(self): | |
| raise | ||
| finally: | ||
| # Destroy the tables | ||
| session.execute('drop table users;') | ||
| session.execute(text('drop table users;')) | ||
| pass | ||
|
|
||
| def test_delete_obsolete_versions_number(self): | ||
|
|
@@ -184,8 +185,8 @@ def test_delete_obsolete_versions_number(self): | |
|
|
||
| with self.app.session_scope() as session: | ||
| # We do not add user 1 to the API database | ||
| session.execute('create table users (id integer, random integer);') | ||
| session.execute('insert into users (id, random) values (2, 7);') | ||
| session.execute(text('create table users (id integer, random integer);')) | ||
| session.execute(text('insert into users (id, random) values (2, 7);')) | ||
| session.commit() | ||
|
|
||
| with self.app.session_scope() as session: | ||
|
|
@@ -340,7 +341,7 @@ def test_delete_obsolete_versions_number(self): | |
| raise | ||
| finally: | ||
| # Destroy the tables | ||
| session.execute('drop table users;') | ||
| session.execute(text('drop table users;')) | ||
| pass | ||
|
|
||
| def test_delete_obsolete_versions_time(self): | ||
|
|
@@ -353,8 +354,8 @@ def test_delete_obsolete_versions_time(self): | |
|
|
||
| with self.app.session_scope() as session: | ||
| # We do not add user 1 to the API database | ||
| session.execute('create table users (id integer, random integer);') | ||
| session.execute('insert into users (id, random) values (2, 7);') | ||
| session.execute(text('create table users (id integer, random integer);')) | ||
| session.execute(text('insert into users (id, random) values (2, 7);')) | ||
| session.commit() | ||
|
|
||
| with self.app.session_scope() as session: | ||
|
|
@@ -535,7 +536,7 @@ def test_delete_obsolete_versions_time(self): | |
| raise | ||
| finally: | ||
| # Destroy the tables | ||
| session.execute('drop table users;') | ||
| session.execute(text('drop table users;')) | ||
| pass | ||
|
|
||
| if __name__ == '__main__': | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an interesting way to solve the relocated config. We can talk more about it at our next weekly and decide if this is how we want to do it going forward?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That bypassed my review. I just moved the config to the root so it can be found after the changes to adsmutils. |
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.
Did something change in sqlalchemy that now requires us to explicitly strip the 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.
The change is in Python.
datetime.utcnow()is deprecated and should be replaced withdatetime.now(timezone.utc), but the former is timezone-naive, and the latter is timezone-aware. I'm stripping the timezone so the new version is the same as the previous one.