Elad: > Why? The contrib just import the new file/class path. How can it be broken? It has no functionality other than change the path
Because even if we import the path, the provider operator/hook itself might change in backwards incompatible ways. And we've already done that: Example: We released a breaking change of google provider 6.0.0: 6.0.0 Breaking changes Migrate Google Cloud Build from Discovery API to Python SDK (#18184) In contrib we have airflow/contrib/operators/gcp_cloud_build_operator.py which does this: from airflow.providers.google.cloud.operators.cloud_build import CloudBuildCreateBuildOperator # noqa The CloudBuildCreateBuildOperator in provider 6.0.0 changed in backwards-incompatible way. This means that when you upgraded Airflow (and used "from airflow.contrib.operators.gcp_cloud_build_operator import CloudBuildCreateBuildOperator" - it would be backwards incompatible in the release with 5.* google provide and 6.* provider. One could even say that this is worse than missing the operator completely, because it could be broken without you knowing that (and might succeed even if - for example - you pass it wrong values). Technically we broke compatibility of "airflow/contrib/operators/gcp_cloud_build_operator.py" already back then. Bas: > Having to make a change to keep your system working throughout minor versions — to me that’s a breaking change. This is exactly what happens now with Providers. When you migrate to Airflow 2.4 and it links to the 8.0 version of Google provider (assume 2.3 linked to 7) you have to make changes to make it work in backwards incompatible way - you have to downgrade google provider to 7.0. You still can do it, but it is not "out-of-the-box". What I am proposing is to extend the same approach to "contrib" - you have a way to keep compatibility (by installing an extra package) but it is not "out-of-the box". When you upgrade following our recommended (and the only one we consider as valid) way - with constraints, you have to perform an action to keep perfect compatibility. Seems that, for some strange reason, we have a much stricter approach currently to "contrib" operators than "providers", which makes very little sense IMHO. J. On Thu, Aug 4, 2022 at 1:18 PM Bas Harenslak <b...@astronomer.io.invalid> wrote: > Not if you add install `airflow[contrib]` - then it won't break. >>> >> > Having to make a change to keep your system working throughout minor > versions — to me that’s a breaking change. > > So as much as we’d like to get rid of the contrib folder, we should really > *only* change that throughout major versions. > > Bas > > On 4 Aug 2022, at 12:46, Elad Kalif <elad...@apache.org> wrote: > > > Every time we release Amazon major version of provider, it might break > if you relied on it being backwards compatible. > > Why? The contrib just import the new file/class path. How can it be > broken? It has no functionality other than change the path > > On Thu, Aug 4, 2022 at 1:25 PM Jarek Potiuk <ja...@potiuk.com> wrote: > >> And BTW. airflow/contrib/operators/ecs_operator.py is potentially >> already broken multiple times - Every time we release Amazon major >> version of provider, it might break if you relied on it being backwards >> compatible. >> >> On Thu, Aug 4, 2022 at 12:22 PM Jarek Potiuk <ja...@potiuk.com> wrote: >> >>> Not if you add install `airflow[contrib]` - then it won't break. >>> >>> On Thu, Aug 4, 2022 at 12:21 PM Ash Berlin-Taylor <a...@apache.org> >>> wrote: >>> >>>> Looking at the PR I see you are talking specifically about removing >>>> airflow/contrib/operators/ecs_operator.py. >>>> >>>> I disagree with you that this doesn't count as a breaking change. >>>> >>>> For example, if we remove that file: >>>> >>>> I have a dag on Airflow 2.3. I upgrade to 2.4. My dag breaks. >>>> >>>> That is 100% a breaking change to me. >>>> >>>> -a >>>> >>>> On Thu, Aug 4 2022 at 11:08:14 +01:00:00, Ash Berlin-Taylor < >>>> a...@apache.org> wrote: >>>> >>>> One question: Are you talking about removing things from >>>> airflow.contrib, or things already with in airflow.providers.*? >>>> >>>> -a >>>> >>>> On Thu, Aug 4 2022 at 11:55:52 +02:00:00, Jarek Potiuk < >>>> ja...@potiuk.com> wrote: >>>> >>>> Hello everyone, >>>> >>>> Following the discussion in >>>> https://github.com/apache/airflow/pull/25413 I have a proposal. >>>> >>>> Why don't we remove all "contrib" and other Airflow 1.10 deprecated >>>> classes to a separate package and add dependency to that package as >>>> [contrib] or [deprecated] extra in Airflow - and release one of the >>>> next 2.* (2.4/2.5) airflow versions without those classes ? >>>> >>>> This should be possible I think (We could do it for all "contrib" >>>> easily and for some other individual classes in "operators" and "hooks" >>>> that would likely require a little dynamic python magic to not override the >>>> folders). >>>> >>>> There are a number of benefits: >>>> >>>> 0) lots of old, defunc mostly deprecation code can be removed from the >>>> main repo - including lots of tests that verify that. >>>> >>>> 1) new users would not even know about those classes/contrib - less >>>> confusion >>>> >>>> 2) many of those "contrib" classes are not backwards-compatible with >>>> old 1.10 classes already as we had many "major" releases in many providers >>>> - so this might be a little misleading for those who are still in 1.10 >>>> that they can easily "migrate" without making any changes >>>> >>>> 3) there are many users who even now use "contrib" and we could use the >>>> opportunity to "guide them" into migration. To make it smoother, we could >>>> likely implement dynamic attribute checking in packages and raise >>>> appropriate instructions to those who still use it and migrate to 2.5 or >>>> 2.6 (and they will still have the option to install the "contrib" package). >>>> The instructions could be the same as in deprecation messages today (but >>>> they would fail in case the "contrib" package is not installed) >>>> >>>> 4) We give a great tool for admins of Airflow installation. Currently >>>> the admins have no tools to force their users to switch-off from using >>>> contrib if they still do. But with this one they will simply be able to >>>> install airflow without the "contrib" package and the users will have to >>>> adapt. We can even provide those "Admins" instructions on how to build your >>>> own "contrib" package if you want to do it gradually and ask your users to >>>> remove class-by-class or whatever way you want. >>>> >>>> Technically - we are not breaking SemVer compatibility - you can still >>>> get back to the contrib if you install the separate package. So we can do >>>> it without bumping Major version >>>> >>>> WDYT? >>>> >>>> J. >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >
