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

Remove SQLAlchemy <1.4 constraint #16630

Merged
merged 1 commit into from
Jun 24, 2021

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented Jun 24, 2021

This was added due to flask-sqlalchemy and sqlalchemy-utils not declaring the upper bounds. They have since released sqlalchemy 1.4-compatible versions, so we can remove that hack.

Note that this does not actually make us run on sqlalchemy 1.4 since flask-appbuilder still has a <1.4 pin. But that's for flask-appbuilder to worry about—code in Airflow is compatible, so we can remove the constraint now, and get sqlalchemy 1.4 as soon as flask-appbuilder allows us to.

This was added due to flask-sqlalchemy and sqlalchemy-utils not declaring
the upper bounds. They have since released sqlalchemy 1.4-compatible
versions, so we can remove that hack.

Note that this does *not* actually make us run on sqlalchemy 1.4 since
flask-appbuilder still has a <1.4 pin. But that's for flask-appbuilder
to worry about -- code in Airflow is compatible, so we can remove the
constraint now, and get sqlalchemy 1.4 as soon as flask-appbuilder
allows us to.
@uranusjr uranusjr force-pushed the sqlalchemy-utils-constraints-update branch from b15ce91 to 842ead8 Compare June 24, 2021 10:52
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jun 24, 2021
@uranusjr uranusjr force-pushed the sqlalchemy-utils-constraints-update branch from 842ead8 to 967aa42 Compare June 24, 2021 10:55
@kaxil kaxil merged commit d181604 into apache:main Jun 24, 2021
@uranusjr uranusjr deleted the sqlalchemy-utils-constraints-update branch June 24, 2021 13:13
kaxil added a commit to astronomer/airflow that referenced this pull request Jul 12, 2021
FAB 3.3 has some issues with permissions as mentioned in apache#16202 and dpgaspar/Flask-AppBuilder#1667

There is no reason we should require >= 3.3 (This was added in apache#15792)

Allow more versions as discussed in apache#16630 (comment)
potiuk pushed a commit to potiuk/airflow that referenced this pull request Aug 2, 2021
This was added due to flask-sqlalchemy and sqlalchemy-utils not declaring
the upper bounds. They have since released sqlalchemy 1.4-compatible
versions, so we can remove that hack.

Note that this does *not* actually make us run on sqlalchemy 1.4 since
flask-appbuilder still has a <1.4 pin. But that's for flask-appbuilder
to worry about -- code in Airflow is compatible, so we can remove the
constraint now, and get sqlalchemy 1.4 as soon as flask-appbuilder
allows us to.

(cherry picked from commit d181604)
potiuk pushed a commit to potiuk/airflow that referenced this pull request Aug 5, 2021
This was added due to flask-sqlalchemy and sqlalchemy-utils not declaring
the upper bounds. They have since released sqlalchemy 1.4-compatible
versions, so we can remove that hack.

Note that this does *not* actually make us run on sqlalchemy 1.4 since
flask-appbuilder still has a <1.4 pin. But that's for flask-appbuilder
to worry about -- code in Airflow is compatible, so we can remove the
constraint now, and get sqlalchemy 1.4 as soon as flask-appbuilder
allows us to.

(cherry picked from commit d181604)
kaxil pushed a commit that referenced this pull request Aug 17, 2021
This was added due to flask-sqlalchemy and sqlalchemy-utils not declaring
the upper bounds. They have since released sqlalchemy 1.4-compatible
versions, so we can remove that hack.

Note that this does *not* actually make us run on sqlalchemy 1.4 since
flask-appbuilder still has a <1.4 pin. But that's for flask-appbuilder
to worry about -- code in Airflow is compatible, so we can remove the
constraint now, and get sqlalchemy 1.4 as soon as flask-appbuilder
allows us to.

(cherry picked from commit d181604)
jhtimmins pushed a commit that referenced this pull request Aug 17, 2021
This was added due to flask-sqlalchemy and sqlalchemy-utils not declaring
the upper bounds. They have since released sqlalchemy 1.4-compatible
versions, so we can remove that hack.

Note that this does *not* actually make us run on sqlalchemy 1.4 since
flask-appbuilder still has a <1.4 pin. But that's for flask-appbuilder
to worry about -- code in Airflow is compatible, so we can remove the
constraint now, and get sqlalchemy 1.4 as soon as flask-appbuilder
allows us to.

(cherry picked from commit d181604)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants