-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Upgrade SQLAlchemy (SQLA) to 2.0 #59218
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
Conversation
17dec44 to
f3cc8f8
Compare
5ce5e2e to
084d78f
Compare
0a11b78 to
a410faf
Compare
7112da5 to
20def2f
Compare
Nataneljpwd
left a comment
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.
Looks great!
20def2f to
438246b
Compare
potiuk
left a comment
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.
LGTM - but I think at least @vincbeck review will be needed as well - possibly few other people who are more into SQLA.
Also - the DISCUSS thread is overwhelmingly "let's drop SQLA1" as expected, I propose @Dev-iL you start a LAZY CONSENSUS thread and link to this PR - this will drag attention of more maintainers.
Related: apache#59218, apache#59402 Passes mypy & pre-commit checks airflow-core/src/airflow/api_fastapi airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_dag_runs.py. airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_variables.py airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_xcoms.py
438246b to
0c069eb
Compare
|
Lazy consensus passed. I guess we're waiting for @vincbeck's blessing now... |
|
Interestingly, after the latest rebase, several deps get downgraded: < cadwyn==6.0.0
> cadwyn==5.4.6
< fastapi==0.128.0
> fastapi==0.117.1
< starlette==0.50.0
> starlette==0.48.0 |
I think @jscheffl wrote something about it in the cicd channel on slack, where the pipeline got dependency conflicts on Arm, maybe it can help him. Here is the slack message: |
6549cb9 to
1c84f22
Compare
jscheffl
left a comment
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.
No objection from @vincbeck then LGTM!
1c84f22 to
887dad6
Compare
|
For safety made a final rebase, let's have CI green and then merge ASAP! |
vincbeck
left a comment
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.
Awesome!
|
#protm |
* Remove SQLA 1 limit in Fab provider * Simplify the SQLA debug logging * Modify session start/end and release metadata locks * Address Nataneljpwd's comments * Address copilot's comments --------- Co-authored-by: vincbeck <vincbeck@amazon.com>
Related: apache#59218, apache#59402 Passes mypy & pre-commit checks airflow-core/src/airflow/api_fastapi airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_dag_runs.py. airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_variables.py airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_xcoms.py
* Remove SQLA 1 limit in Fab provider * Simplify the SQLA debug logging * Modify session start/end and release metadata locks * Address Nataneljpwd's comments * Address copilot's comments --------- Co-authored-by: vincbeck <vincbeck@amazon.com>
related: #28723, #58049
Fixing assorted issues preventing us from removing the SQLA1.4 limitation. Note that many fixes done as part of this effort were split into standalone PRs as can be seen in this PR's history below.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.