Merged the compatibility fix - nice and straightforward :D On Mon, Jun 30, 2025 at 12:05 PM Jarek Potiuk <ja...@potiuk.com> wrote:
> NICE! > > On Mon, Jun 30, 2025 at 11:50 AM Dev-iL <gid....@gmail.com> wrote: > >> Good news, Everyone! >> >> I addressed CI-reported issues related to 2.0. Originally all my work >> was organized in a single PR (#52233), but after the CI became >> green(er), I decided to split it into two for better organization/scope >> and easier review. These are the PRs in question: >> >> 1. #52518: The cherry-picked compatibility fixes. This has no >> prerequisites, is green, and waiting for review/approval. I will say >> carefully that once this is merged we'll have "experimental" support >> of SQLA2. >> 2. #52233: (The original PR with all the changes.) This adds the SQLA >> 2.0 CI jobs (w/o forcing `SQLALCHEMY_WARN_20`). This requires FAB5 >> PR by Jarek to be merged first. >> >> You might wonder what the plan is with regard to the deprecation fixes - >> that's a separate effort to be coordinated later on. >> >> For now - your attention is needed in >> https://github.com/apache/airflow/pull/52518. >> >> Thanks, >> Dev-iL >> >> >> On 2025/06/26 12:49:25 Dev iL wrote: >> > Hello everyone, >> > >> > With the release of Airflow 3 and the refactoring that moved >> > flask-appbuilder (FAB) into a separate provider, as well as recent >> work by >> > the FAB developers to support SQLAlchemy v2.0 (SQLA2), it is finally >> > possible to work on supporting SQLA2 in Airflow as well. >> > >> > To get this process going, I added two CI tasks that install SQLA2 (PR >> > #52233) and am now slowly adding various workarounds to overcome >> repeated >> > test failures. After doing this for a while, there's a couple of >> discussion >> > points I want to raise: >> > >> > 1. What is the plan regarding dual support of 1.4 & 2.x? If we want to >> > have that, it might lead to pretty messy code (see details below). >> > 2. Should we work on supporting SQLA 2.0 or jump straight to 2.1 (which >> > is currently in beta)? >> > >> > I think we should decide on the refactoring principles and start >> moving in >> > that direction. With some luck, it might be possible to get this done >> in >> > time for 3.1. >> > >> > I'd love your inputs on how to best approach this task. More >> importantly, >> > if anyone wants to participate - please leave your comment on my PR, >> and >> > let's coordinate. I suggest you use my branch as a starting point, >> since it >> > has the tests in place. >> > >> > Best, >> > Dev-iL >> > >> > P.S. >> > At the time of writing, the status of running the tests (via `breeze >> > testing core-tests --upgrade-sqlalchemy --maxfail=1000`) is: 134 >> failed, >> > 6469 passed, 47 skipped, 8 xfailed, 1 warning >> > ------------------- >> > >> > SQLA2 introduced stricter type annotation requirements for ORM mapped >> > attributes. All mapped columns need to use the `Mapped[]` generic type >> > annotation: >> > >> > <pre> >> > class TaskInstance(Base): >> > __tablename__ = "task_instance" >> > >> > # Before (SQLA 1.4) >> > # id = Column(Integer, primary_key=True) >> > # dag_id = Column(String, ForeignKey("dag.id")) >> > # dag_model = relationship("DagModel") >> > >> > # After (SQLA 2.0) >> > id: Mapped[int] = mapped_column(Integer, primary_key=True) >> > dag_id: Mapped[str] = mapped_column(String, ForeignKey("dag.id")) >> > dag_model: Mapped["DagModel"] = relationship() >> > </pre> >> > >> > To address differences of similar nature, and assuming we're >> interested in >> > dual SQLA version support, I can think of two extremes: >> > >> > 1. We implement two copies of each model, one for each version of SQLA, >> > and use the relevant one based on a runtime check. >> > 2. We use workarounds that disable the breaking features of SQLA2 (e.g. >> > >> >> https://github.com/apache/airflow/pull/52233/commits/90d738880553707fa01ee2a6b4acd40609e8cc2f >> ) >> > where possible, and resort to runtime branching where impossible, thus >> > postponing the rewrite to whenever SQLA1.4 support will be dropped. >> > >> > ---------------------- >> > >> > References: >> > >> > 1. Main SQLA2 support issue: >> > https://github.com/apache/airflow/issues/28723 >> > 2. My PR adding SQLA2 to the CI: >> > https://github.com/apache/airflow/pull/52233 >> > 3. FAB's SQLA2 support PR: >> > https://github.com/dpgaspar/Flask-AppBuilder/pull/2241 >> > > >
