Limit SQLAlchemy to < 1.4.0 for 2.2.* line

[potiuk]

The recent release of FAB 3.4.4 has unblocked us from upgrading
SQLAlchemy to 1.4.* version. We wanted to do it for quite some
time however upgrading to 1.4.* of sqlalchemy and allowing our
users to use it for 2.2.4 is a bit risky.

We are fixing resulting "aftermath" in the main branch and as
of this commit there are two fixes merged and remaining MsSQL
problem. The MSSql problem does not affect 2.2.4 as MsSQL will
be available only starting from 2.3.0, however the two other
problems have shown that SQLAlchemy has a potential to break
things and we might want to test it more thoroughly before
releasing 2.3.0.

The problems in question are #21205 and #21228. Both were only
test problems but the indicate that there might be more hidden
issues involved.

In order to limit risks, this PR proposes to limit SQLAlchemy
for 2.2.* to < 1.4.0. This will allow to upgrade FAB and
related dependencies without opening up Airflow to upgrade to
SQLAlchemy 1.4 (yet).

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

[potiuk]

NOTE! This PR's target it v2-2-test - not main and the proposal is to limit SQLAlchemy in 2.2.4 and above (not in main - I am working to fix the last test failure resulting from SQLAlchemy update in main).

[ashb]

I thought we targeted v2-2-stable in case of a PR (and v2-2-test was only for cherry-picks and is then rebased on top of -stable as needed?)

[github-actions]

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.

[potiuk]

I thought we targeted v2-2-stable in case of a PR (and v2-2-test was only for cherry-picks and is then rebased on top of -stable as needed?)

Yeah. Usually yes, but we are really close to release and there are many changes cherry-picked to v2-2-test already - including setup.py/.cfg changes, so wanted to avoid merge conflicts - we have no changes in v2-2-stable yet since it was released, so if we make it to v2-2-test, then moving stable will be just "fast-forward".

[uranusjr]

Hmm, do we have a mechanism to limit SQLAlchemy < 1.4 in the constraints file, but not in the actual package metadata? This way we can “officially” only support <1.4, but someone adventurous enough to want 1.4+ can still have that without the package metadata blocking the possibility. I feel that’d be a better approach.

[potiuk]

Hmm, do we have a mechanism to limit SQLAlchemy < 1.4 in the constraints file, but not in the actual package metadata? This way we can “officially” only support <1.4, but someone adventurous enough to want 1.4+ can still have that without the package metadata blocking the possibility. I feel that’d be a better approach.

Yes. We can do that as well. We already do that in few other cases and that's also a possibilitty.

The way we do that we add limitation in the Dockerfile.ci (this image is used in CI to prepare the constraints):

https://github.com/apache/airflow/blob/v2-2-test/Dockerfile.ci#L276 ​

Actually I toyed with that idea too, but I thought that install_requires is a bit safer as SQLALchemy is really important piece for us. I think this case is actually a very nice supportive (and not strawman) case to the proposal I made here:

https://lists.apache.org/thread/250pfvogkqb31g2vj5p0yz3ntz5qj1ht

Sqlalchemy for me is really important piece of Airflow and even small change might impact us a lot - for example performance of certain queries might change dramaticaly if SqlAlchemy changes the way how they are generated - even if for them this will be a "small change" and results might be ''catastrophic'.

As I explained in my latest proposal there, I think we should identifty all the "really important" dependencies (few of them) and upper-bound them, all the rest we should not. This way we could take a "deliberate" effort to migrate to higher versions (as we do now in main trying to fix it and then later release it in 2.3.0 with enough manual testing. Just having a few of those, will make sure that we can manage the upper-bounds "deliberately". All the rest can easily rely on constraints.

My point here is that sqlalchemy is on top of the list of those deps which should be upperbound (if we decide to upperbound dependencies). So if we agreee that sometimes we do upper-bound, IMHO we should do it here.

But if we decide to relax it, I am also fine (but then IMHO we should relax all the upperbound limits in this case).

[jedcunningham]

We see enough users not using constraints that I don't think we should rely on it in this case, especially given we are so close to 2.2.4's release.

[potiuk]

And this one - I thing should be quite proving the point on upper-bounding SQLAlchemy is a good idea:

#21238

This was only MsSQL but I wonder what else will find when we start more thoroughly testing it.