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