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

Remove pin on SQLAlchemy version < 2.0 #45

Merged
merged 5 commits into from Jan 4, 2024
Merged

Conversation

jace
Copy link
Contributor

@jace jace commented Nov 6, 2023

WTForms-SQLAlchemy with SQLAlchemy 2.0 is known to be working.

@jace jace mentioned this pull request Nov 6, 2023
@azmeuk
Copy link
Member

azmeuk commented Nov 18, 2023

CI is failing. Under normal circumstances, there is at least one SQLAlchemy 2 deprecation warning raised when running the tests.

tests/test_main.py::QuerySelectFieldTest::test_with_query_factory
  /home/eloi/dev/wtforms/wtforms-sqlalchemy/tests/test_main.py:77: RemovedIn20Warning: Deprecated API features detected! These feature(s) are not compatible with SQLAlchemy 2.0. To prevent incompatible upgrades prior to updating applications, ensure requirements files are pinned to "sqlalchemy<2.0". Set environment variable SQLALCHEMY_WARN_20=1 to show all deprecation warnings.  Set environment variable SQLALCHEMY_SILENCE_UBER_WARNING=1 to silence this message. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    mapper(Test, test_table)

@jace
Copy link
Contributor Author

jace commented Nov 18, 2023

I'll review and push a fix tonight.

@jace
Copy link
Contributor Author

jace commented Nov 18, 2023

SQLAlchemy 1.4 is now the minimum version as the location of declarative_base moved from sqlalchemy.ext.declarative to sqlalchemy.orm in 1.4 and was removed from the old location in 2.0.

Since SQLAlchemy has recommended use of inspect since 0.8, I've switched to it in wtforms_sqlalchemy.orm.

mapper.iterate_properties does not return relationships unless mappers have been fully initialized, which appears to be delayed in 2.0. However, accessing mapper.attrs causes initialization, so I've switched from mapper.iterate_properties to mapper.attrs.values.

There is a LegacyAPIWarning emitted due to the use of Query.get(), which is deprecated in 2.0 but not removed. Switching to the recommended select construct is not taken up here.

@jace
Copy link
Contributor Author

jace commented Nov 24, 2023

Sorry for the merge commits. This will need a squash. Should I force-push one?

Also, does this need additional tests? I had to manually swap SQLAlchemy versions between test runs as it's not in the tox config. It may be worth adding (I'm not familiar with tox).

@azmeuk
Copy link
Member

azmeuk commented Nov 27, 2023

No need to force push, there is no conflict so this will be fine.
Now the only thing needed is that I find a bit of time to spend on reviewing this.

@azmeuk azmeuk merged commit 83115b6 into wtforms:master Jan 4, 2024
8 checks passed
@azmeuk
Copy link
Member

azmeuk commented Jan 4, 2024

Thank you!
edit: I just released 0.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants