Skip to content

Upgrade SQLAlchemy v1 to v2 (Python 3.14 blocker)#3293

Merged
anth-volk merged 3 commits intomasterfrom
upgrade-sqlalchemy-v2
Mar 4, 2026
Merged

Upgrade SQLAlchemy v1 to v2 (Python 3.14 blocker)#3293
anth-volk merged 3 commits intomasterfrom
upgrade-sqlalchemy-v2

Conversation

@MaxGhenis
Copy link
Collaborator

@MaxGhenis MaxGhenis commented Feb 25, 2026

Summary

Ref: #2822 (comment)

Changes

  • setup.py: bump SQLAlchemy to v2
  • data.py: replace removed engine.execute() with connection-based exec_driver_sql() + _ResultProxy wrapper
  • *_service.py: LegacyRow -> Row
  • endpoints/policy.py: fix missing dataset param in set_user_policy SELECT query
  • 10 new SQLAlchemy v2 compatibility tests

Test plan

  • All 443 tests pass locally (330 unit + 103 to_refactor + 10 new)
  • CI passes with org secrets

SQLAlchemy v1 does not support Python 3.14, making this upgrade a blocker.

Key changes:
- Update setup.py pin from sqlalchemy>=1.4,<2 to sqlalchemy>=2,<3
- Replace removed engine.execute() with connection-based execution
  using conn.exec_driver_sql() inside a connection context manager
- Add _ResultProxy wrapper to eagerly fetch results so they survive
  connection closure (maintains existing fetchone()/fetchall() API)
- Replace LegacyRow (removed in v2) with Row in type annotations
- Update test mocks that used spec=LegacyRow to use plain MagicMock()
  since Row in v2 has a different interface
- Add 10 dedicated SQLAlchemy v2 compatibility tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Feb 25, 2026

Codecov Report

❌ Patch coverage is 84.61538% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.20%. Comparing base (1e18234) to head (f3a7d44).
⚠️ Report is 28 commits behind head on master.

Files with missing lines Patch % Lines
policyengine_api/data/data.py 89.28% 2 Missing and 1 partial ⚠️
policyengine_api/endpoints/policy.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3293      +/-   ##
==========================================
+ Coverage   70.10%   72.20%   +2.10%     
==========================================
  Files          56       56              
  Lines        2398     2425      +27     
  Branches      419      423       +4     
==========================================
+ Hits         1681     1751      +70     
+ Misses        641      595      -46     
- Partials       76       79       +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MaxGhenis MaxGhenis requested a review from anth-volk February 25, 2026 20:42
When dataset is truthy, the query builds "AND dataset = ?" (7
placeholders) but the params tuple only contained 6 values — the
dataset value was never appended. This caused a parameter binding
crash when saving a policy with a custom dataset.

Build the params list conditionally so dataset is included when present.

Fixes #3310

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@anth-volk anth-volk force-pushed the upgrade-sqlalchemy-v2 branch from a329b2d to e13743d Compare March 4, 2026 19:47
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@anth-volk anth-volk merged commit 76bc19b into master Mar 4, 2026
5 checks passed
@anth-volk anth-volk deleted the upgrade-sqlalchemy-v2 branch March 4, 2026 20:05
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.

Bug: set_user_policy crashes when dataset is not None (missing parameter)

2 participants