That's a good point Daniel but I think it's a bit more nuanced,

The reschedule mode in community providers serves a purpose for Airflow 2.
If a deployment in Airflow 2 does not have Triggerer - then deferrable
version of the sensor will not work for it.
Triggerer was really optional in Airflow 2. I bet a number of people who
migrated from Airflow 2.1 or 2.2 do not still have Triggerer running.

We have not discussed it but I guess for Airflow 3 we should make Triggerer
mandatory - which - if we do it - indeed makes the "reschedule" option for
our providers redundant.

And yes - I think it is valuable to remove rescheduler options for the
reasons Kaxil mentioned.

But IMHO we should only do that when min-airflow-version >= 3.0 (i.e. ~
October 2025), otherwise we are taking away promised provider features for
Airflow 2 users.


J.

On Wed, Nov 13, 2024 at 10:38 PM Kaxil Naik <kaxiln...@gmail.com> wrote:

> Yeah, as a maintainer it is pain to have to maintain both sync and async
> version of a hook for a Sensor in providers
>
> On Wed, 13 Nov 2024 at 20:42, Daniel Standish
> <daniel.stand...@astronomer.io.invalid> wrote:
>
> > Yeah everything is related. I'm just saying for the purpose of picking a
> > path forward i'm just trying to clarify the options.
> >
> > If we can rule out any of the options I presented we'll have made some
> > progress.
> >
> > Do you have an opinion on any of those?
> >
> > I would probably lean towards keep reschedule interface, keep it on base
> > sensor, remove it from providers.
> >
> >
> >
> > On Wed, Nov 13, 2024 at 12:37 PM Kaxil Naik <kaxiln...@gmail.com> wrote:
> >
> > > > I think we need to separate what to do about the sensors we have in
> > > > providers from the core interface, essentially.
> > >
> > >
> > > I think they are interconnected, especially if one of the goals is to
> > have
> > > the "most performant" option (deferral) as the default or the only
> option
> > > for the users, regardless of the internal.
> > >
> > > From the user's POV, they can care about performance and visibility
> > > ("rescheduled"/"deferral" as TI states).
> > >
> > > The secondary issue is what you are mentioning about the implementation
> > of
> > > "reschedule" method and what to do when reschedule happens, IIRC none
> of
> > > the Operators in the "official" providers do anything special then
> > > inheriting BaseSensorOperator.
> > >
> > > 1. remove the rescheduling interface in core.  No more "up for
> > reschedule"
> > > > state etc.  No more reschedule mode in base sensor. No more
> reschedule
> > > mode
> > > > in any derivative sensor.
> > > > 2. keep rescheduling in core. remove from base sensor and
> derivatives.
> > > > 3. keep reschedule in core. keep in base sensor. remove from all
> > > > derivatives. ban the reintroduction of reschedule mode in derivative
> > > > sensors.
> > >
> > >
> > >
> > >
> > > On Wed, 13 Nov 2024 at 19:46, Ash Berlin-Taylor <a...@apache.org>
> wrote:
> > >
> > > > > Reschedulability in airflow (making up a word here) to my knowledge
> > > only
> > > > exists to support reschedule mode in sensors.
> > > >
> > > > Yes, this is true.
> > > >
> > > >
> > > > On 13 November 2024 19:32:23 GMT, Daniel Standish
> > > > <daniel.stand...@astronomer.io.INVALID> wrote:
> > > > >The options are a bit muddled here.
> > > > >
> > > > >I think we need to separate what to do about the sensors we have in
> > > > >providers from the core interface, essentially.
> > > > >
> > > > >Reschedulability in airflow (making up a word here) to my knowledge
> > only
> > > > >exists to support reschedule mode in sensors.
> > > > >
> > > > >Let's also separate poke from reschedule.
> > > > >
> > > > >For now, let's just look at reschedule.
> > > > >
> > > > >Let me enumerate some paths as i see them.
> > > > >
> > > > >1. remove the rescheduling interface in core.  No more "up for
> > > reschedule"
> > > > >state etc.  No more reschedule mode in base sensor. No more
> reschedule
> > > > mode
> > > > >in any derivative sensor.
> > > > >2. keep rescheduling in core. remove from base sensor and
> derivatives.
> > > > >3. keep reschedule in core. keep in base sensor. remove from all
> > > > >derivatives. ban the reintroduction of reschedule mode in derivative
> > > > >sensors.
> > > >
> > >
> >
>

Reply via email to