OK. I think I found and fixed all the compatibility issues in https://github.com/apache/airflow/pull/39513 - except one last openlineage plugin enablement fix (but I think reviews for all the other changes would be great until we fix the issue). There are probably a few incompatibilities that will need to be addressed before we release 2.1.10 so I need confirmation / comments from Niko/Daniel if my findings are correct.
On Fri, May 10, 2024 at 12:54 PM Jarek Potiuk <ja...@potiuk.com> wrote: > > Just for clarification, this is only related to the provider's tests, > right? > > Absolutely. > > On Fri, May 10, 2024 at 11:21 AM Andrey Anshin <andrey.ans...@taragol.is> > wrote: > >> > "enable" tests for 2.8 and 2.7 separately >> >> Just for clarification, this is only related to the provider's tests, >> right? >> >> >> >> On Fri, 10 May 2024 at 13:15, Jarek Potiuk <ja...@potiuk.com> wrote: >> >> > > Regarding Airflow 2.7 and Airflow 2.8 in the time we are ready to move >> > forward to the initial version of Airflow 3 providers might already drop >> > support of these versions in providers. >> > Airflow 2.7 in the mid of August 2024 >> > Airflow 2.8 in the mid of December 2024 >> > >> > Yep. But also "here and now" those compatibility tests might help us to >> > find some hidden incompatibilities (and prevent adding future ones). We >> can >> > see how much complexity we are dealing with when we attempt to enable >> the >> > tests for 2.8 and then 2.7 and decide if it's worth it. The change I >> added >> > makes it easy to just "enable" tests for 2.8 and 2.7 separately. >> > >> > On Fri, May 10, 2024 at 11:10 AM Andrey Anshin < >> andrey.ans...@taragol.is> >> > wrote: >> > >> > > BTW, forget to mention that we should also check Pytest: Good >> Integration >> > > Practices from >> > > https://docs.pytest.org/en/stable/explanation/goodpractices.html >> > > >> > > >> > > >> > > >> > > >> > > On Fri, 10 May 2024 at 13:07, Andrey Anshin <andrey.ans...@taragol.is >> > >> > > wrote: >> > > >> > > > I think the current solution with run tests against installed >> packages >> > > > might help with future modifications and develop new dev experience. >> > And >> > > > what is more important is help to find problems and >> incompatibilities >> > of >> > > > providers with the previous version of Airflow "here and now". >> > > > >> > > > Regarding Airflow 2.7 and Airflow 2.8 in the time we are ready to >> move >> > > > forward to the initial version of Airflow 3 providers might already >> > drop >> > > > support of these versions in providers. >> > > > Airflow 2.7 in the mid of August 2024 >> > > > Airflow 2.8 in the mid of December 2024 >> > > > >> > > > >> > > > >> > > > On Fri, 10 May 2024 at 12:32, Jarek Potiuk <ja...@potiuk.com> >> wrote: >> > > > >> > > >> And yes - as we get down to 2.8 and 2.7 it might be possible that >> we >> > > will >> > > >> already implement some of the simplifications you mentioned as it >> > might >> > > be >> > > >> easier than adding back-compatiblity to the current ways. I assume >> it >> > > will >> > > >> be `quite` a bit harder to make our test suite work with Airflow >> 2.8 >> > and >> > > >> then 2.7 - so it might be that some of the refactors and changes >> will >> > > need >> > > >> to be applied to make it easier to maintain. >> > > >> >> > > >> On Fri, May 10, 2024 at 10:27 AM Jarek Potiuk <ja...@potiuk.com> >> > wrote: >> > > >> >> > > >> > Yep. I think these are all good ideas, and I think this should be >> > part >> > > >> of >> > > >> > our big Airflow 2 vs. Airflow 3 discussion. Almost as important >> as >> > > what >> > > >> is >> > > >> > in and what is out is where and how development of different >> > > components >> > > >> > happen. Same repo? Different repos? Different branches? Single >> > > monorepo >> > > >> for >> > > >> > Airflow2 + Providers, and separate repo for Airflow 3 only? >> Keeping >> > > >> > monorepo for Airflow 3 ? How do we cherry-pick? >> > > >> > I think we need to "design" the developer experience as part of >> our >> > > >> > discussion - and it should be a serious discussion considering >> all >> > the >> > > >> > consequences. How do we test things together? How do we test >> > > >> > back-compatibility? How do we prevent Airflow 3 PRs breaking >> > > providers? >> > > >> > Should we separate-out Helm chart as well? There are many many >> > > questions >> > > >> > and multiple possible answers. >> > > >> > >> > > >> > But let's not derail this discussion - my proposal is to use >> what we >> > > >> have >> > > >> > now and simply get back-compatibility working without changing >> the >> > > >> > structure (yet), but as part of Airflow 2 vs. Airflow 3 we should >> > make >> > > >> sure >> > > >> > this topic is fully covered and we get to consensus on the >> answers. >> > > >> > >> > > >> > J >> > > >> > >> > > >> > >> > > >> > On Fri, May 10, 2024 at 9:17 AM Andrey Anshin < >> > > andrey.ans...@taragol.is >> > > >> > >> > > >> > wrote: >> > > >> > >> > > >> >> Great job, Jarek! >> > > >> >> >> > > >> >> I would have some proposals, which should be considered as a >> long >> > > term >> > > >> >> >> > > >> >> >> > > >> >> We should rework our test structure to fully run provider tests >> > > without >> > > >> >> touching the Core tests. >> > > >> >> The main problem here is that we configure a lot of things into >> the >> > > >> root >> > > >> >> conftest.py which might be a problem in case of running tests >> on a >> > > >> >> provider >> > > >> >> under a different version of the airflow. Core itself might use >> > > >> something >> > > >> >> which was only added in a recent version of Airflow, but this >> > should >> > > >> not >> > > >> >> be >> > > >> >> a case in case of providers. So we should slightly change the >> test >> > > >> >> structure, unless we could decouple providers for the mono repo >> > (i'm >> > > >> not >> > > >> >> sure it is even a case in the future). E.g. move >> tests/providers to >> > > >> >> tests/providers/unit and after so w would have >> > > >> >> tests/system/{unit|system|integration|conftest.py) maybe also >> some >> > > >> helpers >> > > >> >> for providers should be moved into the tests/providers/helpers >> (I >> > > don't >> > > >> >> like name helpers but this only for the reference). In the same >> > > momemen >> > > >> >> move core related tests to the tests/core (name could be >> different) >> > > and >> > > >> >> create structure like >> > > >> >> tests/core/{unit|system|integration|helpers|conftest.py}. And >> move >> > as >> > > >> much >> > > >> >> as possible from tests/conftest.py to appropriate in >> > > >> >> tests/{core|providers}/conftest.py >> > > >> >> >> > > >> >> >> > > >> >> Providers tests should not be relied on DB backend, and could be >> > > easily >> > > >> >> run >> > > >> >> on any of the supported, because providers not extend DB backend >> > > >> support >> > > >> >> DB, and if tests pass in core we take an assumption that >> providers >> > > >> could >> > > >> >> use any of them e.g. SQlite (preferable for setup in xdists) or >> > > >> Postgres. >> > > >> >> >> > > >> >> If we go even further we might want to move specific helpers in >> the >> > > >> >> separate test package, e.g `pytest-apache-airflow`, and move all >> > > common >> > > >> >> helpers and simple setup/configuration tests airflow environment >> > > >> (really >> > > >> >> simple one as first steps) and compatibility level, same as >> > provider >> > > I >> > > >> >> year >> > > >> >> after feature version released. We could test this package >> against >> > > >> >> different versions of airflow to make sure that within >> combination >> > > >> Airflow >> > > >> >> (2.7-2.9 + main) + `pytest-apache-airflow` we could run tests >> > against >> > > >> each >> > > >> >> provider. >> > > >> >> This pytest package also would be released, uploaded into the >> PyPI >> > > and >> > > >> >> could be installed via pip/uv however at least for the initial >> > stage >> > > it >> > > >> >> shouldn't be considered to use outside of Airflow and Airflow >> > > Providers >> > > >> >> CI, >> > > >> >> in another word it is no GA for the end users. This might be >> > changed >> > > in >> > > >> >> the >> > > >> >> future but let's focus that this package only for Airflow >> > development >> > > >> >> internals >> > > >> >> >> > > >> >> On Fri, 10 May 2024 at 01:08, Jarek Potiuk <ja...@potiuk.com> >> > wrote: >> > > >> >> >> > > >> >> > Hello everyone, >> > > >> >> > >> > > >> >> > As part of preparation for the Airflow 3 move and (possible) >> > > provider >> > > >> >> > separation (I have some ideas how to do it but that should be >> a >> > > >> separate >> > > >> >> > discussion) I took on the task of improving our compatibility >> > tests >> > > >> for >> > > >> >> > Providers. I discussed it briefly with Kaxil and Ash and >> decided >> > to >> > > >> >> give it >> > > >> >> > a go and see what it takes. >> > > >> >> > >> > > >> >> > The PR here: https://github.com/apache/airflow/pull/39513 >> > > >> >> > >> > > >> >> > I extended our "import" checks with checks that also run all >> > > provider >> > > >> >> unit >> > > >> >> > tests for specified airflow versions (for now 2.9.1 - but >> once we >> > > >> get it >> > > >> >> > merged/approved we can make sure the tests are working for 2.7 >> > and >> > > >> >> 2.8). We >> > > >> >> > will also be able to run "future" compatibility tests in case >> we >> > > >> decide >> > > >> >> to >> > > >> >> > leave providers aside from Airflow 3 and will be able to run >> the >> > > >> tests >> > > >> >> for >> > > >> >> > both`main` and `pypi`-released versions of airflow. >> > > >> >> > >> > > >> >> > A number of our tests rely on some internals of Airflow and >> they >> > > >> >> > implicitly rely on the fact that they are run directly in >> airflow >> > > >> source >> > > >> >> > tree - but there are not many of those - after some initial >> > > >> >> compatibility >> > > >> >> > fixes I got 50 or so tests failing for 2.9.1 (probably there >> will >> > > be >> > > >> >> more >> > > >> >> > for 2.8.0 and 2.7.0, but I want to make 2.9.1 works first). >> > > >> >> > >> > > >> >> > I almost got it working (few tests are still failing) with >> > > >> compatibility >> > > >> >> > for 2.9.1 but I will need some help from a few people - around >> > > >> >> > openlineage and serialization but also around recently >> improved >> > > >> >> try_number >> > > >> >> > :). I will reach out to the relevant people individually if we >> > see >> > > >> that >> > > >> >> as >> > > >> >> > a good idea. >> > > >> >> > >> > > >> >> > It requires some care when writing tests to make sure the >> tests >> > can >> > > >> be >> > > >> >> run >> > > >> >> > against installed airflow and not from sources. So in the >> future >> > > >> anyone >> > > >> >> > contributing provider changes will have to make sure the tests >> > pass >> > > >> also >> > > >> >> > for past airflow versions (there are simple instructions >> > explaining >> > > >> how >> > > >> >> to >> > > >> >> > do it with breeze). But once we merge it, this will be caught >> on >> > PR >> > > >> >> level >> > > >> >> > and should be easy to fix any of those problems. >> > > >> >> > >> > > >> >> > The benefit of having the tests is that we not only do simple >> > > import >> > > >> >> tests >> > > >> >> > but actually run provider tests, the drawback is that >> sometimes >> > > tests >> > > >> >> will >> > > >> >> > have to be adapted to make sure they work also for installed >> > older >> > > >> >> airflow >> > > >> >> > versions (which is not always straightforward or easy and will >> > need >> > > >> some >> > > >> >> > compatibility code in tests - for example after recent rename >> of >> > > >> >> > airflow.models.ImportError to ParsingImportError we had to add >> > > >> >> compat.py to >> > > >> >> > test_utils and import ParsingImportError from there rather >> than >> > > from >> > > >> >> > Airflow directly in tests. >> > > >> >> > >> > > >> >> > I don't think it's too controversial - being able to run unit >> > tests >> > > >> for >> > > >> >> > providers for old (and future) versions of Airflow is >> generally >> > > >> quite an >> > > >> >> > improvement in stability, but this adds a bit overhead on >> > > >> >> contributions, so >> > > >> >> > I am letting everyone here know it's coming, so that it's not >> a >> > > >> >> surprise to >> > > >> >> > contributors. >> > > >> >> > >> > > >> >> > J. >> > > >> >> > >> > > >> >> >> > > >> > >> > > >> >> > > > >> > > >> > >> >
