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
>
