-
Notifications
You must be signed in to change notification settings - Fork 16.6k
Add CI support for SQLAlchemy 2.0#52233
Conversation
closes: #48953, #52663
related: #28723, #50960 (prerequisite that's included in this PR)
- Add the
check_upgrade_sqlalchemycontainer entrypoint for testing SQLA v2 on CI - Fix some existing shellcheck violations reported by shellcheck-py.
- Robustify multiple unit tests and amend some business logic.
^ 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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.
96d2e8d to
482fff4
Compare
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.
Pull Request Overview
This PR introduces a new entrypoint function, check_upgrade_sqlalchemy, to handle upgrading SQLAlchemy for testing SQLA v2 while also addressing several shellcheck violations.
- Adds check_upgrade_sqlalchemy implementations to both scripts/docker/entrypoint_ci.sh and Dockerfile.ci.
- Updates Breeze parameters, commands, and CI workflow to include an upgrade_sqlalchemy flag.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/docker/entrypoint_ci.sh | Adds check_upgrade_sqlalchemy function with minor quoting adjustments. |
| scripts/ci/docker-compose/devcontainer.env | Introduces UPGRADE_SQLALCHEMY environment variable. |
| dev/breeze/src/airflow_breeze/params/shell_params.py | Adds new boolean parameter upgrade_sqlalchemy. |
| dev/breeze/src/airflow_breeze/commands/developer_commands.py | Adds upgrade_sqlalchemy option to developer commands. |
| dev/breeze/src/airflow_breeze/commands/common_options.py | Adds a new click option for upgrade_sqlalchemy with help text. |
| Dockerfile.ci | Adds check_upgrade_sqlalchemy function similar to the entrypoint script. |
| .github/workflows/run-unit-tests.yml | Adds upgrade-sqlalchemy input to the CI workflow. |
Comments suppressed due to low confidence (1)
dev/breeze/src/airflow_breeze/commands/common_options.py:157
- [nitpick] The help text for the upgrade_sqlalchemy option mentions an 'experimental supported version'; consider revising it to clearly indicate that it upgrades SQLAlchemy to the latest version required for running tests, ensuring consistency with the function output.
option_upgrade_sqlalchemy = click.option(
d3dd43d to
d55a749
Compare
|
Yes. And make it a part of your PR and we can continuously rebase the PRs on top of yours. This is going to be a little involved and require rebasing here and there. |
|
Should we perhaps decide on a single "main" branch (e.g. your one) and target the various SQLA2 PRs into it? Just to save the effort of rebasing across multiple forks... |
7bde1ac to
34e7502
Compare
5efe8cb to
9cb0ba2
Compare
bb95354 to
c6b5a04
Compare
c6b5a04 to
c8dc921
Compare
c8dc921 to
907e709
Compare
+ Skip multiple tests that rely on FAB
- Add a couple of fixtures to ensure a predictable engine state.
- Replace uses of unittest.mock with the mocker fixture.
- Add tests for `AutocommitEngineForMySQL`.