@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 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 >> >>
