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 <[email protected]> wrote: > https://lists.apache.org/thread/cp9n8r9x75xzzsdjgdqd82p8nmyn1nd5 -> > non-broken link here. > > On Fri, Aug 5, 2022 at 1:15 PM Jarek Potiuk <[email protected]> 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 <[email protected]> wrote: >> >>> Yeah. Removing them from IDE is PRECISELY the goal :). >>> >>> On Fri, Aug 5, 2022 at 11:01 AM Ash Berlin-Taylor <[email protected]> >>> 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 <[email protected]> >>>> 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" <[email protected]> >>>>> 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 <[email protected]> >>>>>> 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 <[email protected]> >>>>>>> *Sent:* Thursday, August 4, 2022 6:52 AM >>>>>>> *To:* [email protected] >>>>>>> *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 <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> On Thu, Aug 4 2022 at 13:59:23 +02:00:00, Jarek Potiuk < >>>>>>>> [email protected]> 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. >>>>>>>> >>>>>>>> >>>>>>>>
