Skip to content

Make SQA store SQLAlchemy 2.0-compatible#5201

Closed
LeoMoonStar wants to merge 1 commit into
facebook:mainfrom
LeoMoonStar:export-D104875017
Closed

Make SQA store SQLAlchemy 2.0-compatible#5201
LeoMoonStar wants to merge 1 commit into
facebook:mainfrom
LeoMoonStar:export-D104875017

Conversation

@LeoMoonStar
Copy link
Copy Markdown

@LeoMoonStar LeoMoonStar commented May 14, 2026

Summary:
The OSS Ax SQA store has a hard guard in with_db_settings_base.py that raises IncompatibleDependencyVersion when SQLAlchemy major > 1, disabling SQL storage entirely. This blocks SA 2.0 adoption (T163607006) and Python 3.13/3.14 (which auto-select SA 2.0.48 from third-party). Two additional SA 2.0 incompatibilities exist in OSS: defer("col_name") in load.py and reduced_state.py, which SA 2.0 rejects in favor of class-bound attribute references.

This diff removes the guard and converts the string-based loader options to attribute references. Adds a dual-version Buck test target tests_sa2 via constraint_overrides plus a self-proving TestSQLAlchemyDualVersionCompat class so each target proves its constraint took effect (EXPECTED_SA_MAJOR env var asserted at runtime).

Reviewed By: yangjoanna, andycylmeta

Differential Revision: D104875017

@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 14, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented May 14, 2026

@LeoMoonStar has exported this pull request. If you are a Meta employee, you can view the originating Diff in D104875017.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.38%. Comparing base (14d3475) to head (a8c0860).

Files with missing lines Patch % Lines
...rage/sqa_store/tests/test_with_db_settings_base.py 80.00% 3 Missing ⚠️
ax/storage/sqa_store/with_db_settings_base.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5201      +/-   ##
==========================================
- Coverage   96.38%   96.38%   -0.01%     
==========================================
  Files         617      617              
  Lines       69605    69615      +10     
==========================================
+ Hits        67090    67099       +9     
- Misses       2515     2516       +1     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Summary:
The OSS Ax SQA store has a hard guard in with_db_settings_base.py that raises IncompatibleDependencyVersion when SQLAlchemy major > 1, disabling SQL storage entirely. This blocks SA 2.0 adoption (T163607006) and Python 3.13/3.14 (which auto-select SA 2.0.48 from third-party). Two additional SA 2.0 incompatibilities exist in OSS: defer("col_name") in load.py and reduced_state.py, which SA 2.0 rejects in favor of class-bound attribute references.

This diff removes the guard and converts the string-based loader options to attribute references. Adds a dual-version Buck test target tests_sa2 via constraint_overrides plus a self-proving TestSQLAlchemyDualVersionCompat class so each target proves its constraint took effect (EXPECTED_SA_MAJOR env var asserted at runtime).

Reviewed By: yangjoanna, andycylmeta

Differential Revision: D104875017
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented May 18, 2026

This pull request has been merged in bd2ff97.

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

Labels

CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants