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]
