Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-04-05 Thread Kaxil Naik
@tamara : Correct, we are now proposing to remove overriding the pre/post
execute

>Quick question if I am understanding the proposed change correctly.


What you want to remove is overriding the pre/post execute when creating
> custom operators:
>
> class MyOperator(BaseOperator):
>
> ...
>
> def pre_execute(self, context):   # This would break?
> 
> def post_execute(self, context):   # And this as well?
> But keep the (currently experimental) use of the pre_execute and
> post_execute parameters (I've only used post_execute before for similar
> reasons as TP posted, interacting with outlets assets)


On Sat, 29 Mar 2025 at 18:23, Kaxil Naik  wrote:

> No, @run_if / @skip_if uses pre_execute from task argument [1] not the the
> method and is just a syntactic sugar. You can also do the following as an
> example:
>
> ```
> def skip_at_random(context):
> if randint(0, 1) == 0:
> raise AirflowSkipException()
>
> t2 = BashOperator(task_id='conditional2', pre_execute=skip_at_random,
> dag=dag, bash_command="airflow version")
> ```
>
> [1]:
> https://github.com/apache/airflow/blob/8c3a30e3ffc3f114c1d2cc3e6e109f4d9e29ca8b/airflow-core/src/airflow/decorators/condition.py#L57-L59
>
> On Sat, 29 Mar 2025 at 05:09, Matthew Block 
> wrote:
>
>> Would this also break @run_if/@skip_if decorators?
>>
>> Best,
>> Matt Block
>>
>> > On Mar 28, 2025, at 3:44 PM, Tamara Fingerlin
>>  wrote:
>> >
>> > Hey :)
>> >
>> > Quick question if I am understanding the proposed change correctly.
>> >
>> > What you want to remove is overriding the pre/post execute when creating
>> > custom operators:
>> >
>> > class MyOperator(BaseOperator):
>> >...
>> >def pre_execute(self, context):   # This would break?
>> >
>> >
>> >def post_execute(self, context):   # And this as well?
>> >
>> >
>> > But keep the (currently experimental) use of the pre_execute and
>> > post_execute parameters (I've only used post_execute before for similar
>> > reasons as TP posted, interacting with outlets assets)
>> >
>> >BashOperator(
>> >task_id='hello_world',
>> >bash_command='sleep 5',
>> >pre_execute=lambda context: print("Pre-execute function
>> called!"),
>> > # this would still work?
>> >post_execute=lambda context: print("Post-execute function
>> > called!"),   # this would be supposed to still work (it does not rn 😅)
>> >)
>> >
>> > The one situation I am worried about here is larger teams/orgs using pre
>> > and post execute to standardize custom operators. For example team A
>> writes
>> > OurCompanyDatabaseBaseOperator that has a pre_execute and post_execute
>> with
>> > mandatory business logic and team B is allowed to write custom
>> operators on
>> > top of that but only override execute.
>> > I assume this would break?
>> >
>> >
>> >> On Fri, Mar 28, 2025 at 9:32 PM Jarek Potiuk  wrote:
>> >>
>> >> I think the current proposal is to remove the pre/post in the Base
>> Operator
>> >> class (and overridability) and leave passing pre/post as constructor
>> >> arguments..
>> >>
>> >>> On Fri, Mar 28, 2025 at 9:20 PM Bolke de Bruin 
>> wrote:
>> >>>
>> >>> Just one thing - the pre / post mechanisms are executed in-process of
>> the
>> >>> task rather than the DAG. So they are not equal to setup/teardown. Are
>> >> you
>> >>> proposing to remove the argument or the whole thing?
>> >>>
>> >>> B.
>> >>>
>> >>>
>> >>>
>>  On Fri, 28 Mar 2025 at 20:58, Jarek Potiuk  wrote:
>> >>>
>>  Indeed. Post/pre overriding in sub-classes should go away (and we
>> could
>>  even likely implement a ruff rule to auto-fix those if someone has a
>> >>> custom
>>  executor. Sounds like 100% doable
>> 
>>  But passing them as "cross-cutting concerns" via callable in a
>> >>> constructor
>>  is pretty useful and not easily fixable for back-compatibilty
>> 
>>  J.
>> 
>>  On Fri, Mar 28, 2025 at 6:14 PM Kaxil Naik 
>> >> wrote:
>> 
>> >> I think the ability of overriding pre_execute and post_execute in a
>> > subclass can definitely go away. They are practically useles; you
>> can
>>  just
>> > put everything in execute, which always needs to exist in a
>> >>> BaseOperator
>> > subclass anyway.
>> >
>> > Yeah I am fine with removing that then. Anyone disagrees?
>> >
>> > On Fri, 28 Mar 2025 at 20:36, Michał Modras <
>> michalmod...@google.com
>> > .invalid>
>> > wrote:
>> >
>> >> I'd prefer a world without separate pre_execute and post_execute
>> > functions
>> >> - as pointed out in the PR, they make reasoning about DAGs more
>>  complex,
>> >> and can be error prone.
>> >>
>> >> Having said that, I know there are multiple users relying on these
>> >> functionalities, so I'll bring up my usual - another breaking
>> >> change
>> >>> -
>> >> another obstacle to the AF3 adoption argument.
>> >>
>> >> And as for relying on operators vs. PythonOpe

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-04-04 Thread Jarek Potiuk
Yeah. I am convinced by Bolke's and Constance posts. Changing my vote to
-0.9.

On Sun, Mar 30, 2025 at 8:25 PM Constance Martineau
 wrote:

> Just to add to the discussion:
>
> Removing these overrides will disproportionately impact teams at companies
> with more than a handful of Airflow users. Unless there’s a clear benefit
> to removing them *and* a well-defined migration path, I don’t think it’s
> worth the disruption. +1 to Bolke’s points.
>
> Like Tamara mentioned, we frequently see teams build internal providers
> where they subclass standard operators and use pre_execute/post_execute to
> inject things like service-specific auth, tracking identifiers, or other
> cross-cutting concerns. These hooks act as a central entry point for
> operational consistency, especially in environments where DAG authors
> aren’t expected to manage those behaviors themselves.
>
> Pushing that logic down to every DAG or task definition would be a pretty
> big shift for those teams, and, from what I’ve seen, a regression in terms
> of maintainability and user experience.
>
> On Sun, Mar 30, 2025 at 4:33 AM Michał Modras
>  wrote:
>
> > +1 to Bolke's points
> >
> > niedz., 30 mar 2025, 10:30 użytkownik Bolke de Bruin 
> > napisał:
> >
> > > I would be in favor of removing the experimental feature of the
> > constructor
> > > arguments, but I don't understand the reasoning for removing the
> > override.
> > > It is a breaking change from something that was there since 1.X so not
> > > really experimental anymore. I think you might underestimate how much
> it
> > is
> > > used. What would be the migration path? It does not execute the same as
> > > TearDown / Setup and Moving it to a constructor argument requires it to
> > be
> > > a part of a DAG, limiting deployment options.
> > >
> > > B.
> > >
> > >
> > >
> > > On Sat, 29 Mar 2025 at 13:56, Kaxil Naik  wrote:
> > >
> > > > @tamara : Correct, we are now proposing to remove overriding the
> > pre/post
> > > > execute
> > > >
> > > > >Quick question if I am understanding the proposed change correctly.
> > > >
> > > >
> > > > What you want to remove is overriding the pre/post execute when
> > creating
> > > > > custom operators:
> > > > >
> > > > > class MyOperator(BaseOperator):
> > > > >
> > > > > ...
> > > > >
> > > > > def pre_execute(self, context):   # This would break?
> > > > > 
> > > > > def post_execute(self, context):   # And this as well?
> > > > > But keep the (currently experimental) use of the pre_execute and
> > > > > post_execute parameters (I've only used post_execute before for
> > similar
> > > > > reasons as TP posted, interacting with outlets assets)
> > > >
> > > >
> > > > On Sat, 29 Mar 2025 at 18:23, Kaxil Naik 
> wrote:
> > > >
> > > > > No, @run_if / @skip_if uses pre_execute from task argument [1] not
> > the
> > > > the
> > > > > method and is just a syntactic sugar. You can also do the following
> > as
> > > an
> > > > > example:
> > > > >
> > > > > ```
> > > > > def skip_at_random(context):
> > > > > if randint(0, 1) == 0:
> > > > > raise AirflowSkipException()
> > > > >
> > > > > t2 = BashOperator(task_id='conditional2',
> pre_execute=skip_at_random,
> > > > > dag=dag, bash_command="airflow version")
> > > > > ```
> > > > >
> > > > > [1]:
> > > > >
> > > >
> > >
> >
> https://github.com/apache/airflow/blob/8c3a30e3ffc3f114c1d2cc3e6e109f4d9e29ca8b/airflow-core/src/airflow/decorators/condition.py#L57-L59
> > > > >
> > > > > On Sat, 29 Mar 2025 at 05:09, Matthew Block <
> > matthew.l.bl...@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > >> Would this also break @run_if/@skip_if decorators?
> > > > >>
> > > > >> Best,
> > > > >> Matt Block
> > > > >>
> > > > >> > On Mar 28, 2025, at 3:44 PM, Tamara Fingerlin
> > > > >>  wrote:
> > > > >> >
> > > > >> > Hey :)
> > > > >> >
> > > > >> > Quick question if I am understanding the proposed change
> > correctly.
> > > > >> >
> > > > >> > What you want to remove is overriding the pre/post execute when
> > > > creating
> > > > >> > custom operators:
> > > > >> >
> > > > >> > class MyOperator(BaseOperator):
> > > > >> >...
> > > > >> >def pre_execute(self, context):   # This would break?
> > > > >> >
> > > > >> >
> > > > >> >def post_execute(self, context):   # And this as well?
> > > > >> >
> > > > >> >
> > > > >> > But keep the (currently experimental) use of the pre_execute and
> > > > >> > post_execute parameters (I've only used post_execute before for
> > > > similar
> > > > >> > reasons as TP posted, interacting with outlets assets)
> > > > >> >
> > > > >> >BashOperator(
> > > > >> >task_id='hello_world',
> > > > >> >bash_command='sleep 5',
> > > > >> >pre_execute=lambda context: print("Pre-execute function
> > > > >> called!"),
> > > > >> > # this would still work?
> > > > >> >post_execute=lambda context: print("Post-execute function
> > > > >> > called!"),   # this would be supposed to still work 

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-31 Thread Kaxil Naik
Sane points, keeping it for now :)

On Mon, 31 Mar 2025 at 00:00, Jarek Potiuk  wrote:

> Yeah. I am convinced by Bolke's and Constance posts. Changing my vote to
> -0.9.
>
> On Sun, Mar 30, 2025 at 8:25 PM Constance Martineau
>  wrote:
>
> > Just to add to the discussion:
> >
> > Removing these overrides will disproportionately impact teams at
> companies
> > with more than a handful of Airflow users. Unless there’s a clear benefit
> > to removing them *and* a well-defined migration path, I don’t think it’s
> > worth the disruption. +1 to Bolke’s points.
> >
> > Like Tamara mentioned, we frequently see teams build internal providers
> > where they subclass standard operators and use pre_execute/post_execute
> to
> > inject things like service-specific auth, tracking identifiers, or other
> > cross-cutting concerns. These hooks act as a central entry point for
> > operational consistency, especially in environments where DAG authors
> > aren’t expected to manage those behaviors themselves.
> >
> > Pushing that logic down to every DAG or task definition would be a pretty
> > big shift for those teams, and, from what I’ve seen, a regression in
> terms
> > of maintainability and user experience.
> >
> > On Sun, Mar 30, 2025 at 4:33 AM Michał Modras
> >  wrote:
> >
> > > +1 to Bolke's points
> > >
> > > niedz., 30 mar 2025, 10:30 użytkownik Bolke de Bruin <
> bdbr...@gmail.com>
> > > napisał:
> > >
> > > > I would be in favor of removing the experimental feature of the
> > > constructor
> > > > arguments, but I don't understand the reasoning for removing the
> > > override.
> > > > It is a breaking change from something that was there since 1.X so
> not
> > > > really experimental anymore. I think you might underestimate how much
> > it
> > > is
> > > > used. What would be the migration path? It does not execute the same
> as
> > > > TearDown / Setup and Moving it to a constructor argument requires it
> to
> > > be
> > > > a part of a DAG, limiting deployment options.
> > > >
> > > > B.
> > > >
> > > >
> > > >
> > > > On Sat, 29 Mar 2025 at 13:56, Kaxil Naik 
> wrote:
> > > >
> > > > > @tamara : Correct, we are now proposing to remove overriding the
> > > pre/post
> > > > > execute
> > > > >
> > > > > >Quick question if I am understanding the proposed change
> correctly.
> > > > >
> > > > >
> > > > > What you want to remove is overriding the pre/post execute when
> > > creating
> > > > > > custom operators:
> > > > > >
> > > > > > class MyOperator(BaseOperator):
> > > > > >
> > > > > > ...
> > > > > >
> > > > > > def pre_execute(self, context):   # This would break?
> > > > > > 
> > > > > > def post_execute(self, context):   # And this as well?
> > > > > > But keep the (currently experimental) use of the pre_execute and
> > > > > > post_execute parameters (I've only used post_execute before for
> > > similar
> > > > > > reasons as TP posted, interacting with outlets assets)
> > > > >
> > > > >
> > > > > On Sat, 29 Mar 2025 at 18:23, Kaxil Naik 
> > wrote:
> > > > >
> > > > > > No, @run_if / @skip_if uses pre_execute from task argument [1]
> not
> > > the
> > > > > the
> > > > > > method and is just a syntactic sugar. You can also do the
> following
> > > as
> > > > an
> > > > > > example:
> > > > > >
> > > > > > ```
> > > > > > def skip_at_random(context):
> > > > > > if randint(0, 1) == 0:
> > > > > > raise AirflowSkipException()
> > > > > >
> > > > > > t2 = BashOperator(task_id='conditional2',
> > pre_execute=skip_at_random,
> > > > > > dag=dag, bash_command="airflow version")
> > > > > > ```
> > > > > >
> > > > > > [1]:
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/airflow/blob/8c3a30e3ffc3f114c1d2cc3e6e109f4d9e29ca8b/airflow-core/src/airflow/decorators/condition.py#L57-L59
> > > > > >
> > > > > > On Sat, 29 Mar 2025 at 05:09, Matthew Block <
> > > matthew.l.bl...@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > >> Would this also break @run_if/@skip_if decorators?
> > > > > >>
> > > > > >> Best,
> > > > > >> Matt Block
> > > > > >>
> > > > > >> > On Mar 28, 2025, at 3:44 PM, Tamara Fingerlin
> > > > > >>  wrote:
> > > > > >> >
> > > > > >> > Hey :)
> > > > > >> >
> > > > > >> > Quick question if I am understanding the proposed change
> > > correctly.
> > > > > >> >
> > > > > >> > What you want to remove is overriding the pre/post execute
> when
> > > > > creating
> > > > > >> > custom operators:
> > > > > >> >
> > > > > >> > class MyOperator(BaseOperator):
> > > > > >> >...
> > > > > >> >def pre_execute(self, context):   # This would break?
> > > > > >> >
> > > > > >> >
> > > > > >> >def post_execute(self, context):   # And this as well?
> > > > > >> >
> > > > > >> >
> > > > > >> > But keep the (currently experimental) use of the pre_execute
> and
> > > > > >> > post_execute parameters (I've only used post_execute before
> for
> > > > > similar
> > > > > >> > reasons as TP posted, interacting with outlets assets)

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-30 Thread Constance Martineau
Just to add to the discussion:

Removing these overrides will disproportionately impact teams at companies
with more than a handful of Airflow users. Unless there’s a clear benefit
to removing them *and* a well-defined migration path, I don’t think it’s
worth the disruption. +1 to Bolke’s points.

Like Tamara mentioned, we frequently see teams build internal providers
where they subclass standard operators and use pre_execute/post_execute to
inject things like service-specific auth, tracking identifiers, or other
cross-cutting concerns. These hooks act as a central entry point for
operational consistency, especially in environments where DAG authors
aren’t expected to manage those behaviors themselves.

Pushing that logic down to every DAG or task definition would be a pretty
big shift for those teams, and, from what I’ve seen, a regression in terms
of maintainability and user experience.

On Sun, Mar 30, 2025 at 4:33 AM Michał Modras
 wrote:

> +1 to Bolke's points
>
> niedz., 30 mar 2025, 10:30 użytkownik Bolke de Bruin 
> napisał:
>
> > I would be in favor of removing the experimental feature of the
> constructor
> > arguments, but I don't understand the reasoning for removing the
> override.
> > It is a breaking change from something that was there since 1.X so not
> > really experimental anymore. I think you might underestimate how much it
> is
> > used. What would be the migration path? It does not execute the same as
> > TearDown / Setup and Moving it to a constructor argument requires it to
> be
> > a part of a DAG, limiting deployment options.
> >
> > B.
> >
> >
> >
> > On Sat, 29 Mar 2025 at 13:56, Kaxil Naik  wrote:
> >
> > > @tamara : Correct, we are now proposing to remove overriding the
> pre/post
> > > execute
> > >
> > > >Quick question if I am understanding the proposed change correctly.
> > >
> > >
> > > What you want to remove is overriding the pre/post execute when
> creating
> > > > custom operators:
> > > >
> > > > class MyOperator(BaseOperator):
> > > >
> > > > ...
> > > >
> > > > def pre_execute(self, context):   # This would break?
> > > > 
> > > > def post_execute(self, context):   # And this as well?
> > > > But keep the (currently experimental) use of the pre_execute and
> > > > post_execute parameters (I've only used post_execute before for
> similar
> > > > reasons as TP posted, interacting with outlets assets)
> > >
> > >
> > > On Sat, 29 Mar 2025 at 18:23, Kaxil Naik  wrote:
> > >
> > > > No, @run_if / @skip_if uses pre_execute from task argument [1] not
> the
> > > the
> > > > method and is just a syntactic sugar. You can also do the following
> as
> > an
> > > > example:
> > > >
> > > > ```
> > > > def skip_at_random(context):
> > > > if randint(0, 1) == 0:
> > > > raise AirflowSkipException()
> > > >
> > > > t2 = BashOperator(task_id='conditional2', pre_execute=skip_at_random,
> > > > dag=dag, bash_command="airflow version")
> > > > ```
> > > >
> > > > [1]:
> > > >
> > >
> >
> https://github.com/apache/airflow/blob/8c3a30e3ffc3f114c1d2cc3e6e109f4d9e29ca8b/airflow-core/src/airflow/decorators/condition.py#L57-L59
> > > >
> > > > On Sat, 29 Mar 2025 at 05:09, Matthew Block <
> matthew.l.bl...@gmail.com
> > >
> > > > wrote:
> > > >
> > > >> Would this also break @run_if/@skip_if decorators?
> > > >>
> > > >> Best,
> > > >> Matt Block
> > > >>
> > > >> > On Mar 28, 2025, at 3:44 PM, Tamara Fingerlin
> > > >>  wrote:
> > > >> >
> > > >> > Hey :)
> > > >> >
> > > >> > Quick question if I am understanding the proposed change
> correctly.
> > > >> >
> > > >> > What you want to remove is overriding the pre/post execute when
> > > creating
> > > >> > custom operators:
> > > >> >
> > > >> > class MyOperator(BaseOperator):
> > > >> >...
> > > >> >def pre_execute(self, context):   # This would break?
> > > >> >
> > > >> >
> > > >> >def post_execute(self, context):   # And this as well?
> > > >> >
> > > >> >
> > > >> > But keep the (currently experimental) use of the pre_execute and
> > > >> > post_execute parameters (I've only used post_execute before for
> > > similar
> > > >> > reasons as TP posted, interacting with outlets assets)
> > > >> >
> > > >> >BashOperator(
> > > >> >task_id='hello_world',
> > > >> >bash_command='sleep 5',
> > > >> >pre_execute=lambda context: print("Pre-execute function
> > > >> called!"),
> > > >> > # this would still work?
> > > >> >post_execute=lambda context: print("Post-execute function
> > > >> > called!"),   # this would be supposed to still work (it does not
> rn
> > > 😅)
> > > >> >)
> > > >> >
> > > >> > The one situation I am worried about here is larger teams/orgs
> >  using
> > > pre
> > > >> > and post execute to standardize custom operators. For example
> team A
> > > >> writes
> > > >> > OurCompanyDatabaseBaseOperator that has a pre_execute and
> > post_execute
> > > >> with
> > > >> > mandatory business lo

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-30 Thread Michał Modras
+1 to Bolke's points

niedz., 30 mar 2025, 10:30 użytkownik Bolke de Bruin 
napisał:

> I would be in favor of removing the experimental feature of the constructor
> arguments, but I don't understand the reasoning for removing the override.
> It is a breaking change from something that was there since 1.X so not
> really experimental anymore. I think you might underestimate how much it is
> used. What would be the migration path? It does not execute the same as
> TearDown / Setup and Moving it to a constructor argument requires it to be
> a part of a DAG, limiting deployment options.
>
> B.
>
>
>
> On Sat, 29 Mar 2025 at 13:56, Kaxil Naik  wrote:
>
> > @tamara : Correct, we are now proposing to remove overriding the pre/post
> > execute
> >
> > >Quick question if I am understanding the proposed change correctly.
> >
> >
> > What you want to remove is overriding the pre/post execute when creating
> > > custom operators:
> > >
> > > class MyOperator(BaseOperator):
> > >
> > > ...
> > >
> > > def pre_execute(self, context):   # This would break?
> > > 
> > > def post_execute(self, context):   # And this as well?
> > > But keep the (currently experimental) use of the pre_execute and
> > > post_execute parameters (I've only used post_execute before for similar
> > > reasons as TP posted, interacting with outlets assets)
> >
> >
> > On Sat, 29 Mar 2025 at 18:23, Kaxil Naik  wrote:
> >
> > > No, @run_if / @skip_if uses pre_execute from task argument [1] not the
> > the
> > > method and is just a syntactic sugar. You can also do the following as
> an
> > > example:
> > >
> > > ```
> > > def skip_at_random(context):
> > > if randint(0, 1) == 0:
> > > raise AirflowSkipException()
> > >
> > > t2 = BashOperator(task_id='conditional2', pre_execute=skip_at_random,
> > > dag=dag, bash_command="airflow version")
> > > ```
> > >
> > > [1]:
> > >
> >
> https://github.com/apache/airflow/blob/8c3a30e3ffc3f114c1d2cc3e6e109f4d9e29ca8b/airflow-core/src/airflow/decorators/condition.py#L57-L59
> > >
> > > On Sat, 29 Mar 2025 at 05:09, Matthew Block  >
> > > wrote:
> > >
> > >> Would this also break @run_if/@skip_if decorators?
> > >>
> > >> Best,
> > >> Matt Block
> > >>
> > >> > On Mar 28, 2025, at 3:44 PM, Tamara Fingerlin
> > >>  wrote:
> > >> >
> > >> > Hey :)
> > >> >
> > >> > Quick question if I am understanding the proposed change correctly.
> > >> >
> > >> > What you want to remove is overriding the pre/post execute when
> > creating
> > >> > custom operators:
> > >> >
> > >> > class MyOperator(BaseOperator):
> > >> >...
> > >> >def pre_execute(self, context):   # This would break?
> > >> >
> > >> >
> > >> >def post_execute(self, context):   # And this as well?
> > >> >
> > >> >
> > >> > But keep the (currently experimental) use of the pre_execute and
> > >> > post_execute parameters (I've only used post_execute before for
> > similar
> > >> > reasons as TP posted, interacting with outlets assets)
> > >> >
> > >> >BashOperator(
> > >> >task_id='hello_world',
> > >> >bash_command='sleep 5',
> > >> >pre_execute=lambda context: print("Pre-execute function
> > >> called!"),
> > >> > # this would still work?
> > >> >post_execute=lambda context: print("Post-execute function
> > >> > called!"),   # this would be supposed to still work (it does not rn
> > 😅)
> > >> >)
> > >> >
> > >> > The one situation I am worried about here is larger teams/orgs
>  using
> > pre
> > >> > and post execute to standardize custom operators. For example team A
> > >> writes
> > >> > OurCompanyDatabaseBaseOperator that has a pre_execute and
> post_execute
> > >> with
> > >> > mandatory business logic and team B is allowed to write custom
> > >> operators on
> > >> > top of that but only override execute.
> > >> > I assume this would break?
> > >> >
> > >> >
> > >> >> On Fri, Mar 28, 2025 at 9:32 PM Jarek Potiuk 
> > wrote:
> > >> >>
> > >> >> I think the current proposal is to remove the pre/post in the Base
> > >> Operator
> > >> >> class (and overridability) and leave passing pre/post as
> constructor
> > >> >> arguments..
> > >> >>
> > >> >>> On Fri, Mar 28, 2025 at 9:20 PM Bolke de Bruin  >
> > >> wrote:
> > >> >>>
> > >> >>> Just one thing - the pre / post mechanisms are executed in-process
> > of
> > >> the
> > >> >>> task rather than the DAG. So they are not equal to setup/teardown.
> > Are
> > >> >> you
> > >> >>> proposing to remove the argument or the whole thing?
> > >> >>>
> > >> >>> B.
> > >> >>>
> > >> >>>
> > >> >>>
> > >>  On Fri, 28 Mar 2025 at 20:58, Jarek Potiuk 
> > wrote:
> > >> >>>
> > >>  Indeed. Post/pre overriding in sub-classes should go away (and we
> > >> could
> > >>  even likely implement a ruff rule to auto-fix those if someone
> has
> > a
> > >> >>> custom
> > >>  executor. Sounds like 100% doable
> > >> 
> > >>  But passing them as "cross-cutting concerns" via

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-30 Thread Bolke de Bruin
I would be in favor of removing the experimental feature of the constructor
arguments, but I don't understand the reasoning for removing the override.
It is a breaking change from something that was there since 1.X so not
really experimental anymore. I think you might underestimate how much it is
used. What would be the migration path? It does not execute the same as
TearDown / Setup and Moving it to a constructor argument requires it to be
a part of a DAG, limiting deployment options.

B.



On Sat, 29 Mar 2025 at 13:56, Kaxil Naik  wrote:

> @tamara : Correct, we are now proposing to remove overriding the pre/post
> execute
>
> >Quick question if I am understanding the proposed change correctly.
>
>
> What you want to remove is overriding the pre/post execute when creating
> > custom operators:
> >
> > class MyOperator(BaseOperator):
> >
> > ...
> >
> > def pre_execute(self, context):   # This would break?
> > 
> > def post_execute(self, context):   # And this as well?
> > But keep the (currently experimental) use of the pre_execute and
> > post_execute parameters (I've only used post_execute before for similar
> > reasons as TP posted, interacting with outlets assets)
>
>
> On Sat, 29 Mar 2025 at 18:23, Kaxil Naik  wrote:
>
> > No, @run_if / @skip_if uses pre_execute from task argument [1] not the
> the
> > method and is just a syntactic sugar. You can also do the following as an
> > example:
> >
> > ```
> > def skip_at_random(context):
> > if randint(0, 1) == 0:
> > raise AirflowSkipException()
> >
> > t2 = BashOperator(task_id='conditional2', pre_execute=skip_at_random,
> > dag=dag, bash_command="airflow version")
> > ```
> >
> > [1]:
> >
> https://github.com/apache/airflow/blob/8c3a30e3ffc3f114c1d2cc3e6e109f4d9e29ca8b/airflow-core/src/airflow/decorators/condition.py#L57-L59
> >
> > On Sat, 29 Mar 2025 at 05:09, Matthew Block 
> > wrote:
> >
> >> Would this also break @run_if/@skip_if decorators?
> >>
> >> Best,
> >> Matt Block
> >>
> >> > On Mar 28, 2025, at 3:44 PM, Tamara Fingerlin
> >>  wrote:
> >> >
> >> > Hey :)
> >> >
> >> > Quick question if I am understanding the proposed change correctly.
> >> >
> >> > What you want to remove is overriding the pre/post execute when
> creating
> >> > custom operators:
> >> >
> >> > class MyOperator(BaseOperator):
> >> >...
> >> >def pre_execute(self, context):   # This would break?
> >> >
> >> >
> >> >def post_execute(self, context):   # And this as well?
> >> >
> >> >
> >> > But keep the (currently experimental) use of the pre_execute and
> >> > post_execute parameters (I've only used post_execute before for
> similar
> >> > reasons as TP posted, interacting with outlets assets)
> >> >
> >> >BashOperator(
> >> >task_id='hello_world',
> >> >bash_command='sleep 5',
> >> >pre_execute=lambda context: print("Pre-execute function
> >> called!"),
> >> > # this would still work?
> >> >post_execute=lambda context: print("Post-execute function
> >> > called!"),   # this would be supposed to still work (it does not rn
> 😅)
> >> >)
> >> >
> >> > The one situation I am worried about here is larger teams/orgs using
> pre
> >> > and post execute to standardize custom operators. For example team A
> >> writes
> >> > OurCompanyDatabaseBaseOperator that has a pre_execute and post_execute
> >> with
> >> > mandatory business logic and team B is allowed to write custom
> >> operators on
> >> > top of that but only override execute.
> >> > I assume this would break?
> >> >
> >> >
> >> >> On Fri, Mar 28, 2025 at 9:32 PM Jarek Potiuk 
> wrote:
> >> >>
> >> >> I think the current proposal is to remove the pre/post in the Base
> >> Operator
> >> >> class (and overridability) and leave passing pre/post as constructor
> >> >> arguments..
> >> >>
> >> >>> On Fri, Mar 28, 2025 at 9:20 PM Bolke de Bruin 
> >> wrote:
> >> >>>
> >> >>> Just one thing - the pre / post mechanisms are executed in-process
> of
> >> the
> >> >>> task rather than the DAG. So they are not equal to setup/teardown.
> Are
> >> >> you
> >> >>> proposing to remove the argument or the whole thing?
> >> >>>
> >> >>> B.
> >> >>>
> >> >>>
> >> >>>
> >>  On Fri, 28 Mar 2025 at 20:58, Jarek Potiuk 
> wrote:
> >> >>>
> >>  Indeed. Post/pre overriding in sub-classes should go away (and we
> >> could
> >>  even likely implement a ruff rule to auto-fix those if someone has
> a
> >> >>> custom
> >>  executor. Sounds like 100% doable
> >> 
> >>  But passing them as "cross-cutting concerns" via callable in a
> >> >>> constructor
> >>  is pretty useful and not easily fixable for back-compatibilty
> >> 
> >>  J.
> >> 
> >>  On Fri, Mar 28, 2025 at 6:14 PM Kaxil Naik 
> >> >> wrote:
> >> 
> >> >> I think the ability of overriding pre_execute and post_execute
> in a
> >> > subclass can definitely go away. They are practically useles; you
> >> can
> >>  just
> >> > put every

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-29 Thread Kaxil Naik
No, @run_if / @skip_if uses pre_execute from task argument [1] not the the
method and is just a syntactic sugar. You can also do the following as an
example:

```
def skip_at_random(context):
if randint(0, 1) == 0:
raise AirflowSkipException()

t2 = BashOperator(task_id='conditional2', pre_execute=skip_at_random,
dag=dag, bash_command="airflow version")
```

[1]:
https://github.com/apache/airflow/blob/8c3a30e3ffc3f114c1d2cc3e6e109f4d9e29ca8b/airflow-core/src/airflow/decorators/condition.py#L57-L59

On Sat, 29 Mar 2025 at 05:09, Matthew Block 
wrote:

> Would this also break @run_if/@skip_if decorators?
>
> Best,
> Matt Block
>
> > On Mar 28, 2025, at 3:44 PM, Tamara Fingerlin
>  wrote:
> >
> > Hey :)
> >
> > Quick question if I am understanding the proposed change correctly.
> >
> > What you want to remove is overriding the pre/post execute when creating
> > custom operators:
> >
> > class MyOperator(BaseOperator):
> >...
> >def pre_execute(self, context):   # This would break?
> >
> >
> >def post_execute(self, context):   # And this as well?
> >
> >
> > But keep the (currently experimental) use of the pre_execute and
> > post_execute parameters (I've only used post_execute before for similar
> > reasons as TP posted, interacting with outlets assets)
> >
> >BashOperator(
> >task_id='hello_world',
> >bash_command='sleep 5',
> >pre_execute=lambda context: print("Pre-execute function called!"),
> > # this would still work?
> >post_execute=lambda context: print("Post-execute function
> > called!"),   # this would be supposed to still work (it does not rn 😅)
> >)
> >
> > The one situation I am worried about here is larger teams/orgs using pre
> > and post execute to standardize custom operators. For example team A
> writes
> > OurCompanyDatabaseBaseOperator that has a pre_execute and post_execute
> with
> > mandatory business logic and team B is allowed to write custom operators
> on
> > top of that but only override execute.
> > I assume this would break?
> >
> >
> >> On Fri, Mar 28, 2025 at 9:32 PM Jarek Potiuk  wrote:
> >>
> >> I think the current proposal is to remove the pre/post in the Base
> Operator
> >> class (and overridability) and leave passing pre/post as constructor
> >> arguments..
> >>
> >>> On Fri, Mar 28, 2025 at 9:20 PM Bolke de Bruin 
> wrote:
> >>>
> >>> Just one thing - the pre / post mechanisms are executed in-process of
> the
> >>> task rather than the DAG. So they are not equal to setup/teardown. Are
> >> you
> >>> proposing to remove the argument or the whole thing?
> >>>
> >>> B.
> >>>
> >>>
> >>>
>  On Fri, 28 Mar 2025 at 20:58, Jarek Potiuk  wrote:
> >>>
>  Indeed. Post/pre overriding in sub-classes should go away (and we
> could
>  even likely implement a ruff rule to auto-fix those if someone has a
> >>> custom
>  executor. Sounds like 100% doable
> 
>  But passing them as "cross-cutting concerns" via callable in a
> >>> constructor
>  is pretty useful and not easily fixable for back-compatibilty
> 
>  J.
> 
>  On Fri, Mar 28, 2025 at 6:14 PM Kaxil Naik 
> >> wrote:
> 
> >> I think the ability of overriding pre_execute and post_execute in a
> > subclass can definitely go away. They are practically useles; you can
>  just
> > put everything in execute, which always needs to exist in a
> >>> BaseOperator
> > subclass anyway.
> >
> > Yeah I am fine with removing that then. Anyone disagrees?
> >
> > On Fri, 28 Mar 2025 at 20:36, Michał Modras  > .invalid>
> > wrote:
> >
> >> I'd prefer a world without separate pre_execute and post_execute
> > functions
> >> - as pointed out in the PR, they make reasoning about DAGs more
>  complex,
> >> and can be error prone.
> >>
> >> Having said that, I know there are multiple users relying on these
> >> functionalities, so I'll bring up my usual - another breaking
> >> change
> >>> -
> >> another obstacle to the AF3 adoption argument.
> >>
> >> And as for relying on operators vs. PythonOperator + hooks - there
> >>> are
> > good
> >> arguments for continuing relying on operators, or even rely on them
>  more,
> >> depending on customers' need and organizational setup.
> >>
> >> On Fri, Mar 28, 2025 at 3:42 PM Tzu-ping Chung
>   >>
> >> wrote:
> >>
> >>> Passing post_execute as an argument is somewhat useful for
> >>> operators
> > that
> >>> don’t support assets natively (most of them) but when you want to
>  emit
> > a
> >>> dynamic path. For example:
> >>>
> >>> def _send_asset_event(context, result):
> >>># Rendered value!
> >>>name = context["task"].output
> >>># Trigger an event against the emitted path.
> >>>context["outlet_events"][write_data_outlet].add(Asset(name))
> >>>
> >>> write_data_outlet = AssetAlias("data")

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Matthew Block
Would this also break @run_if/@skip_if decorators?

Best,
Matt Block

> On Mar 28, 2025, at 3:44 PM, Tamara Fingerlin 
>  wrote:
> 
> Hey :)
> 
> Quick question if I am understanding the proposed change correctly.
> 
> What you want to remove is overriding the pre/post execute when creating
> custom operators:
> 
> class MyOperator(BaseOperator):
>...
>def pre_execute(self, context):   # This would break?
>
> 
>def post_execute(self, context):   # And this as well?
> 
> 
> But keep the (currently experimental) use of the pre_execute and
> post_execute parameters (I've only used post_execute before for similar
> reasons as TP posted, interacting with outlets assets)
> 
>BashOperator(
>task_id='hello_world',
>bash_command='sleep 5',
>pre_execute=lambda context: print("Pre-execute function called!"),
> # this would still work?
>post_execute=lambda context: print("Post-execute function
> called!"),   # this would be supposed to still work (it does not rn 😅)
>)
> 
> The one situation I am worried about here is larger teams/orgs using pre
> and post execute to standardize custom operators. For example team A writes
> OurCompanyDatabaseBaseOperator that has a pre_execute and post_execute with
> mandatory business logic and team B is allowed to write custom operators on
> top of that but only override execute.
> I assume this would break?
> 
> 
>> On Fri, Mar 28, 2025 at 9:32 PM Jarek Potiuk  wrote:
>> 
>> I think the current proposal is to remove the pre/post in the Base Operator
>> class (and overridability) and leave passing pre/post as constructor
>> arguments..
>> 
>>> On Fri, Mar 28, 2025 at 9:20 PM Bolke de Bruin  wrote:
>>> 
>>> Just one thing - the pre / post mechanisms are executed in-process of the
>>> task rather than the DAG. So they are not equal to setup/teardown. Are
>> you
>>> proposing to remove the argument or the whole thing?
>>> 
>>> B.
>>> 
>>> 
>>> 
 On Fri, 28 Mar 2025 at 20:58, Jarek Potiuk  wrote:
>>> 
 Indeed. Post/pre overriding in sub-classes should go away (and we could
 even likely implement a ruff rule to auto-fix those if someone has a
>>> custom
 executor. Sounds like 100% doable
 
 But passing them as "cross-cutting concerns" via callable in a
>>> constructor
 is pretty useful and not easily fixable for back-compatibilty
 
 J.
 
 On Fri, Mar 28, 2025 at 6:14 PM Kaxil Naik 
>> wrote:
 
>> I think the ability of overriding pre_execute and post_execute in a
> subclass can definitely go away. They are practically useles; you can
 just
> put everything in execute, which always needs to exist in a
>>> BaseOperator
> subclass anyway.
> 
> Yeah I am fine with removing that then. Anyone disagrees?
> 
> On Fri, 28 Mar 2025 at 20:36, Michał Modras  .invalid>
> wrote:
> 
>> I'd prefer a world without separate pre_execute and post_execute
> functions
>> - as pointed out in the PR, they make reasoning about DAGs more
 complex,
>> and can be error prone.
>> 
>> Having said that, I know there are multiple users relying on these
>> functionalities, so I'll bring up my usual - another breaking
>> change
>>> -
>> another obstacle to the AF3 adoption argument.
>> 
>> And as for relying on operators vs. PythonOperator + hooks - there
>>> are
> good
>> arguments for continuing relying on operators, or even rely on them
 more,
>> depending on customers' need and organizational setup.
>> 
>> On Fri, Mar 28, 2025 at 3:42 PM Tzu-ping Chung
 > 
>> wrote:
>> 
>>> Passing post_execute as an argument is somewhat useful for
>>> operators
> that
>>> don’t support assets natively (most of them) but when you want to
 emit
> a
>>> dynamic path. For example:
>>> 
>>> def _send_asset_event(context, result):
>>># Rendered value!
>>>name = context["task"].output
>>># Trigger an event against the emitted path.
>>>context["outlet_events"][write_data_outlet].add(Asset(name))
>>> 
>>> write_data_outlet = AssetAlias("data")
>>> 
>>> WriteSomeDataOperator(
>>>task_id="write_data",
>>>output="write_data_{{ run_id }}.parquet",
>>>outlets=[write_data_outlet],
>>>post_execute=_send_asset_event,
>>> )
>>> 
>>> Without the functionality, you’ll have to write a subclass for
>> each
>>> operator you want to do this, which is quite a bit boilerplate.
>>> 
>>> Arguably this is only needed since we use operators too much.
>> This
>>> wouldn’t be an issue if we rely more on the PythonOperator+hooks
> approach
>>> (like Bolke discussed at last year’s Airflow Summit), but alas,
 people
>>> don’t like to change how they do things, and operators are still
>>> very
>>> popular.
>>> 
>>> The pre_execute argument *might* also be useful i

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Bolke de Bruin
Just one thing - the pre / post mechanisms are executed in-process of the
task rather than the DAG. So they are not equal to setup/teardown. Are you
proposing to remove the argument or the whole thing?

B.



On Fri, 28 Mar 2025 at 20:58, Jarek Potiuk  wrote:

> Indeed. Post/pre overriding in sub-classes should go away (and we could
> even likely implement a ruff rule to auto-fix those if someone has a custom
> executor. Sounds like 100% doable
>
> But passing them as "cross-cutting concerns" via callable in a constructor
> is pretty useful and not easily fixable for back-compatibilty
>
> J.
>
> On Fri, Mar 28, 2025 at 6:14 PM Kaxil Naik  wrote:
>
> > >I think the ability of overriding pre_execute and post_execute in a
> > subclass can definitely go away. They are practically useles; you can
> just
> > put everything in execute, which always needs to exist in a BaseOperator
> > subclass anyway.
> >
> > Yeah I am fine with removing that then. Anyone disagrees?
> >
> > On Fri, 28 Mar 2025 at 20:36, Michał Modras  > .invalid>
> > wrote:
> >
> > > I'd prefer a world without separate pre_execute and post_execute
> > functions
> > > - as pointed out in the PR, they make reasoning about DAGs more
> complex,
> > > and can be error prone.
> > >
> > > Having said that, I know there are multiple users relying on these
> > > functionalities, so I'll bring up my usual - another breaking change -
> > > another obstacle to the AF3 adoption argument.
> > >
> > > And as for relying on operators vs. PythonOperator + hooks - there are
> > good
> > > arguments for continuing relying on operators, or even rely on them
> more,
> > > depending on customers' need and organizational setup.
> > >
> > > On Fri, Mar 28, 2025 at 3:42 PM Tzu-ping Chung
>  > >
> > > wrote:
> > >
> > > > Passing post_execute as an argument is somewhat useful for operators
> > that
> > > > don’t support assets natively (most of them) but when you want to
> emit
> > a
> > > > dynamic path. For example:
> > > >
> > > > def _send_asset_event(context, result):
> > > > # Rendered value!
> > > > name = context["task"].output
> > > > # Trigger an event against the emitted path.
> > > > context["outlet_events"][write_data_outlet].add(Asset(name))
> > > >
> > > > write_data_outlet = AssetAlias("data")
> > > >
> > > > WriteSomeDataOperator(
> > > > task_id="write_data",
> > > > output="write_data_{{ run_id }}.parquet",
> > > > outlets=[write_data_outlet],
> > > > post_execute=_send_asset_event,
> > > > )
> > > >
> > > > Without the functionality, you’ll have to write a subclass for each
> > > > operator you want to do this, which is quite a bit boilerplate.
> > > >
> > > > Arguably this is only needed since we use operators too much. This
> > > > wouldn’t be an issue if we rely more on the PythonOperator+hooks
> > approach
> > > > (like Bolke discussed at last year’s Airflow Summit), but alas,
> people
> > > > don’t like to change how they do things, and operators are still very
> > > > popular.
> > > >
> > > > The pre_execute argument *might* also be useful if you want to
> > > pre-process
> > > > some values. That’s probably a lot less common, so I wouldn’t fret
> too
> > > much
> > > > if it goes away. However, since post_execute and pre_execute
> basically
> > > use
> > > > the same implementation, just one run right before and one right
> after
> > > > execute, they should probably stay or go together.
> > > >
> > > > I think the ability of overriding pre_execute and post_execute in a
> > > > subclass can definitely go away. They are practically useles; you can
> > > just
> > > > put everything in execute, which always needs to exist in a
> > BaseOperator
> > > > subclass anyway.
> > > >
> > > > TP
> > > >
> > > >
> > > > > On 28 Mar 2025, at 22:12, Kaxil Naik  wrote:
> > > > >
> > > > > I am in favor of dropping support as they essentially do the same
> --
> > > and
> > > > > setup & teardown is more "native" (first-class UI support)
> > > > >
> > > > > On Fri, 28 Mar 2025 at 19:41, Kaxil Naik 
> > wrote:
> > > > >
> > > > >> Hi team,
> > > > >>
> > > > >> Should we drop support for pre_execute and post_execute for AF
> 3.0?
> > > They
> > > > >> are still marked as experimental [1]. They were added [2] in a
> world
> > > > >> without Setup and Teardown tasks.
> > > > >>
> > > > >> Regards,
> > > > >>
> > > > >> Kaxil
> > > > >>
> > > > >>
> > > > >> [1]:
> > > > >>
> > > >
> > >
> >
> https://github.com/apache/airflow/blob/7af0319ba16749f4aea78085dfe7823f321d262a/task-sdk/src/airflow/sdk/bases/baseoperator.py#L715-L724
> > > > >> [2]: https://github.com/apache/airflow/pull/17576
> > > > >>
> > > >
> > > >
> > > > -
> > > > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
> > > > For additional commands, e-mail: dev-h...@airflow.apache.org
> > > >
> > > >
> > >
> >
>


-- 

--
Bolke de Bruin
bdbr...@gmail.com


Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Jarek Potiuk
I think the current proposal is to remove the pre/post in the Base Operator
class (and overridability) and leave passing pre/post as constructor
arguments..

On Fri, Mar 28, 2025 at 9:20 PM Bolke de Bruin  wrote:

> Just one thing - the pre / post mechanisms are executed in-process of the
> task rather than the DAG. So they are not equal to setup/teardown. Are you
> proposing to remove the argument or the whole thing?
>
> B.
>
>
>
> On Fri, 28 Mar 2025 at 20:58, Jarek Potiuk  wrote:
>
> > Indeed. Post/pre overriding in sub-classes should go away (and we could
> > even likely implement a ruff rule to auto-fix those if someone has a
> custom
> > executor. Sounds like 100% doable
> >
> > But passing them as "cross-cutting concerns" via callable in a
> constructor
> > is pretty useful and not easily fixable for back-compatibilty
> >
> > J.
> >
> > On Fri, Mar 28, 2025 at 6:14 PM Kaxil Naik  wrote:
> >
> > > >I think the ability of overriding pre_execute and post_execute in a
> > > subclass can definitely go away. They are practically useles; you can
> > just
> > > put everything in execute, which always needs to exist in a
> BaseOperator
> > > subclass anyway.
> > >
> > > Yeah I am fine with removing that then. Anyone disagrees?
> > >
> > > On Fri, 28 Mar 2025 at 20:36, Michał Modras  > > .invalid>
> > > wrote:
> > >
> > > > I'd prefer a world without separate pre_execute and post_execute
> > > functions
> > > > - as pointed out in the PR, they make reasoning about DAGs more
> > complex,
> > > > and can be error prone.
> > > >
> > > > Having said that, I know there are multiple users relying on these
> > > > functionalities, so I'll bring up my usual - another breaking change
> -
> > > > another obstacle to the AF3 adoption argument.
> > > >
> > > > And as for relying on operators vs. PythonOperator + hooks - there
> are
> > > good
> > > > arguments for continuing relying on operators, or even rely on them
> > more,
> > > > depending on customers' need and organizational setup.
> > > >
> > > > On Fri, Mar 28, 2025 at 3:42 PM Tzu-ping Chung
> >  > > >
> > > > wrote:
> > > >
> > > > > Passing post_execute as an argument is somewhat useful for
> operators
> > > that
> > > > > don’t support assets natively (most of them) but when you want to
> > emit
> > > a
> > > > > dynamic path. For example:
> > > > >
> > > > > def _send_asset_event(context, result):
> > > > > # Rendered value!
> > > > > name = context["task"].output
> > > > > # Trigger an event against the emitted path.
> > > > > context["outlet_events"][write_data_outlet].add(Asset(name))
> > > > >
> > > > > write_data_outlet = AssetAlias("data")
> > > > >
> > > > > WriteSomeDataOperator(
> > > > > task_id="write_data",
> > > > > output="write_data_{{ run_id }}.parquet",
> > > > > outlets=[write_data_outlet],
> > > > > post_execute=_send_asset_event,
> > > > > )
> > > > >
> > > > > Without the functionality, you’ll have to write a subclass for each
> > > > > operator you want to do this, which is quite a bit boilerplate.
> > > > >
> > > > > Arguably this is only needed since we use operators too much. This
> > > > > wouldn’t be an issue if we rely more on the PythonOperator+hooks
> > > approach
> > > > > (like Bolke discussed at last year’s Airflow Summit), but alas,
> > people
> > > > > don’t like to change how they do things, and operators are still
> very
> > > > > popular.
> > > > >
> > > > > The pre_execute argument *might* also be useful if you want to
> > > > pre-process
> > > > > some values. That’s probably a lot less common, so I wouldn’t fret
> > too
> > > > much
> > > > > if it goes away. However, since post_execute and pre_execute
> > basically
> > > > use
> > > > > the same implementation, just one run right before and one right
> > after
> > > > > execute, they should probably stay or go together.
> > > > >
> > > > > I think the ability of overriding pre_execute and post_execute in a
> > > > > subclass can definitely go away. They are practically useles; you
> can
> > > > just
> > > > > put everything in execute, which always needs to exist in a
> > > BaseOperator
> > > > > subclass anyway.
> > > > >
> > > > > TP
> > > > >
> > > > >
> > > > > > On 28 Mar 2025, at 22:12, Kaxil Naik 
> wrote:
> > > > > >
> > > > > > I am in favor of dropping support as they essentially do the same
> > --
> > > > and
> > > > > > setup & teardown is more "native" (first-class UI support)
> > > > > >
> > > > > > On Fri, 28 Mar 2025 at 19:41, Kaxil Naik 
> > > wrote:
> > > > > >
> > > > > >> Hi team,
> > > > > >>
> > > > > >> Should we drop support for pre_execute and post_execute for AF
> > 3.0?
> > > > They
> > > > > >> are still marked as experimental [1]. They were added [2] in a
> > world
> > > > > >> without Setup and Teardown tasks.
> > > > > >>
> > > > > >> Regards,
> > > > > >>
> > > > > >> Kaxil
> > > > > >>
> > > > > >>
> > > > > >> [1]:
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://github.com/apache/airflow/blob/7af031

Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Jarek Potiuk
Indeed. Post/pre overriding in sub-classes should go away (and we could
even likely implement a ruff rule to auto-fix those if someone has a custom
executor. Sounds like 100% doable

But passing them as "cross-cutting concerns" via callable in a constructor
is pretty useful and not easily fixable for back-compatibilty

J.

On Fri, Mar 28, 2025 at 6:14 PM Kaxil Naik  wrote:

> >I think the ability of overriding pre_execute and post_execute in a
> subclass can definitely go away. They are practically useles; you can just
> put everything in execute, which always needs to exist in a BaseOperator
> subclass anyway.
>
> Yeah I am fine with removing that then. Anyone disagrees?
>
> On Fri, 28 Mar 2025 at 20:36, Michał Modras  .invalid>
> wrote:
>
> > I'd prefer a world without separate pre_execute and post_execute
> functions
> > - as pointed out in the PR, they make reasoning about DAGs more complex,
> > and can be error prone.
> >
> > Having said that, I know there are multiple users relying on these
> > functionalities, so I'll bring up my usual - another breaking change -
> > another obstacle to the AF3 adoption argument.
> >
> > And as for relying on operators vs. PythonOperator + hooks - there are
> good
> > arguments for continuing relying on operators, or even rely on them more,
> > depending on customers' need and organizational setup.
> >
> > On Fri, Mar 28, 2025 at 3:42 PM Tzu-ping Chung  >
> > wrote:
> >
> > > Passing post_execute as an argument is somewhat useful for operators
> that
> > > don’t support assets natively (most of them) but when you want to emit
> a
> > > dynamic path. For example:
> > >
> > > def _send_asset_event(context, result):
> > > # Rendered value!
> > > name = context["task"].output
> > > # Trigger an event against the emitted path.
> > > context["outlet_events"][write_data_outlet].add(Asset(name))
> > >
> > > write_data_outlet = AssetAlias("data")
> > >
> > > WriteSomeDataOperator(
> > > task_id="write_data",
> > > output="write_data_{{ run_id }}.parquet",
> > > outlets=[write_data_outlet],
> > > post_execute=_send_asset_event,
> > > )
> > >
> > > Without the functionality, you’ll have to write a subclass for each
> > > operator you want to do this, which is quite a bit boilerplate.
> > >
> > > Arguably this is only needed since we use operators too much. This
> > > wouldn’t be an issue if we rely more on the PythonOperator+hooks
> approach
> > > (like Bolke discussed at last year’s Airflow Summit), but alas, people
> > > don’t like to change how they do things, and operators are still very
> > > popular.
> > >
> > > The pre_execute argument *might* also be useful if you want to
> > pre-process
> > > some values. That’s probably a lot less common, so I wouldn’t fret too
> > much
> > > if it goes away. However, since post_execute and pre_execute basically
> > use
> > > the same implementation, just one run right before and one right after
> > > execute, they should probably stay or go together.
> > >
> > > I think the ability of overriding pre_execute and post_execute in a
> > > subclass can definitely go away. They are practically useles; you can
> > just
> > > put everything in execute, which always needs to exist in a
> BaseOperator
> > > subclass anyway.
> > >
> > > TP
> > >
> > >
> > > > On 28 Mar 2025, at 22:12, Kaxil Naik  wrote:
> > > >
> > > > I am in favor of dropping support as they essentially do the same --
> > and
> > > > setup & teardown is more "native" (first-class UI support)
> > > >
> > > > On Fri, 28 Mar 2025 at 19:41, Kaxil Naik 
> wrote:
> > > >
> > > >> Hi team,
> > > >>
> > > >> Should we drop support for pre_execute and post_execute for AF 3.0?
> > They
> > > >> are still marked as experimental [1]. They were added [2] in a world
> > > >> without Setup and Teardown tasks.
> > > >>
> > > >> Regards,
> > > >>
> > > >> Kaxil
> > > >>
> > > >>
> > > >> [1]:
> > > >>
> > >
> >
> https://github.com/apache/airflow/blob/7af0319ba16749f4aea78085dfe7823f321d262a/task-sdk/src/airflow/sdk/bases/baseoperator.py#L715-L724
> > > >> [2]: https://github.com/apache/airflow/pull/17576
> > > >>
> > >
> > >
> > > -
> > > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
> > > For additional commands, e-mail: dev-h...@airflow.apache.org
> > >
> > >
> >
>


Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Kaxil Naik
>I think the ability of overriding pre_execute and post_execute in a
subclass can definitely go away. They are practically useles; you can just
put everything in execute, which always needs to exist in a BaseOperator
subclass anyway.

Yeah I am fine with removing that then. Anyone disagrees?

On Fri, 28 Mar 2025 at 20:36, Michał Modras 
wrote:

> I'd prefer a world without separate pre_execute and post_execute functions
> - as pointed out in the PR, they make reasoning about DAGs more complex,
> and can be error prone.
>
> Having said that, I know there are multiple users relying on these
> functionalities, so I'll bring up my usual - another breaking change -
> another obstacle to the AF3 adoption argument.
>
> And as for relying on operators vs. PythonOperator + hooks - there are good
> arguments for continuing relying on operators, or even rely on them more,
> depending on customers' need and organizational setup.
>
> On Fri, Mar 28, 2025 at 3:42 PM Tzu-ping Chung 
> wrote:
>
> > Passing post_execute as an argument is somewhat useful for operators that
> > don’t support assets natively (most of them) but when you want to emit a
> > dynamic path. For example:
> >
> > def _send_asset_event(context, result):
> > # Rendered value!
> > name = context["task"].output
> > # Trigger an event against the emitted path.
> > context["outlet_events"][write_data_outlet].add(Asset(name))
> >
> > write_data_outlet = AssetAlias("data")
> >
> > WriteSomeDataOperator(
> > task_id="write_data",
> > output="write_data_{{ run_id }}.parquet",
> > outlets=[write_data_outlet],
> > post_execute=_send_asset_event,
> > )
> >
> > Without the functionality, you’ll have to write a subclass for each
> > operator you want to do this, which is quite a bit boilerplate.
> >
> > Arguably this is only needed since we use operators too much. This
> > wouldn’t be an issue if we rely more on the PythonOperator+hooks approach
> > (like Bolke discussed at last year’s Airflow Summit), but alas, people
> > don’t like to change how they do things, and operators are still very
> > popular.
> >
> > The pre_execute argument *might* also be useful if you want to
> pre-process
> > some values. That’s probably a lot less common, so I wouldn’t fret too
> much
> > if it goes away. However, since post_execute and pre_execute basically
> use
> > the same implementation, just one run right before and one right after
> > execute, they should probably stay or go together.
> >
> > I think the ability of overriding pre_execute and post_execute in a
> > subclass can definitely go away. They are practically useles; you can
> just
> > put everything in execute, which always needs to exist in a BaseOperator
> > subclass anyway.
> >
> > TP
> >
> >
> > > On 28 Mar 2025, at 22:12, Kaxil Naik  wrote:
> > >
> > > I am in favor of dropping support as they essentially do the same --
> and
> > > setup & teardown is more "native" (first-class UI support)
> > >
> > > On Fri, 28 Mar 2025 at 19:41, Kaxil Naik  wrote:
> > >
> > >> Hi team,
> > >>
> > >> Should we drop support for pre_execute and post_execute for AF 3.0?
> They
> > >> are still marked as experimental [1]. They were added [2] in a world
> > >> without Setup and Teardown tasks.
> > >>
> > >> Regards,
> > >>
> > >> Kaxil
> > >>
> > >>
> > >> [1]:
> > >>
> >
> https://github.com/apache/airflow/blob/7af0319ba16749f4aea78085dfe7823f321d262a/task-sdk/src/airflow/sdk/bases/baseoperator.py#L715-L724
> > >> [2]: https://github.com/apache/airflow/pull/17576
> > >>
> >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
> > For additional commands, e-mail: dev-h...@airflow.apache.org
> >
> >
>


Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Michał Modras
I'd prefer a world without separate pre_execute and post_execute functions
- as pointed out in the PR, they make reasoning about DAGs more complex,
and can be error prone.

Having said that, I know there are multiple users relying on these
functionalities, so I'll bring up my usual - another breaking change -
another obstacle to the AF3 adoption argument.

And as for relying on operators vs. PythonOperator + hooks - there are good
arguments for continuing relying on operators, or even rely on them more,
depending on customers' need and organizational setup.

On Fri, Mar 28, 2025 at 3:42 PM Tzu-ping Chung 
wrote:

> Passing post_execute as an argument is somewhat useful for operators that
> don’t support assets natively (most of them) but when you want to emit a
> dynamic path. For example:
>
> def _send_asset_event(context, result):
> # Rendered value!
> name = context["task"].output
> # Trigger an event against the emitted path.
> context["outlet_events"][write_data_outlet].add(Asset(name))
>
> write_data_outlet = AssetAlias("data")
>
> WriteSomeDataOperator(
> task_id="write_data",
> output="write_data_{{ run_id }}.parquet",
> outlets=[write_data_outlet],
> post_execute=_send_asset_event,
> )
>
> Without the functionality, you’ll have to write a subclass for each
> operator you want to do this, which is quite a bit boilerplate.
>
> Arguably this is only needed since we use operators too much. This
> wouldn’t be an issue if we rely more on the PythonOperator+hooks approach
> (like Bolke discussed at last year’s Airflow Summit), but alas, people
> don’t like to change how they do things, and operators are still very
> popular.
>
> The pre_execute argument *might* also be useful if you want to pre-process
> some values. That’s probably a lot less common, so I wouldn’t fret too much
> if it goes away. However, since post_execute and pre_execute basically use
> the same implementation, just one run right before and one right after
> execute, they should probably stay or go together.
>
> I think the ability of overriding pre_execute and post_execute in a
> subclass can definitely go away. They are practically useles; you can just
> put everything in execute, which always needs to exist in a BaseOperator
> subclass anyway.
>
> TP
>
>
> > On 28 Mar 2025, at 22:12, Kaxil Naik  wrote:
> >
> > I am in favor of dropping support as they essentially do the same -- and
> > setup & teardown is more "native" (first-class UI support)
> >
> > On Fri, 28 Mar 2025 at 19:41, Kaxil Naik  wrote:
> >
> >> Hi team,
> >>
> >> Should we drop support for pre_execute and post_execute for AF 3.0? They
> >> are still marked as experimental [1]. They were added [2] in a world
> >> without Setup and Teardown tasks.
> >>
> >> Regards,
> >>
> >> Kaxil
> >>
> >>
> >> [1]:
> >>
> https://github.com/apache/airflow/blob/7af0319ba16749f4aea78085dfe7823f321d262a/task-sdk/src/airflow/sdk/bases/baseoperator.py#L715-L724
> >> [2]: https://github.com/apache/airflow/pull/17576
> >>
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
> For additional commands, e-mail: dev-h...@airflow.apache.org
>
>


Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Tzu-ping Chung
Passing post_execute as an argument is somewhat useful for operators that don’t 
support assets natively (most of them) but when you want to emit a dynamic 
path. For example:

def _send_asset_event(context, result):
# Rendered value!
name = context["task"].output
# Trigger an event against the emitted path.
context["outlet_events"][write_data_outlet].add(Asset(name))

write_data_outlet = AssetAlias("data")

WriteSomeDataOperator(
task_id="write_data",
output="write_data_{{ run_id }}.parquet",
outlets=[write_data_outlet],
post_execute=_send_asset_event,
)

Without the functionality, you’ll have to write a subclass for each operator 
you want to do this, which is quite a bit boilerplate.

Arguably this is only needed since we use operators too much. This wouldn’t be 
an issue if we rely more on the PythonOperator+hooks approach (like Bolke 
discussed at last year’s Airflow Summit), but alas, people don’t like to change 
how they do things, and operators are still very popular.

The pre_execute argument *might* also be useful if you want to pre-process some 
values. That’s probably a lot less common, so I wouldn’t fret too much if it 
goes away. However, since post_execute and pre_execute basically use the same 
implementation, just one run right before and one right after execute, they 
should probably stay or go together.

I think the ability of overriding pre_execute and post_execute in a subclass 
can definitely go away. They are practically useles; you can just put 
everything in execute, which always needs to exist in a BaseOperator subclass 
anyway.

TP


> On 28 Mar 2025, at 22:12, Kaxil Naik  wrote:
> 
> I am in favor of dropping support as they essentially do the same -- and
> setup & teardown is more "native" (first-class UI support)
> 
> On Fri, 28 Mar 2025 at 19:41, Kaxil Naik  wrote:
> 
>> Hi team,
>> 
>> Should we drop support for pre_execute and post_execute for AF 3.0? They
>> are still marked as experimental [1]. They were added [2] in a world
>> without Setup and Teardown tasks.
>> 
>> Regards,
>> 
>> Kaxil
>> 
>> 
>> [1]:
>> https://github.com/apache/airflow/blob/7af0319ba16749f4aea78085dfe7823f321d262a/task-sdk/src/airflow/sdk/bases/baseoperator.py#L715-L724
>> [2]: https://github.com/apache/airflow/pull/17576
>> 


-
To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
For additional commands, e-mail: dev-h...@airflow.apache.org



Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Kaxil Naik
Hi team,

Should we drop support for pre_execute and post_execute for AF 3.0? They
are still marked as experimental [1]. They were added [2] in a world
without Setup and Teardown tasks.

Regards,

Kaxil


[1]:
https://github.com/apache/airflow/blob/7af0319ba16749f4aea78085dfe7823f321d262a/task-sdk/src/airflow/sdk/bases/baseoperator.py#L715-L724
[2]: https://github.com/apache/airflow/pull/17576


Re: Should we drop support for pre_execute & post_execute for AF 3.0

2025-03-28 Thread Kaxil Naik
I am in favor of dropping support as they essentially do the same -- and
setup & teardown is more "native" (first-class UI support)

On Fri, 28 Mar 2025 at 19:41, Kaxil Naik  wrote:

> Hi team,
>
> Should we drop support for pre_execute and post_execute for AF 3.0? They
> are still marked as experimental [1]. They were added [2] in a world
> without Setup and Teardown tasks.
>
> Regards,
>
> Kaxil
>
>
> [1]:
> https://github.com/apache/airflow/blob/7af0319ba16749f4aea78085dfe7823f321d262a/task-sdk/src/airflow/sdk/bases/baseoperator.py#L715-L724
> [2]: https://github.com/apache/airflow/pull/17576
>