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
