Consensus has been reached. You can see the change here:
https://github.com/apache/airflow/pull/46391
Thanks.
On 2025/01/31 00:09:15 Vikram Koka wrote:
> Vincent,
>
> Thank you for taking the feedback and the quick update.
>
> Vikram
>
> On 2025/01/29 19:12:17 "Beck, Vincent" wrote:
> > Hello everyone,
> >
> > During my AIP-82 demo at the last Airflow 3 dev call, I highlighted
> > potential issues if users select an incorrect trigger when scheduling DAGs
> > using event-driven scheduling. For example, using `S3KeyTrigger`, which
> > fires whenever a specific file exists in an S3 bucket:
> >
> > ```
> > trigger = S3KeyTrigger(bucket_name="<my-bucket>", bucket_key="<my-file>")
> > asset = Asset("s3_asset", watchers=[
> > AssetWatcher(name="s3_asset_watcher", trigger=trigger)
> > ])
> >
> > with DAG(
> > dag_id="example_dag",
> > schedule=[asset],
> > catchup=False,
> > ):
> > ...
> > ```
> >
> > This setup would schedule the DAG `example_dag` whenever `<my-file>` exists
> > in `<my-bucket>`, and as long as it exists. If the file is never deleted,
> > the DAG will continue scheduling indefinitely. While this is not a bug, it
> > is a potential misuse of the feature that we should mitigate to prevent
> > users from running into this issue.
> >
> > After a discussion on Slack
> > (https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1737999509008529),
> > the following solution emerged: introduce a new base class,
> > `BaseEventTrigger`, which will be an empty subclass of `BaseTrigger`:
> >
> > ```
> > class BaseEventTrigger(BaseTrigger):
> > ...
> > ```
> >
> > The `AssetWatcher` signature will be updated to accept only
> > `BaseEventTrigger` instead of `BaseTrigger`. This means that only triggers
> > inheriting from `BaseEventTrigger` can be used to schedule DAGs based on
> > events.
> >
> > Benefits:
> > - We limit the set of triggers users can use with event driven scheduling.
> > Allowing all triggers to be used to schedule DAGs does not make sense and
> > can lead to some bigger issues (scheduling DAGs indefinitely.)
> > - Developers can explicitly mark a trigger as suitable for event-driven
> > scheduling by inheriting `BaseEventTrigger`
> > - If no modifications are needed, the developer simply updates the class
> > to inherit from `BaseEventTrigger`
> > - If modifications are needed, a new trigger class can be created,
> > inheriting from both the existing trigger and `BaseEventTrigger`, with
> > event-driven scheduling logic handled separately.
> >
> > The lazy consensus will be reached on 2025-02-03 (unless someone surfaces
> > an objection).
> >
> > Vincent
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
