I'll just +1 that IMO a UI reminder is a great option. 2.3.0 added an alert when robots.txt was being scanned, which was great, because I ignored it for a while, but eventually caved and looked into it. This I would see as more of the same. I currently get loads of warnings about things in my config file that have moved; but typically only if I have to do a `airflow db upgrade` or `airflow dags backfill`. I see the alert, think "I must fix those", and then the Homer effect kicks in, and something else takes up the space in my brain. Having them as a banner in the GUI would 100% get me to sort it sooner or later.
On Thu, Aug 4, 2022 at 9:32 PM Vikram Koka <vik...@astronomer.io.invalid> wrote: > Hi Daniel, > > I completely agree with wanting to consolidate, but I am very concerned > about people needing to change their existing code. > As Ping and Felix also said, I worry that people ignore deprecation > warnings, especially since the original code authors don't really look at > logs or other pipeline artifacts unless something is broken. > > With respect to the options you outlined, I am personally leaning towards > (4) i.e. "don't deprecate timetable, but remove schedule_on (AIP-48 has > added it in main) and take List[Dataset] in `schedule_interval`". I could > be convinced with (3), since I think that "timetable" is relatively recent > and have not seen as much usage for this invocation yet. I am definitely > curious about other perspectives on this as well. > > I have a hard time with option (1) of deprecating "schedule interval", > because of the historical nature of it and the code changes that people may > have to make to deal with it. > > Thanks for raising this topic and the discussion, > > Vikram > > > On Thu, Aug 4, 2022 at 10:24 AM Daniel Standish > <daniel.stand...@astronomer.io.invalid> wrote: > >> I agree with those (including myself) who have suggested that it would be >> great to have a deprecation fix tool. >> >> I also think that UI notifications would be valuable. Another option >> would be to provide a less intrusive indicator somewhere (like when chrome >> lets you know you need to update), and then you click on it and it takes >> you to a page that lists out all the deprecation warnings. >> >> >> On Thu, Aug 4, 2022 at 1:03 AM Jarek Potiuk <ja...@potiuk.com> wrote: >> >>> I know we are distracting a bit from the main subject, but I think it's >>> worth it. >>> >>> I think this is a fact of life that people don't remove deprecations. >>> But we have a choice of "not doing anything about it" and complaining. Or >>> doing something that might improve it. >>> >>> I usually err on the side of "doing something" when I see a problem. I >>> am a doer :). >>> >>> And I have a possibly controversial proposal. I got the inspiration from >>> node.js old versions. Whenever an old version of Node is being used, you >>> have a 20 seconds delay artificially added and information that in order to >>> remove those 20 seconds you need to upgrade. >>> >>> Why don't we do that for some really old/serious deprecations ? Maybe >>> not 20 seconds. Maybe adding an unremovable RED warning in the UI if you >>> have any deprecation unhandled. Something that is annoying enough that >>> will cause people to make an action but not intrusive enough to break >>> things. >>> >>> I think this is a very basic psychological trait: >>> >>> If it does not cost the user anything to leave the deprecations, there >>> will be a big number of people who will do that. >>> We need to make sure that deprecations are costing our users "a little" >>> and that they are aware that they can remove the cost (and know how - this >>> is super important that we only show such a message with clear instructions >>> on what to do). >>> >>> Such a message might be annoying in many ways: >>> >>> * it can obstruct significant part of the screen >>> * It can be flashy and had very annoying colours (think bold pink fonts >>> on flash-green background for example) >>> * It can not allow to do anything before the user closes it with a >>> REALLY small 'x' that it is difficult to point at (think cookie banners). >>> And when they click they can be greeted with "You know - you can actually >>> remove this warning by doing THIS". >>> * and we have probably more options - our UX specialists might have some >>> good ideas here >>> >>> For anyone who complains about "people do not act on deprecation." - >>> yeah this is true. >>> But one universal truth there is that if we won't act, this will change >>> nothing in the behaviour of our users - this is 100% certain. >>> >>> If we do something - it might improve it. We will not get a 100% success >>> for sure, but we can at least do something to improve it. >>> >>> WDYT? >>> >>> J. >>> >>> >>> On Thu, Aug 4, 2022 at 6:52 AM Felix Uellendall <felue...@pm.me.invalid> >>> wrote: >>> >>>> Hi Ping, >>>> >>>> I had this idea about auto-fixing deprecation warnings too when I >>>> started the airflint project. >>>> I totally agree with what you said that often people ignore these >>>> warnings and therefore a cli tool would come in very handy. It could look >>>> similar to the airflint project and also be based on the python refactor >>>> package, but I would keep it out of this discussion for now. >>>> >>>> Regarding the schedule arg, I don’t have a strong opinion there but >>>> also tend to having one schedule arg, because of simplicity for the >>>> dag author. >>>> >>>> Best, >>>> Felix >>>> >>>> >>>> >>>> Sent from Proton Mail for iOS >>>> >>>> >>>> On Thu, Aug 4, 2022 at 00:23, Ping Zhang <pin...@umich.edu> wrote: >>>> >>>> Hi Daniel, >>>> >>>> +1 for ‘schedule' and thanks for bringing this up. >>>> >>>> I agree with Vikram, we should be very careful about deprecating >>>> existing params even if we have warnings around it. Not sure if this is a >>>> general case, but I notice that people ignore warnings all the time until >>>> it starts to break. Thus I am thinking if airflow can have a cli tool to >>>> help users to batch update these deprecating params just like python's >>>> futurize tool. This can greatly ease the burden of upgrading the airflow >>>> version. >>>> >>>> >>>> Thanks, >>>> >>>> Ping >>>> >>>> >>>> On Wed, Aug 3, 2022 at 3:13 PM Daniel Standish >>>> <daniel.stand...@astronomer.io.invalid> wrote: >>>> >>>>> In case you forgot about us @Vikram, gentle nudge >>>>> >>>>> On Mon, Aug 1, 2022 at 8:40 AM Daniel Standish < >>>>> daniel.stand...@astronomer.io> wrote: >>>>> >>>>>> Well, this is a discussion thread, so let's discuss! >>>>>> >>>>>> Vikram, what kind of implications are you thinking of? Maybe you >>>>>> could provide a little more detail about your concerns? >>>>>> >>>>>> There are certainly alternatives, and of course each of them has >>>>>> tradeoffs. >>>>>> >>>>>> Here are the options I see: >>>>>> >>>>>> 1. consolidate to schedule (deprecate schedule_interval and >>>>>> timetable; the proposal under discussion in this thread) >>>>>> 2. do nothing (keep 3 different scheduling params) >>>>>> 3. consolidate to schedule_interval (deprecate timetable) >>>>>> 4. don't deprecate timetable, but remove schedule_on (AIP-48 has >>>>>> added it in main) and take List[Dataset] in `schedule_interval` >>>>>> >>>>>> Which would you propose we go with? >>>>>> >>>>>> If we have a veto against 1 (and again, we're in hypothetical >>>>>> territory since this is still a discussion thread), my vote would >>>>>> definitely be to go with 3. >>>>>> >>>>>> The param `schedule_interval` as catch all is not *sooo *bad, but >>>>>> `schedule` definitely seems better. From a new user's perspective, having >>>>>> misleading names is confusing, and can be a negative when looking at >>>>>> adoption. And for a tenured user, it can nag at you. And dealing with >>>>>> deprecations as an Airflow user, well it's sort of part of the job, >>>>>> though >>>>>> it can be bothersome, certainly. >>>>>> >>>>>> Just to look at / compare feasibility. >>>>>> >>>>>> Deprecating schedule_interval, it would be more prominent than some >>>>>> other deprecations because it's a DAG param. That said, I think it's >>>>>> pretty >>>>>> straightforward to handle. For one, it's at the cardinality of DAG and >>>>>> not >>>>>> *task*, so there is less to change with than some other >>>>>> deprecations. And being kwargs-only, it's an easy find and replace that >>>>>> could be automated with an `upgrade-check` script. It's very easy to >>>>>> detect, either to warn about, or ultimately, after upgrade, to fail the >>>>>> DAG >>>>>> parse (which forces user to fix). Perhaps we could even experiment with a >>>>>> deprecation fix tool, so that users could get help fixing deprecations, >>>>>> not >>>>>> necessarily in the context of an upgrade, since we're not looking at 3.0 >>>>>> any time soon. >>>>>> >>>>>> We could compare with execution_date, if execution_date is to be >>>>>> removed in 3.0. I think the deprecation of execution_date will be a more >>>>>> difficult thing for users to handle. For one, potentially higher >>>>>> cardinality, since it is used in tasks. But also, the use of >>>>>> execution_date >>>>>> can be harder to detect, since it can be in templates and elsewhere in >>>>>> custom operators. *(With schedule_interval, since it's a top-level >>>>>> dag kwarg, it will be easier.)* And execution_date was replaced with >>>>>> `logical_date` for similar reasons -- avoiding user confusion. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> On Fri, Jul 29, 2022 at 2:42 PM Vikram Koka >>>>>> <vik...@astronomer.io.invalid> wrote: >>>>>> >>>>>>> -1 from me. >>>>>>> >>>>>>> Though I agree in principle with the idea of consolidation, I don't >>>>>>> think we should be doing this yet until we understand the implications >>>>>>> completely. >>>>>>> I am really not in favor of deprecation of the existing params, >>>>>>> unless there is really no alternative. >>>>>>> >>>>>>> >>>>>>> On Fri, Jul 29, 2022 at 2:37 PM Daniel Standish >>>>>>> <daniel.stand...@astronomer.io.invalid> wrote: >>>>>>> >>>>>>>> So far, seems all in favor. >>>>>>>> >>>>>>>> I will just highlight, in case it's not clear.... when we release >>>>>>>> this (presumably 2.4), basically every single dag in existence will >>>>>>>> start >>>>>>>> emitting deprecation warnings, and prior to 3.0, basically every dag in >>>>>>>> existence will need to be updated. >>>>>>>> >>>>>>>> Thankfully, for most people, this should be an easy update since >>>>>>>> DAG is kwargs only, IIRC. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Jul 29, 2022, 2:27 PM Brent Bovenzi >>>>>>>> <br...@astronomer.io.invalid> wrote: >>>>>>>> >>>>>>>>> +1 to consolidating and "schedule". >>>>>>>>> >>>>>>>>> On Fri, Jul 29, 2022, 10:12 PM Drew Hubl >>>>>>>>> <drew.h...@astronomer.io.invalid> wrote: >>>>>>>>> >>>>>>>>>> +1 for ‘schedule', and another +1 to the importance of >>>>>>>>>> consolidating to one being more important than the name of that one >>>>>>>>>> >>>>>>>>>> On Jul 29, 2022, at 1:58 PM, Josh Fell < >>>>>>>>>> josh.d.f...@astronomer.io.INVALID> wrote: >>>>>>>>>> >>>>>>>>>> Consolidating to a single scheduling parameter is a big +1 from >>>>>>>>>> me. `schedule` seems like a nice catch-all. >>>>>>>>>> >>>>>>>>>> On Fri, Jul 29, 2022 at 9:10 AM Jed Cunningham < >>>>>>>>>> jedcunning...@apache.org> wrote: >>>>>>>>>> >>>>>>>>>>> +1 on moving to a single `schedule` param. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>>
