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 <michalmod...@google.com.invalid> 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 <kaxiln...@gmail.com> 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 <kaxiln...@gmail.com> 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 > > > >> <tamara.finger...@astronomer.io.invalid> 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 > > <https://teams.googleplex.com/u/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 <ja...@potiuk.com> > > > 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 < > bdbr...@gmail.com > > > > > > >> 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 <ja...@potiuk.com> > > > 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 < > kaxiln...@gmail.com> > > > >> >> 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. 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 > > > >> >>>> <t...@astronomer.io.invalid > > > >> >>>>>> > > > >> >>>>>> 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 <kaxiln...@gmail.com> > > > >> >>> 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 < > > kaxiln...@gmail.com> > > > >> >>>>> 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 > > > >> >>> > > > >> >> > > > >> > > > >> > --------------------------------------------------------------------- > > > >> 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 > > >
