Hi Pavan, Cool AIP! I posted a discussion yesterday (https://github.com/apache/airflow/discussions/42645) to get feedback on a Periodic[Callback]Trigger that would run callbacks recurrently with custom sensors in mind and thought it could be expanded into essentially what you've envisioned with a "deferrable base sensor".
Personally, I'm quite interested in supporting a rich fixed time or backoff rate configurable behavior so that custom sensors may benefit from enabling deferrable while not having to implement triggers and only configuring the "wait" strategy. A few opinions on the proposal: - In my code test ( https://github.com/Alexhans/airflow/commit/57a0383d8a2e3a4332c79bcac3df100bde777472), because I didn't really need the context, I used the same serialization tool that is used for the classpath: `from airflow.utils.module_loading import import_string` but I understand the base sensor usecase would need the context. Is it possible that the serialization logic already in `airflow.utils.context.py::Context.__reduce_ex__` ? - A consistent behavior where every operator implements deferrable=True/False seems desirable to me. Having just one sensor (The same way operators do it, see AWS provided operators or my code) makes it easier to reason about and code changes for the user would be just at an argument level. I know mode=reschedule|poke seems to conflict with deferrable=True but limiting the incompatible combinations should be simple enough. @Blain David: Thanks for that msgraph example. The usage of TimeDeltaTrigger in combination with retry_execute is very elegant. I'm happy to help in any way. Best, Alex On 2024/09/27 07:51:51 Pavankumar Gopidesu wrote: > Thank you David, Yes this is not to block any workers. Looking for more > feedback. appreciate your time. > > Regards, > Pavan > > On Thu, Sep 26, 2024, 11:26 Blain David <da...@infrabel.be> wrote: > > > Hello Pavan Kumar, > > > > I really like your proposition as this would have facilitated my > > implementation of the MSGraphSensor, which is actually implemented as a > > deferrable using a triggerer. > > > > You can check here how I did it: > > https://github.com/apache/airflow/blob/main/airflow/providers/microsoft/azure/sensors/msgraph.py > > > > The idea was the same as in your proposition to not block workers when > > using a sensor by implementing it as a deferrable. > > > > Kind regards, > > David > > > > -----Original Message----- > > From: Pavankumar Gopidesu <go...@gmail.com> > > Sent: Thursday, September 26, 2024 11:01 AM > > To: dev@airflow.apache.org > > Subject: Re: [DISCUSS] Sensor Improvements With Tirggers > > > > EXTERNAL MAIL: Indien je de afzender van deze e-mail niet kent en deze > > niet vertrouwt, klik niet op een link of open geen bijlages. Bij twijfel, > > stuur deze e-mail als bijlage naar ab...@infrabel.be<mailto: > > ab...@infrabel.be>. > > > > Hello Everyone, > > > > After discussion on the slack channel, I have decided to retain the > > current synchronous sensor to offer users the choice to execute sensors on > > workers. > > I have revised the proposal to introduce a new asynchronous version of the > > sensor, eliminating the need for workers to run sensors. Additional details > > have been included below. Your review and feedback are greatly appreciated. > > I am eager to hear from you. > > > > > > https://cwiki.apache.org/confluence/display/AIRFLOW/AIP+87%3A+Run+Sensors+in+Triggerer+with+AsyncBaseSensor > > > > > > > > Regards, > > Pavan Kumar > > > > > > On Wed, Aug 14, 2024 at 11:48 PM Pavankumar Gopidesu < > > gopidesupa...@gmail.com> wrote: > > > > > Hi Wei Lee, > > > > > > Thanks for the review, While I was working on the POC , I had a bit of > > > confusion about how to use the logic present inside the sensor execute > > > method. for both with and without triggerer flow. > > > so to make it work for both flows, I have moved out to execute logic > > > with two methods. > > > > > > I appreciate the idea of a simple helper function, I'm eager to hear > > > any suggestions you might have or anyone :). > > > > > > Regards, > > > Pavan Kumar > > > > > > On Wed, Aug 14, 2024 at 12:16 PM Wei Lee <we...@gmail.com> wrote: > > > > > >> Thank you for bringing this up! I have added some comments to the > > >> document. I'm unsure if we really want or need to implement more > > >> complex logic for this. What I have in mind is simply adding helper > > >> functions to InternalSensorTrigger and continuing to use the run method > > in BaseTrigger. > > >> The main purpose of those methods is to allow operator authors to > > >> change the sensors to leverage this start/end from trigger more easily. > > >> > > >> Best, > > >> Wei > > >> > > >> > On Aug 14, 2024, at 5:19 PM, Pavankumar Gopidesu < > > >> gopidesupa...@gmail.com> wrote: > > >> > > > >> > Hi Jarek, > > >> > > > >> > Thanks for the questions :) , > > >> > > > >> > I completely agree with you , from 2.10 we have the > > >> > start_from_trigger parameter, which , when set , allows a task to > > >> > be executed directly from the triggerrer without worker > > >> > involvement. I believe that for any sensor to be executed in the > > >> > triggerer, a corresponding trigger implementation must be in place. > > >> > Good thing is , most of the required trigger implementations are > > >> > already available in current airflow providers. > > >> > > > >> > Additionally, i agree that there is no real difference between a > > >> deferrable > > >> > sensor and a deferrable operator, > > >> > generally , users utilize the BaseSensorOperator to create custom > > >> sensors, > > >> > However if they want to run > > >> > these sensors in triggerer, they must implement the triggerer's run > > >> method > > >> > by extending the base trigger class. > > >> > > > >> > The key difference with this proposal is the introduction of a > > >> > common trigger implementation (referred to in this POC as > > >> > InternalSensorTrigger )[1]. This would allow users to use the new > > >> > feature start_from_trigger param with their custom sensor and > > >> > eliminates the need for individual trigger implementations for each > > >> > custom sensor when they create. > > >> > > > >> > Alternatively, we could leave the start_from_trigger parameter in > > >> > BaseSensorOperator with a default value as false and let users > > >> > decide whether they want to run the sensor > > >> in > > >> > triggerrer. If they choose to do so, they can simply set the > > >> > parameter to true and triggerrer uses the InternalSensorTrigger > > >> > class. > > >> > > > >> > Apologies if my answer is too confusing :) > > >> > > > >> > [1]: > > >> > > > >> https://git/ > > >> hub.com%2Fapache%2Fairflow%2Fpull%2F41355%2Ffiles%23diff-7486f32e385d > > >> 7ad0376cccda08d80e54939aa901a24616d11fb1f5cba6af7f83R144&data=05%7C02 > > >> %7Cdavid.blain%40infrabel.be%7Ce13a7cc2a5fd4807701e08dcde09ebc7%7Cb82 > > >> bc314ab8e4d6fb18946f02e1f27f2%7C0%7C0%7C638629381398366280%7CUnknown% > > >> 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ > > >> XVCI6Mn0%3D%7C0%7C%7C%7C&sdata=3wwm43IOv4KUwciQatVOM5CKRmwYn4HBsiBZEM > > >> p2jvI%3D&reserved=0 > > >> > > > >> > Regards, > > >> > Pavan Kumar > > >> > > > >> > On Wed, Aug 14, 2024 at 12:29 AM Jarek Potiuk <ja...@potiuk.com> > > wrote: > > >> > > > >> >> How does it differ from the upcoming 2.10 feature? > > >> >> > > >> >> * Deferrable operators can now execute directly from the triggerer > > >> without > > >> >> needing to go through the worker. This is especially efficient for > > >> certain > > >> >> operators, like sensors, and can help teams save both time and money. > > >> >> > > >> >> As of 2.10 - Sensors already can run mostly in Triggerrer and > > >> >> basically there is no big difference any more between deferrable > > >> >> sensor and deferrable operator. > > >> >> > > >> >> > > >> >> On Mon, Aug 12, 2024 at 3:59 AM Kaxil Naik <ka...@gmail.com> > > >> wrote: > > >> >> > > >> >>> Thanks for putting this together, I will take a look this week. > > >> >>> > > >> >>> On Fri, 9 Aug 2024 at 13:12, Pavankumar Gopidesu < > > >> >> gopidesupa...@gmail.com> > > >> >>> wrote: > > >> >>> > > >> >>>> Hi All, > > >> >>>> > > >> >>>> I have created a draft document for Sensor Improvements using > > >> triggers. > > >> >>>> > > >> >>>> Details: > > >> >>>> > > >> >>>> > > >> >>> > > >> >> > > >> https://doc/ > > >> s.google.com%2Fdocument%2Fd%2F1Kb_wL-T1DHkOpmR_QNa3O5p_2hMTLzM-sb_HzQ > > >> 0PYCo%2Fedit%3Fusp%3Dsharing&data=05%7C02%7Cdavid.blain%40infrabel.be > > >> %7Ce13a7cc2a5fd4807701e08dcde09ebc7%7Cb82bc314ab8e4d6fb18946f02e1f27f > > >> 2%7C0%7C0%7C638629381398384359%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj > > >> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sda > > >> ta=ve%2FbzO8Vqe8K5yKdMVVsnAmD6zPl0gh7WyBrO6IeKjw%3D&reserved=0 > > >> >>>> > > >> >>>> POC: > > >> >>>> > > >> >>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%25 > > >> >>>> 2Fgithub.com%2Fapache%2Fairflow%2Fpull%2F41355&data=05%7C02%7Cda > > >> >>>> vid.blain%40infrabel.be%7Ce13a7cc2a5fd4807701e08dcde09ebc7%7Cb82 > > >> >>>> bc314ab8e4d6fb18946f02e1f27f2%7C0%7C0%7C638629381398402033%7CUnk > > >> >>>> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I > > >> >>>> k1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=sYLKcAYQsysHC2%2FPsyV9m > > >> >>>> Ze%2BWu%2BL2HyJ0SlPulVIvEg%3D&reserved=0 > > >> >>>> > > >> >>>> This is my first draft post, apologies if any mistakes in the > > >> >> document. I > > >> >>>> would greatly appreciate your insights and suggestions on draft. > > >> >>>> > > >> >>>> Thank you very much for your time. > > >> >>>> > > >> >>>> Regards, > > >> >>>> Pavan > > >> >>>> > > >> >>> > > >> >> > > >> > > >> > > >> --------------------------------------------------------------------- > > >> To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org > > >> For additional commands, e-mail: dev-h...@airflow.apache.org > > >> > > >> > > >
