You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Jarek Potiuk <ja...@potiuk.com> on 2022/08/04 09:55:52 UTC
[DISCUSS] Move "contrib" and all old classes to a separate package
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.
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
Only contrib (and possibly a few deprecated operators/hooks from
airflow/operators/, airflow/hooks - for example
airflow.bash.bash_operator.BashOperator which is now
airflow.bash.BAshOperator)
On Thu, Aug 4, 2022 at 12:08 PM Ash Berlin-Taylor <as...@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.
>
>
>
>
>
>
>
>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Ash Berlin-Taylor <as...@apache.org>.
Yeah, that sounds like it could work.
On Thu, Aug 4 2022 at 15:52:28 +02:00:00, Jarek Potiuk
<ja...@potiuk.com> wrote:
> > This is the issue though. apache-airflow does not depend on
> apache-airflow-providers-google >= 8.0, so you can still be on the
> same version of the provider on 2.3 and 2.4, but by _only_ upgrading
> apache-airflow package your DAG now breaks.
>
> Correct. The difference is that by only upgrading apache-airflow
> alone you will break it. And I see why this might be seen as
> "problematic".
>
> But I have another idea. Why - rather than keeping the deprecations
> in the code we load such deprecated we add dynamic attributes to
> __init__ of airflow.contrib, airflow.operators, airflow.hooks. (to
> resolve to provider operators).
>
> Such dynamic attributes will be invisible to autocompletion, will not
> be documented with APIs nor visible in the source code (Except deep
> inspection of __init__ in those packages).
>
> This has no compatibility breaking potential whatsoever
>
>
> J.
>
>
> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <ash@apache.org
> <ma...@apache.org>> wrote:
>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk
>> <jarek@potiuk.com <ma...@potiuk.com>> wrote:
>>> 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".
>>
>> This is the issue though. apache-airflow does not depend on
>> apache-airflow-providers-google >= 8.0, so you can still be on the
>> same version of the provider on 2.3 and 2.4, but by _only_ upgrading
>> apache-airflow package your DAG now breaks.
>>
>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
And there is a possibility of having a switch to turn those deprecation
warnings into errors in this case - still giving the admin of
Airflow installation (even selectively) to migrate out of contrib for their
users.
On Thu, Aug 4, 2022 at 3:52 PM Jarek Potiuk <ja...@potiuk.com> wrote:
> > This is the issue though. apache-airflow does not depend on
> apache-airflow-providers-google >= 8.0, so you can still be on the same
> version of the provider on 2.3 and 2.4, but by _only_ upgrading
> apache-airflow package your DAG now breaks.
>
> Correct. The difference is that by only upgrading apache-airflow alone you
> will break it. And I see why this might be seen as "problematic".
>
> But I have another idea. Why - rather than keeping the deprecations in the
> code we load such deprecated we add dynamic attributes to __init__ of
> airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider
> operators).
>
> Such dynamic attributes will be invisible to autocompletion, will not be
> documented with APIs nor visible in the source code (Except deep inspection
> of __init__ in those packages).
>
> This has no compatibility breaking potential whatsoever
>
>
> J.
>
>
> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>
>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <ja...@potiuk.com>
>> wrote:
>>
>> 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".
>>
>>
>> This is the issue though. apache-airflow does not depend on
>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>> apache-airflow package your DAG now breaks.
>>
>>
>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
The proposal on all contrib classes conversion to PEP-562 here:
https://github.com/apache/airflow/pull/26153.
That's quite a big change (ca. - 9000 lines) and I think it's rather worth
it :). Our code seems quite a bit cleaner after that. And I think we
achieve a much stronger message to our users - the old imports will not be
deprecated any more but they will be reported as missing in their IDEs, but
they will continue to work (and continue raising deprecation warnings),
which I think is the exact message we want to pass to our users at this
moment.
If/when we merge it I can apply the same approach (or one that we agree
with to other deprecated classes as well). I am not sure if the way I've
done is optimal, and I'm looking for comments.
J.
On Fri, Aug 5, 2022 at 1:15 PM Jarek Potiuk <ja...@potiuk.com> wrote:
> https://lists.apache.org/thread/cp9n8r9x75xzzsdjgdqd82p8nmyn1nd5 ->
> non-broken link here.
>
> On Fri, Aug 5, 2022 at 1:15 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>
>> And I agree with Ash - two years ago it would be a bad choice but we are
>> past the time when we should be "gentle" with it :).
>>
>> But we are now at the point when our priorities shifted - we want now to
>> 'strengthen" our message for anyone still using the old imports.
>>
>> I also think that we could use this also to "strengthen" our messages to
>> our users over time (see the little distraction from the other topic we had
>> in https://lisdiscussion in
>> ts.apache.org/thread/cp9n8r9x75xzzsdjgdqd82p8nmyn1nd5)
>>
>> We could (if we want) make each warning generated in this case an
>> "adjustable" level:
>>
>> 1) log message
>> 2) UI "soft" message
>> 3) UI "annoying" message
>> 4) Error
>>
>> This could be easily configurable per group of deprecations and we could
>> change defaults over time and give the opportunity of the person who
>> manages Airflow to strengthen (or weaken) the message.
>>
>> I remember a discussion with a user who manages "Airflow farm" in
>> their company and he argued that it is extremely annoying that his users
>> still use some "contrib" operators and he has a hard time getting rid of
>> those even if he really wants to do it to make it a more consistent usage
>> across his company.
>> With a configurable level of warnings/errors for deprecations, we do not
>> break compatibility, we have more impact and can further strengthen our
>> message in future versions and also give those who manage airflow an
>> opportunity to even block the "deprecation" usage if they want.
>>
>> J.
>>
>> On Fri, Aug 5, 2022 at 1:02 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>>
>>> Yeah. Removing them from IDE is PRECISELY the goal :).
>>>
>>> On Fri, Aug 5, 2022 at 11:01 AM Ash Berlin-Taylor <as...@apache.org>
>>> wrote:
>>>
>>>> And by removing them/making them dynamic the ide will think they don't
>>>> even exist anymore.
>>>>
>>>> (Also: these have been showing as deprecated in IDEs for ~2 years; If
>>>> someone was going to update their code they would have already)
>>>>
>>>> On 5 August 2022 09:58:38 BST, Ash Berlin-Taylor <as...@apache.org>
>>>> wrote:
>>>>>
>>>>> I think the _goal_ is to not have the deprecated classes show up in
>>>>> IDEs - i.e. we want to discourage people from using them.
>>>>>
>>>>> On 5 August 2022 09:44:42 BST, "Kamil Breguła" <dz...@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> I am concerned that the use of dynamic attributes will prevent the
>>>>>> IDE from recognizing these deprecated modules and marking them in the IDE.
>>>>>> This is what it looks like in my IDE now: https://imgur.com/a/OnRjiKs
>>>>>>
>>>>>> pt., 5 sie 2022 o 00:28 Ferruzzi, Dennis <fe...@amazon.com.invalid>
>>>>>> napisał(a):
>>>>>>
>>>>>>> > add dynamic attributes to __init__ of airflow.contrib,
>>>>>>> airflow.operators, airflow.hooks. (to resolve to provider operators).
>>>>>>>
>>>>>>> This sounds promising to me.
>>>>>>>
>>>>>>> ------------------------------
>>>>>>> *From:* Jarek Potiuk <ja...@potiuk.com>
>>>>>>> *Sent:* Thursday, August 4, 2022 6:52 AM
>>>>>>> *To:* dev@airflow.apache.org
>>>>>>> *Subject:* RE: [EXTERNAL][DISCUSS] Move "contrib" and all old
>>>>>>> classes to a separate package
>>>>>>>
>>>>>>>
>>>>>>> *CAUTION*: This email originated from outside of the organization.
>>>>>>> Do not click links or open attachments unless you can confirm the sender
>>>>>>> and know the content is safe.
>>>>>>>
>>>>>>> > This is the issue though. apache-airflow does not depend on
>>>>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>>>>> apache-airflow package your DAG now breaks.
>>>>>>>
>>>>>>> Correct. The difference is that by only upgrading apache-airflow
>>>>>>> alone you will break it. And I see why this might be seen as "problematic".
>>>>>>>
>>>>>>> But I have another idea. Why - rather than keeping the deprecations
>>>>>>> in the code we load such deprecated we add dynamic attributes to __init__
>>>>>>> of airflow.contrib, airflow.operators, airflow.hooks. (to resolve to
>>>>>>> provider operators).
>>>>>>>
>>>>>>> Such dynamic attributes will be invisible to autocompletion, will
>>>>>>> not be documented with APIs nor visible in the source code (Except deep
>>>>>>> inspection of __init__ in those packages).
>>>>>>>
>>>>>>> This has no compatibility breaking potential whatsoever
>>>>>>>
>>>>>>>
>>>>>>> J.
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <
>>>>>>>> jarek@potiuk.com> wrote:
>>>>>>>>
>>>>>>>> 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".
>>>>>>>>
>>>>>>>>
>>>>>>>> This is the issue though. apache-airflow does not depend on
>>>>>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>>>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>>>>>> apache-airflow package your DAG now breaks.
>>>>>>>>
>>>>>>>>
>>>>>>>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
https://lists.apache.org/thread/cp9n8r9x75xzzsdjgdqd82p8nmyn1nd5 ->
non-broken link here.
On Fri, Aug 5, 2022 at 1:15 PM Jarek Potiuk <ja...@potiuk.com> wrote:
> And I agree with Ash - two years ago it would be a bad choice but we are
> past the time when we should be "gentle" with it :).
>
> But we are now at the point when our priorities shifted - we want now to
> 'strengthen" our message for anyone still using the old imports.
>
> I also think that we could use this also to "strengthen" our messages to
> our users over time (see the little distraction from the other topic we had
> in https://lisdiscussion in
> ts.apache.org/thread/cp9n8r9x75xzzsdjgdqd82p8nmyn1nd5)
>
> We could (if we want) make each warning generated in this case an
> "adjustable" level:
>
> 1) log message
> 2) UI "soft" message
> 3) UI "annoying" message
> 4) Error
>
> This could be easily configurable per group of deprecations and we could
> change defaults over time and give the opportunity of the person who
> manages Airflow to strengthen (or weaken) the message.
>
> I remember a discussion with a user who manages "Airflow farm" in
> their company and he argued that it is extremely annoying that his users
> still use some "contrib" operators and he has a hard time getting rid of
> those even if he really wants to do it to make it a more consistent usage
> across his company.
> With a configurable level of warnings/errors for deprecations, we do not
> break compatibility, we have more impact and can further strengthen our
> message in future versions and also give those who manage airflow an
> opportunity to even block the "deprecation" usage if they want.
>
> J.
>
> On Fri, Aug 5, 2022 at 1:02 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>
>> Yeah. Removing them from IDE is PRECISELY the goal :).
>>
>> On Fri, Aug 5, 2022 at 11:01 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>>
>>> And by removing them/making them dynamic the ide will think they don't
>>> even exist anymore.
>>>
>>> (Also: these have been showing as deprecated in IDEs for ~2 years; If
>>> someone was going to update their code they would have already)
>>>
>>> On 5 August 2022 09:58:38 BST, Ash Berlin-Taylor <as...@apache.org> wrote:
>>>>
>>>> I think the _goal_ is to not have the deprecated classes show up in
>>>> IDEs - i.e. we want to discourage people from using them.
>>>>
>>>> On 5 August 2022 09:44:42 BST, "Kamil Breguła" <dz...@gmail.com>
>>>> wrote:
>>>>>
>>>>> I am concerned that the use of dynamic attributes will prevent the IDE
>>>>> from recognizing these deprecated modules and marking them in the IDE.
>>>>> This is what it looks like in my IDE now: https://imgur.com/a/OnRjiKs
>>>>>
>>>>> pt., 5 sie 2022 o 00:28 Ferruzzi, Dennis <fe...@amazon.com.invalid>
>>>>> napisał(a):
>>>>>
>>>>>> > add dynamic attributes to __init__ of airflow.contrib,
>>>>>> airflow.operators, airflow.hooks. (to resolve to provider operators).
>>>>>>
>>>>>> This sounds promising to me.
>>>>>>
>>>>>> ------------------------------
>>>>>> *From:* Jarek Potiuk <ja...@potiuk.com>
>>>>>> *Sent:* Thursday, August 4, 2022 6:52 AM
>>>>>> *To:* dev@airflow.apache.org
>>>>>> *Subject:* RE: [EXTERNAL][DISCUSS] Move "contrib" and all old
>>>>>> classes to a separate package
>>>>>>
>>>>>>
>>>>>> *CAUTION*: This email originated from outside of the organization.
>>>>>> Do not click links or open attachments unless you can confirm the sender
>>>>>> and know the content is safe.
>>>>>>
>>>>>> > This is the issue though. apache-airflow does not depend on
>>>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>>>> apache-airflow package your DAG now breaks.
>>>>>>
>>>>>> Correct. The difference is that by only upgrading apache-airflow
>>>>>> alone you will break it. And I see why this might be seen as "problematic".
>>>>>>
>>>>>> But I have another idea. Why - rather than keeping the deprecations
>>>>>> in the code we load such deprecated we add dynamic attributes to __init__
>>>>>> of airflow.contrib, airflow.operators, airflow.hooks. (to resolve to
>>>>>> provider operators).
>>>>>>
>>>>>> Such dynamic attributes will be invisible to autocompletion, will not
>>>>>> be documented with APIs nor visible in the source code (Except deep
>>>>>> inspection of __init__ in those packages).
>>>>>>
>>>>>> This has no compatibility breaking potential whatsoever
>>>>>>
>>>>>>
>>>>>> J.
>>>>>>
>>>>>>
>>>>>> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org>
>>>>>> wrote:
>>>>>>
>>>>>>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <
>>>>>>> jarek@potiuk.com> wrote:
>>>>>>>
>>>>>>> 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".
>>>>>>>
>>>>>>>
>>>>>>> This is the issue though. apache-airflow does not depend on
>>>>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>>>>> apache-airflow package your DAG now breaks.
>>>>>>>
>>>>>>>
>>>>>>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
And I agree with Ash - two years ago it would be a bad choice but we are
past the time when we should be "gentle" with it :).
But we are now at the point when our priorities shifted - we want now to
'strengthen" our message for anyone still using the old imports.
I also think that we could use this also to "strengthen" our messages to
our users over time (see the little distraction from the other topic we had
in https://lisdiscussion in
ts.apache.org/thread/cp9n8r9x75xzzsdjgdqd82p8nmyn1nd5)
We could (if we want) make each warning generated in this case an
"adjustable" level:
1) log message
2) UI "soft" message
3) UI "annoying" message
4) Error
This could be easily configurable per group of deprecations and we could
change defaults over time and give the opportunity of the person who
manages Airflow to strengthen (or weaken) the message.
I remember a discussion with a user who manages "Airflow farm" in
their company and he argued that it is extremely annoying that his users
still use some "contrib" operators and he has a hard time getting rid of
those even if he really wants to do it to make it a more consistent usage
across his company.
With a configurable level of warnings/errors for deprecations, we do not
break compatibility, we have more impact and can further strengthen our
message in future versions and also give those who manage airflow an
opportunity to even block the "deprecation" usage if they want.
J.
On Fri, Aug 5, 2022 at 1:02 PM Jarek Potiuk <ja...@potiuk.com> wrote:
> Yeah. Removing them from IDE is PRECISELY the goal :).
>
> On Fri, Aug 5, 2022 at 11:01 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>
>> And by removing them/making them dynamic the ide will think they don't
>> even exist anymore.
>>
>> (Also: these have been showing as deprecated in IDEs for ~2 years; If
>> someone was going to update their code they would have already)
>>
>> On 5 August 2022 09:58:38 BST, Ash Berlin-Taylor <as...@apache.org> wrote:
>>>
>>> I think the _goal_ is to not have the deprecated classes show up in IDEs
>>> - i.e. we want to discourage people from using them.
>>>
>>> On 5 August 2022 09:44:42 BST, "Kamil Breguła" <dz...@gmail.com>
>>> wrote:
>>>>
>>>> I am concerned that the use of dynamic attributes will prevent the IDE
>>>> from recognizing these deprecated modules and marking them in the IDE.
>>>> This is what it looks like in my IDE now: https://imgur.com/a/OnRjiKs
>>>>
>>>> pt., 5 sie 2022 o 00:28 Ferruzzi, Dennis <fe...@amazon.com.invalid>
>>>> napisał(a):
>>>>
>>>>> > add dynamic attributes to __init__ of airflow.contrib,
>>>>> airflow.operators, airflow.hooks. (to resolve to provider operators).
>>>>>
>>>>> This sounds promising to me.
>>>>>
>>>>> ------------------------------
>>>>> *From:* Jarek Potiuk <ja...@potiuk.com>
>>>>> *Sent:* Thursday, August 4, 2022 6:52 AM
>>>>> *To:* dev@airflow.apache.org
>>>>> *Subject:* RE: [EXTERNAL][DISCUSS] Move "contrib" and all old classes
>>>>> to a separate package
>>>>>
>>>>>
>>>>> *CAUTION*: This email originated from outside of the organization. Do
>>>>> not click links or open attachments unless you can confirm the sender and
>>>>> know the content is safe.
>>>>>
>>>>> > This is the issue though. apache-airflow does not depend on
>>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>>> apache-airflow package your DAG now breaks.
>>>>>
>>>>> Correct. The difference is that by only upgrading apache-airflow alone
>>>>> you will break it. And I see why this might be seen as "problematic".
>>>>>
>>>>> But I have another idea. Why - rather than keeping the deprecations in
>>>>> the code we load such deprecated we add dynamic attributes to __init__ of
>>>>> airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider
>>>>> operators).
>>>>>
>>>>> Such dynamic attributes will be invisible to autocompletion, will not
>>>>> be documented with APIs nor visible in the source code (Except deep
>>>>> inspection of __init__ in those packages).
>>>>>
>>>>> This has no compatibility breaking potential whatsoever
>>>>>
>>>>>
>>>>> J.
>>>>>
>>>>>
>>>>> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org>
>>>>> wrote:
>>>>>
>>>>>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <
>>>>>> jarek@potiuk.com> wrote:
>>>>>>
>>>>>> 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".
>>>>>>
>>>>>>
>>>>>> This is the issue though. apache-airflow does not depend on
>>>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>>>> apache-airflow package your DAG now breaks.
>>>>>>
>>>>>>
>>>>>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
Yeah. Removing them from IDE is PRECISELY the goal :).
On Fri, Aug 5, 2022 at 11:01 AM Ash Berlin-Taylor <as...@apache.org> wrote:
> And by removing them/making them dynamic the ide will think they don't
> even exist anymore.
>
> (Also: these have been showing as deprecated in IDEs for ~2 years; If
> someone was going to update their code they would have already)
>
> On 5 August 2022 09:58:38 BST, Ash Berlin-Taylor <as...@apache.org> wrote:
>>
>> I think the _goal_ is to not have the deprecated classes show up in IDEs
>> - i.e. we want to discourage people from using them.
>>
>> On 5 August 2022 09:44:42 BST, "Kamil Breguła" <dz...@gmail.com>
>> wrote:
>>>
>>> I am concerned that the use of dynamic attributes will prevent the IDE
>>> from recognizing these deprecated modules and marking them in the IDE.
>>> This is what it looks like in my IDE now: https://imgur.com/a/OnRjiKs
>>>
>>> pt., 5 sie 2022 o 00:28 Ferruzzi, Dennis <fe...@amazon.com.invalid>
>>> napisał(a):
>>>
>>>> > add dynamic attributes to __init__ of airflow.contrib,
>>>> airflow.operators, airflow.hooks. (to resolve to provider operators).
>>>>
>>>> This sounds promising to me.
>>>>
>>>> ------------------------------
>>>> *From:* Jarek Potiuk <ja...@potiuk.com>
>>>> *Sent:* Thursday, August 4, 2022 6:52 AM
>>>> *To:* dev@airflow.apache.org
>>>> *Subject:* RE: [EXTERNAL][DISCUSS] Move "contrib" and all old classes
>>>> to a separate package
>>>>
>>>>
>>>> *CAUTION*: This email originated from outside of the organization. Do
>>>> not click links or open attachments unless you can confirm the sender and
>>>> know the content is safe.
>>>>
>>>> > This is the issue though. apache-airflow does not depend on
>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>> apache-airflow package your DAG now breaks.
>>>>
>>>> Correct. The difference is that by only upgrading apache-airflow alone
>>>> you will break it. And I see why this might be seen as "problematic".
>>>>
>>>> But I have another idea. Why - rather than keeping the deprecations in
>>>> the code we load such deprecated we add dynamic attributes to __init__ of
>>>> airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider
>>>> operators).
>>>>
>>>> Such dynamic attributes will be invisible to autocompletion, will not
>>>> be documented with APIs nor visible in the source code (Except deep
>>>> inspection of __init__ in those packages).
>>>>
>>>> This has no compatibility breaking potential whatsoever
>>>>
>>>>
>>>> J.
>>>>
>>>>
>>>> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org>
>>>> wrote:
>>>>
>>>>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <
>>>>> jarek@potiuk.com> wrote:
>>>>>
>>>>> 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".
>>>>>
>>>>>
>>>>> This is the issue though. apache-airflow does not depend on
>>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>>> apache-airflow package your DAG now breaks.
>>>>>
>>>>>
>>>>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Ash Berlin-Taylor <as...@apache.org>.
And by removing them/making them dynamic the ide will think they don't even exist anymore.
(Also: these have been showing as deprecated in IDEs for ~2 years; If someone was going to update their code they would have already)
On 5 August 2022 09:58:38 BST, Ash Berlin-Taylor <as...@apache.org> wrote:
>I think the _goal_ is to not have the deprecated classes show up in IDEs - i.e. we want to discourage people from using them.
>
>On 5 August 2022 09:44:42 BST, "Kamil Breguła" <dz...@gmail.com> wrote:
>>I am concerned that the use of dynamic attributes will prevent the IDE from
>>recognizing these deprecated modules and marking them in the IDE.
>>This is what it looks like in my IDE now: https://imgur.com/a/OnRjiKs
>>
>>pt., 5 sie 2022 o 00:28 Ferruzzi, Dennis <fe...@amazon.com.invalid>
>>napisał(a):
>>
>>> > add dynamic attributes to __init__ of airflow.contrib,
>>> airflow.operators, airflow.hooks. (to resolve to provider operators).
>>>
>>> This sounds promising to me.
>>>
>>> ------------------------------
>>> *From:* Jarek Potiuk <ja...@potiuk.com>
>>> *Sent:* Thursday, August 4, 2022 6:52 AM
>>> *To:* dev@airflow.apache.org
>>> *Subject:* RE: [EXTERNAL][DISCUSS] Move "contrib" and all old classes to
>>> a separate package
>>>
>>>
>>> *CAUTION*: This email originated from outside of the organization. Do not
>>> click links or open attachments unless you can confirm the sender and know
>>> the content is safe.
>>>
>>> > This is the issue though. apache-airflow does not depend on
>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>> apache-airflow package your DAG now breaks.
>>>
>>> Correct. The difference is that by only upgrading apache-airflow alone you
>>> will break it. And I see why this might be seen as "problematic".
>>>
>>> But I have another idea. Why - rather than keeping the deprecations in the
>>> code we load such deprecated we add dynamic attributes to __init__ of
>>> airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider
>>> operators).
>>>
>>> Such dynamic attributes will be invisible to autocompletion, will not be
>>> documented with APIs nor visible in the source code (Except deep inspection
>>> of __init__ in those packages).
>>>
>>> This has no compatibility breaking potential whatsoever
>>>
>>>
>>> J.
>>>
>>>
>>> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>>>
>>>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <ja...@potiuk.com>
>>>> wrote:
>>>>
>>>> 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".
>>>>
>>>>
>>>> This is the issue though. apache-airflow does not depend on
>>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>>> apache-airflow package your DAG now breaks.
>>>>
>>>>
>>>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Ash Berlin-Taylor <as...@apache.org>.
I think the _goal_ is to not have the deprecated classes show up in IDEs - i.e. we want to discourage people from using them.
On 5 August 2022 09:44:42 BST, "Kamil Breguła" <dz...@gmail.com> wrote:
>I am concerned that the use of dynamic attributes will prevent the IDE from
>recognizing these deprecated modules and marking them in the IDE.
>This is what it looks like in my IDE now: https://imgur.com/a/OnRjiKs
>
>pt., 5 sie 2022 o 00:28 Ferruzzi, Dennis <fe...@amazon.com.invalid>
>napisał(a):
>
>> > add dynamic attributes to __init__ of airflow.contrib,
>> airflow.operators, airflow.hooks. (to resolve to provider operators).
>>
>> This sounds promising to me.
>>
>> ------------------------------
>> *From:* Jarek Potiuk <ja...@potiuk.com>
>> *Sent:* Thursday, August 4, 2022 6:52 AM
>> *To:* dev@airflow.apache.org
>> *Subject:* RE: [EXTERNAL][DISCUSS] Move "contrib" and all old classes to
>> a separate package
>>
>>
>> *CAUTION*: This email originated from outside of the organization. Do not
>> click links or open attachments unless you can confirm the sender and know
>> the content is safe.
>>
>> > This is the issue though. apache-airflow does not depend on
>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>> apache-airflow package your DAG now breaks.
>>
>> Correct. The difference is that by only upgrading apache-airflow alone you
>> will break it. And I see why this might be seen as "problematic".
>>
>> But I have another idea. Why - rather than keeping the deprecations in the
>> code we load such deprecated we add dynamic attributes to __init__ of
>> airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider
>> operators).
>>
>> Such dynamic attributes will be invisible to autocompletion, will not be
>> documented with APIs nor visible in the source code (Except deep inspection
>> of __init__ in those packages).
>>
>> This has no compatibility breaking potential whatsoever
>>
>>
>> J.
>>
>>
>> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>>
>>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <ja...@potiuk.com>
>>> wrote:
>>>
>>> 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".
>>>
>>>
>>> This is the issue though. apache-airflow does not depend on
>>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>>> apache-airflow package your DAG now breaks.
>>>
>>>
>>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Kamil Breguła <dz...@gmail.com>.
I am concerned that the use of dynamic attributes will prevent the IDE from
recognizing these deprecated modules and marking them in the IDE.
This is what it looks like in my IDE now: https://imgur.com/a/OnRjiKs
pt., 5 sie 2022 o 00:28 Ferruzzi, Dennis <fe...@amazon.com.invalid>
napisał(a):
> > add dynamic attributes to __init__ of airflow.contrib,
> airflow.operators, airflow.hooks. (to resolve to provider operators).
>
> This sounds promising to me.
>
> ------------------------------
> *From:* Jarek Potiuk <ja...@potiuk.com>
> *Sent:* Thursday, August 4, 2022 6:52 AM
> *To:* dev@airflow.apache.org
> *Subject:* RE: [EXTERNAL][DISCUSS] Move "contrib" and all old classes to
> a separate package
>
>
> *CAUTION*: This email originated from outside of the organization. Do not
> click links or open attachments unless you can confirm the sender and know
> the content is safe.
>
> > This is the issue though. apache-airflow does not depend on
> apache-airflow-providers-google >= 8.0, so you can still be on the same
> version of the provider on 2.3 and 2.4, but by _only_ upgrading
> apache-airflow package your DAG now breaks.
>
> Correct. The difference is that by only upgrading apache-airflow alone you
> will break it. And I see why this might be seen as "problematic".
>
> But I have another idea. Why - rather than keeping the deprecations in the
> code we load such deprecated we add dynamic attributes to __init__ of
> airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider
> operators).
>
> Such dynamic attributes will be invisible to autocompletion, will not be
> documented with APIs nor visible in the source code (Except deep inspection
> of __init__ in those packages).
>
> This has no compatibility breaking potential whatsoever
>
>
> J.
>
>
> On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>
>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <ja...@potiuk.com>
>> wrote:
>>
>> 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".
>>
>>
>> This is the issue though. apache-airflow does not depend on
>> apache-airflow-providers-google >= 8.0, so you can still be on the same
>> version of the provider on 2.3 and 2.4, but by _only_ upgrading
>> apache-airflow package your DAG now breaks.
>>
>>
>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by "Ferruzzi, Dennis" <fe...@amazon.com.INVALID>.
> add dynamic attributes to __init__ of airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider operators).
This sounds promising to me.
________________________________
From: Jarek Potiuk <ja...@potiuk.com>
Sent: Thursday, August 4, 2022 6:52 AM
To: dev@airflow.apache.org
Subject: RE: [EXTERNAL][DISCUSS] Move "contrib" and all old classes to a separate package
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> This is the issue though. apache-airflow does not depend on apache-airflow-providers-google >= 8.0, so you can still be on the same version of the provider on 2.3 and 2.4, but by _only_ upgrading apache-airflow package your DAG now breaks.
Correct. The difference is that by only upgrading apache-airflow alone you will break it. And I see why this might be seen as "problematic".
But I have another idea. Why - rather than keeping the deprecations in the code we load such deprecated we add dynamic attributes to __init__ of airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider operators).
Such dynamic attributes will be invisible to autocompletion, will not be documented with APIs nor visible in the source code (Except deep inspection of __init__ in those packages).
This has no compatibility breaking potential whatsoever
J.
On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org>> wrote:
On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <ja...@potiuk.com>> wrote:
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".
This is the issue though. apache-airflow does not depend on apache-airflow-providers-google >= 8.0, so you can still be on the same version of the provider on 2.3 and 2.4, but by _only_ upgrading apache-airflow package your DAG now breaks.
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
> This is the issue though. apache-airflow does not depend on
apache-airflow-providers-google >= 8.0, so you can still be on the same
version of the provider on 2.3 and 2.4, but by _only_ upgrading
apache-airflow package your DAG now breaks.
Correct. The difference is that by only upgrading apache-airflow alone you
will break it. And I see why this might be seen as "problematic".
But I have another idea. Why - rather than keeping the deprecations in the
code we load such deprecated we add dynamic attributes to __init__ of
airflow.contrib, airflow.operators, airflow.hooks. (to resolve to provider
operators).
Such dynamic attributes will be invisible to autocompletion, will not be
documented with APIs nor visible in the source code (Except deep inspection
of __init__ in those packages).
This has no compatibility breaking potential whatsoever
J.
On Thu, Aug 4, 2022 at 3:47 PM Ash Berlin-Taylor <as...@apache.org> wrote:
> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk <ja...@potiuk.com>
> wrote:
>
> 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".
>
>
> This is the issue though. apache-airflow does not depend on
> apache-airflow-providers-google >= 8.0, so you can still be on the same
> version of the provider on 2.3 and 2.4, but by _only_ upgrading
> apache-airflow package your DAG now breaks.
>
>
>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Ash Berlin-Taylor <as...@apache.org>.
On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk
<ja...@potiuk.com> wrote:
> 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".
This is the issue though. apache-airflow does not depend on
apache-airflow-providers-google >= 8.0, so you can still be on the same
version of the provider on 2.3 and 2.4, but by _only_ upgrading
apache-airflow package your DAG now breaks.
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
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 <ba...@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 <el...@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 <as...@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 <
>>>> ash@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 <
>>>> jarek@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.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Bas Harenslak <ba...@astronomer.io.INVALID>.
> 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 <el...@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 <jarek@potiuk.com <ma...@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 <jarek@potiuk.com <ma...@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 <ash@apache.org <ma...@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 <ash@apache.org <ma...@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 <jarek@potiuk.com <ma...@potiuk.com>> wrote:
>>> Hello everyone,
>>>
>>> Following the discussion in https://github.com/apache/airflow/pull/25413 <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.
>>>
>>>
>>>
>>>
>>>
>>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Elad Kalif <el...@apache.org>.
> 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 <as...@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 <
>>> ash@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.
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
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 <as...@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 <
>> ash@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.
>>
>>
>>
>>
>>
>>
>>
>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Jarek Potiuk <ja...@potiuk.com>.
Not if you add install `airflow[contrib]` - then it won't break.
On Thu, Aug 4, 2022 at 12:21 PM Ash Berlin-Taylor <as...@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 <
> ash@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.
>
>
>
>
>
>
>
>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Ash Berlin-Taylor <as...@apache.org>.
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
<as...@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.
>>
>>
>>
>>
>>
>>
Re: [DISCUSS] Move "contrib" and all old classes to a separate package
Posted by Ash Berlin-Taylor <as...@apache.org>.
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.
>
>
>
>
>
>