Skip to content

Add common.compat alias for BaseEventTrigger - #72140

Open
amoghrajesh wants to merge 2 commits into
apache:mainfrom
astronomer:common-compat-base-event-trigger
Open

Add common.compat alias for BaseEventTrigger#72140
amoghrajesh wants to merge 2 commits into
apache:mainfrom
astronomer:common-compat-base-event-trigger

Conversation

@amoghrajesh

Copy link
Copy Markdown
Contributor

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Each provider repeats a variant of:

if AIRFLOW_V_3_0_PLUS:
    from airflow.triggers.base import BaseEventTrigger, TriggerEvent
else:
    from airflow.triggers.base import (  # type: ignore[assignment]
        BaseTrigger as BaseEventTrigger,
        TriggerEvent,
    )

Several of these also drag TriggerEvent into the version branch even though it exists in both 2.x and 3.x, and pull in an AIRFLOW_V_3_0_PLUS import that isn't used elsewhere.

I am proposing that we add a airflow.providers.common.compat.triggers, which resolves BaseEventTrigger through the existing create_module_getattr rename machinery and falls back to BaseTrigger on Airflow 2.x. Call sites collapse to a single import.

The iceberg trigger is migrated as the first consumer: #71387


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@1fanwang 1fanwang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @amoghrajesh LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants