Add static check ensuring trigger __init__() and serialize() stay in sync#66960
Merged
Merged
Conversation
6 tasks
This was referenced May 14, 2026
Lee-W
reviewed
May 15, 2026
Closed
1 task
3730302 to
36f07ba
Compare
__init__() and serialize() stay in sync
1 task
Trigger __init__ and serialize() are written as a pair: any __init__ parameter that serialize() does not return is silently dropped when the triggerer re-instantiates the trigger, falling back to the parameter's default. This adds an AST-based prek static check over provider triggers that flags such mismatches, resolving __init__/serialize() pairs through in-file base classes (including **super().serialize() spreads). Five existing violations are excluded as KNOWN_VIOLATIONS pending a follow-up fix; three by-design cases (deprecated/aliased params folded into their replacement at construction time) are permanently excluded.
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
Rename based on Lee-W's review feedback: - _init_param_names → _get_init_param_names - _base_simple_names → _get_base_simple_names - _method → _get_method - _serialize_keys → _get_serialize_keys - _super_serialize_keys → _get_super_serialize_keys - violations → get_violations
36f07ba to
ac6ed56
Compare
potiuk
approved these changes
May 16, 2026
Contributor
Backport successfully created: v3-2-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
github-actions Bot
pushed a commit
to aws-mwaa/upstream-to-airflow
that referenced
this pull request
May 16, 2026
…lize()` stay in sync (apache#66960) * Add static check ensuring trigger __init__ and serialize() stay in sync Trigger __init__ and serialize() are written as a pair: any __init__ parameter that serialize() does not return is silently dropped when the triggerer re-instantiates the trigger, falling back to the parameter's default. This adds an AST-based prek static check over provider triggers that flags such mismatches, resolving __init__/serialize() pairs through in-file base classes (including **super().serialize() spreads). Five existing violations are excluded as KNOWN_VIOLATIONS pending a follow-up fix; three by-design cases (deprecated/aliased params folded into their replacement at construction time) are permanently excluded. * Update scripts/ci/prek/check_trigger_serialize_init.py Co-authored-by: Wei Lee <weilee.rx@gmail.com> * Rename noun-style helper functions to _get_/get_ form Rename based on Lee-W's review feedback: - _init_param_names → _get_init_param_names - _base_simple_names → _get_base_simple_names - _method → _get_method - _serialize_keys → _get_serialize_keys - _super_serialize_keys → _get_super_serialize_keys - violations → get_violations --------- (cherry picked from commit 49958a5) Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com>
aws-airflow-bot
pushed a commit
to aws-mwaa/upstream-to-airflow
that referenced
this pull request
May 16, 2026
…lize()` stay in sync (apache#66960) * Add static check ensuring trigger __init__ and serialize() stay in sync Trigger __init__ and serialize() are written as a pair: any __init__ parameter that serialize() does not return is silently dropped when the triggerer re-instantiates the trigger, falling back to the parameter's default. This adds an AST-based prek static check over provider triggers that flags such mismatches, resolving __init__/serialize() pairs through in-file base classes (including **super().serialize() spreads). Five existing violations are excluded as KNOWN_VIOLATIONS pending a follow-up fix; three by-design cases (deprecated/aliased params folded into their replacement at construction time) are permanently excluded. * Update scripts/ci/prek/check_trigger_serialize_init.py Co-authored-by: Wei Lee <weilee.rx@gmail.com> * Rename noun-style helper functions to _get_/get_ form Rename based on Lee-W's review feedback: - _init_param_names → _get_init_param_names - _base_simple_names → _get_base_simple_names - _method → _get_method - _serialize_keys → _get_serialize_keys - _super_serialize_keys → _get_super_serialize_keys - violations → get_violations --------- (cherry picked from commit 49958a5) Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com>
vatsrahul1001
pushed a commit
that referenced
this pull request
May 20, 2026
…lize()` stay in sync (#66960) * Add static check ensuring trigger __init__ and serialize() stay in sync Trigger __init__ and serialize() are written as a pair: any __init__ parameter that serialize() does not return is silently dropped when the triggerer re-instantiates the trigger, falling back to the parameter's default. This adds an AST-based prek static check over provider triggers that flags such mismatches, resolving __init__/serialize() pairs through in-file base classes (including **super().serialize() spreads). Five existing violations are excluded as KNOWN_VIOLATIONS pending a follow-up fix; three by-design cases (deprecated/aliased params folded into their replacement at construction time) are permanently excluded. * Update scripts/ci/prek/check_trigger_serialize_init.py Co-authored-by: Wei Lee <weilee.rx@gmail.com> * Rename noun-style helper functions to _get_/get_ form Rename based on Lee-W's review feedback: - _init_param_names → _get_init_param_names - _base_simple_names → _get_base_simple_names - _method → _get_method - _serialize_keys → _get_serialize_keys - _super_serialize_keys → _get_super_serialize_keys - violations → get_violations --------- (cherry picked from commit 49958a5) Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com>
vatsrahul1001
pushed a commit
that referenced
this pull request
May 21, 2026
…lize()` stay in sync (#66960) * Add static check ensuring trigger __init__ and serialize() stay in sync Trigger __init__ and serialize() are written as a pair: any __init__ parameter that serialize() does not return is silently dropped when the triggerer re-instantiates the trigger, falling back to the parameter's default. This adds an AST-based prek static check over provider triggers that flags such mismatches, resolving __init__/serialize() pairs through in-file base classes (including **super().serialize() spreads). Five existing violations are excluded as KNOWN_VIOLATIONS pending a follow-up fix; three by-design cases (deprecated/aliased params folded into their replacement at construction time) are permanently excluded. * Update scripts/ci/prek/check_trigger_serialize_init.py Co-authored-by: Wei Lee <weilee.rx@gmail.com> * Rename noun-style helper functions to _get_/get_ form Rename based on Lee-W's review feedback: - _init_param_names → _get_init_param_names - _base_simple_names → _get_base_simple_names - _method → _get_method - _serialize_keys → _get_serialize_keys - _super_serialize_keys → _get_super_serialize_keys - violations → get_violations --------- (cherry picked from commit 49958a5) Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com>
5 tasks
potiuk
pushed a commit
that referenced
this pull request
Jun 3, 2026
…lize()` stay in sync (#66960) * Add static check ensuring trigger __init__ and serialize() stay in sync Trigger __init__ and serialize() are written as a pair: any __init__ parameter that serialize() does not return is silently dropped when the triggerer re-instantiates the trigger, falling back to the parameter's default. This adds an AST-based prek static check over provider triggers that flags such mismatches, resolving __init__/serialize() pairs through in-file base classes (including **super().serialize() spreads). Five existing violations are excluded as KNOWN_VIOLATIONS pending a follow-up fix; three by-design cases (deprecated/aliased params folded into their replacement at construction time) are permanently excluded. * Update scripts/ci/prek/check_trigger_serialize_init.py Co-authored-by: Wei Lee <weilee.rx@gmail.com> * Rename noun-style helper functions to _get_/get_ form Rename based on Lee-W's review feedback: - _init_param_names → _get_init_param_names - _base_simple_names → _get_base_simple_names - _method → _get_method - _serialize_keys → _get_serialize_keys - _super_serialize_keys → _get_super_serialize_keys - violations → get_violations --------- (cherry picked from commit 49958a5) Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com>
potiuk
added a commit
that referenced
this pull request
Jun 4, 2026
…lize()` stay in sync (#66960) (#67048) * [v3-2-test] Skip test_schedule_tis_start_trigger pending #55068 backport decision (#66315) The `start_from_trigger` deferred-at-schedule path is commented out on v3-2-test (disabled in 91e1029 as a TODO) and was only re-enabled on main by #55068, which landed one day after the v3-2 branch was cut. The test asserts the feature works and fails on v3-2-test CI. Skip the test on the 3.2 line and link the tracking issue from both the test skip-reason and the disabled production-code site so the follow-up isn't lost. Decision (backport #55068 or formally drop the feature on 3.2) is tracked in #66307. * [v3-2-test] Add static check ensuring trigger `__init__()` and `serialize()` stay in sync (#66960) * Add static check ensuring trigger __init__ and serialize() stay in sync Trigger __init__ and serialize() are written as a pair: any __init__ parameter that serialize() does not return is silently dropped when the triggerer re-instantiates the trigger, falling back to the parameter's default. This adds an AST-based prek static check over provider triggers that flags such mismatches, resolving __init__/serialize() pairs through in-file base classes (including **super().serialize() spreads). Five existing violations are excluded as KNOWN_VIOLATIONS pending a follow-up fix; three by-design cases (deprecated/aliased params folded into their replacement at construction time) are permanently excluded. * Update scripts/ci/prek/check_trigger_serialize_init.py Co-authored-by: Wei Lee <weilee.rx@gmail.com> * Rename noun-style helper functions to _get_/get_ form Rename based on Lee-W's review feedback: - _init_param_names → _get_init_param_names - _base_simple_names → _get_base_simple_names - _method → _get_method - _serialize_keys → _get_serialize_keys - _super_serialize_keys → _get_super_serialize_keys - violations → get_violations --------- (cherry picked from commit 49958a5) Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com> * Fix SageMaker/MLEngine triggers dropping params on triggerer restart The new trigger __init__/serialize() sync check surfaced two pre-existing violations on v3-2-test (the triggers do not exist or are already correct on main, so they were not in the cherry-picked KNOWN_VIOLATIONS list): - SageMakerNotebookJobTrigger.serialize() returned a non-existent self.poll_interval (an AttributeError on triggerer round-trip) and dropped waiter_delay / waiter_max_attempts. Serialize the stored waiter_* params instead, matching the fixed version on main. - MLEngineStartTrainingJobTrigger.serialize() dropped gcp_conn_id and impersonation_chain, silently resetting them to defaults when the triggerer re-instantiates the trigger. Serialize both. This makes the backported check pass without adding new KNOWN_VIOLATIONS. --------- Co-authored-by: Jarek Potiuk <jarek@potiuk.com> Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com> Co-authored-by: Wei Lee <weilee.rx@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A trigger's
__init__andserialize()are written as a pair: any__init__parameter thatserialize()does not return is silently dropped when the triggerer re-instantiates the trigger, falling back to the parameter's default. This kind of mismatch is easy to introduce and hard to spot in review.This adds an AST-based prek static check over provider trigger modules (
*/src/airflow/providers/*/triggers/*.py) that flags such mismatches.Inspiration for this PR: #66917
Details
__init__/serialize()pair through in-file base classes, so a class that inherits one and overrides the other is still checked. Also follows**super().serialize()[1]spreads recursively..update(), base class in another file), it skips rather than guessing — no false positives.KNOWN_VIOLATIONSand excluded for now; they will be fixed in a follow-up PR (DatabricksExecutionTrigger,DatabricksSQLStatementExecutionTrigger,BigQueryIntervalCheckTrigger,DataFusionStartPipelineTrigger,LivyTrigger).BY_DESIGN_EXCLUSIONSwith documented reasons — a deprecated/aliased parameter (delta→moment,pod_name→pod_names) is folded into its replacement at construction time, so it does not need to round-trip.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Sonnet 4.6) following the guidelines