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

WIP: Migrate to SQLAlchemy 1.4 #473

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

WIP: Migrate to SQLAlchemy 1.4 #473

wants to merge 2 commits into from

Conversation

soininen
Copy link
Collaborator

@soininen soininen commented Jan 9, 2025

This PR migrates spinedb_api to SQLAlchemy 1.4.

  • DatabaseMapping now opens and closes a session only when needed. When using the low-level queries, the session must be opened manually using DatabaseMapping as a context manager.
  • We now use vanilla SQLAlchemy queries, our custom query module is gone.
  • DatabaseMapping's low-level subqueries now return SQLAlchemy's Result objects. The high-level functions work as previously.
  • Clients are now responsible for locking the DatabaseMapping if they are working in multithreaded environment.
  • Some migrations towards SQLAlchemy 2.0 are included but we still get a lot of warnings with the SQLALCHEMY_WARN_20=1 environment variable.

Resolves #121

Checklist before merging

  • Documentation (also in Toolbox repo) is up-to-date
  • Release notes have been updated
  • Unit tests have been added/updated accordingly
  • Code has been formatted by black & isort
  • Unit tests pass

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 91.93548% with 20 lines in your changes missing coverage. Please review.

Project coverage is 80.56%. Comparing base (8a5e084) to head (23bdb36).

Files with missing lines Patch % Lines
spinedb_api/spine_db_server.py 0.00% 9 Missing ⚠️
spinedb_api/db_mapping.py 91.56% 3 Missing and 4 partials ⚠️
...rsions/bba1e2ef5153_move_to_entity_based_design.py 75.00% 2 Missing ⚠️
spinedb_api/db_mapping_base.py 93.33% 1 Missing ⚠️
spinedb_api/export_mapping/export_mapping.py 98.41% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #473      +/-   ##
==========================================
+ Coverage   80.48%   80.56%   +0.07%     
==========================================
  Files          77       76       -1     
  Lines        9962     9884      -78     
  Branches     1477     1472       -5     
==========================================
- Hits         8018     7963      -55     
+ Misses       1602     1582      -20     
+ Partials      342      339       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@soininen soininen changed the title Migrate to SQLAlchemy 1.4 WIP: Migrate to SQLAlchemy 1.4 Jan 9, 2025
Migrated spinedb_api to use SQLAlchemy 1.4.
- DatabaseMapping must now always be used with the 'with' block.
  This is because only then we create the connection and session
  instances.
- We now use vanilla SQLAlchemy queries, our custom query module
  is gone.
- DatabaseMapping's subqueries now return SQLAlchemy Result objects
  which is a BREAKING change.
- We never really supported sending DatabaseMapping objects to
  multiprocessing.Process because DatabaseMapping is not picklable.
  This has been made more explicit now by removing a unit test
  that depended on the feature.
- Some migrations towards SQLAlchemy 2.0 are included but we still
  get a lot of warnings with the SQLALCHEMY_WARN_20=1
  environment variable.

Re #121
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.

Support sqlalchemy 1.4?
1 participant