Allow template-field copies into StartTriggerArgs in operator __init__ - #73040
Merged
Merged
Conversation
shahar1
requested review from
amoghrajesh,
ashb,
bugraoz93,
gopidesupavan,
jason810496,
jscheffl and
potiuk
as code owners
September 12, 2026 17:53
shahar1
force-pushed
the
sanction-start-trigger-args-reads
branch
from
September 12, 2026 18:54
ff1602c to
019846a
Compare
This was referenced Sep 12, 2026
shahar1
force-pushed
the
sanction-start-trigger-args-reads
branch
from
September 14, 2026 15:19
019846a to
a919643
Compare
shahar1
commented
Sep 14, 2026
potiuk
approved these changes
Sep 22, 2026
With start_from_trigger, the scheduler hands trigger_kwargs straight to the triggerer, which renders the operator's template fields itself. Copying the un-rendered fields at construct time is therefore the intended data flow, and there is nothing to move to execute(). The validate-operators-init hook could not tell such a copy from a transformation and flagged DataprocSubmitJobOperator for it, leaving an entry in the exemption list that no operator change can clear without hiding the reads from the hook. Generated-by: Claude Code (Fable 5.1)
Only trigger_kwargs entries whose key is an operator template field and an attribute of the trigger are rendered by the triggerer. timeout, next_kwargs, a mismatched key, or ** unpacking are never rendered, so a template field in those positions is still a genuine constructor read and must stay flagged. Also documents the exception alongside the other sanctioned constructor patterns, as the provision-check narrowing did. Generated-by: Claude Code (Fable 5.1)
Resolving the callable through _resolve_base_name accepted any attribute whose last segment matched, so factory.StartTriggerArgs, helper.replace and helper.dict were treated as the real constructor, dataclasses.replace and the builtin dict. Only the bare names, dataclasses.replace and copy.replace are the forms the triggerer contract is written against; anything else is an unrelated callable that could transform the unrendered template value and is now flagged.
The sanction accepts "copy" as a qualifier for replace and deliberately leaves positional StartTriggerArgs arguments unmatched, but neither edge was covered: dropping "copy" from the qualifier tuple or extending the trigger_kwargs scan to positional arguments kept the suite green, so a later refactor could widen or narrow the sanction unnoticed. Generated-by: Claude Code (Claude Opus 5)
Both guards that keep the StartTriggerArgs sanction narrow were only covered by cases that another condition already rejected, so the suite stayed green when either guard was removed. The keyword guard was covered by timeout=foo, whose value never reaches the dict walk, and the verbatim-value conjunct by a key the single-field harness rejects on membership alone. The new cases fail without each guard. The docs claimed a copy under a different key is never rendered. It is, when that key is itself a template field of the operator; the hook flags it because the value stored there is not the field the key names. Generated-by: Claude Code (Claude Opus 5)
Four conditions that keep the sanction to the one assignment the triggerer actually renders could each be deleted with the suite still green: the bare-name arm of _is_call_to, the two conjuncts that require the target to be self.start_trigger_args, and the one that requires dataclasses.replace to copy start_trigger_args itself. Each deletion sanctions a shape whose trigger_kwargs are never rendered, so the hook would start waving through un-rendered reads. The existing bare-name case used factory.StartTriggerArgs, which only exercises the attribute arm. Three further cases pin the guards that keep a parseable but odd constructor from crashing the hook instead of reporting it. The membership half of "key in template_fields and _target_name(item) == key" cannot change a verdict: a node the logic check would report resolves to a name that is already in template_fields, and the second conjunct ties the key to that name. Dropping it leaves every verdict unchanged across the 554 files the hook covers. The note about positional StartTriggerArgs arguments sat on the function that does match them; it describes where trigger_kwargs are read, so it moves there. Generated-by: Claude Code (Claude Opus 5)
shahar1
force-pushed
the
sanction-start-trigger-args-reads
branch
from
September 22, 2026 12:34
a919643 to
1a9722a
Compare
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
3 participants
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.
Human Summary
There's a false positive in
validate-operators-initpre-commit, where we need to use the templated form when initializingStartFromArgsobject within the operator's constructor. I found this use case only inDataprocSubmitJobOperator.This PR exempts this case.
AI Summary
Click here
`validate-operators-init` flags every non-sanctioned read of a template field in `__init__`, on the premise that such a read acts on the un-rendered Jinja expression and belongs in `execute()`. `DataprocSubmitJobOperator` is flagged for six reads that only copy template fields verbatim into `start_trigger_args.trigger_kwargs`. With `start_from_trigger`, the scheduler hands those kwargs straight to the triggerer, which renders the operator template fields itself (`BaseTrigger.task_instance` setter in `airflow-core/src/airflow/triggers/base.py`), so copying them un-rendered at construct time is the intended data flow and there is nothing to move. The entry cannot be cleared by an operator change without hiding the reads from the hook (see the discussion on #70296 from 30 July and the approach in #72083).This teaches the hook that verbatim reads inside
StartTriggerArgs(...)ordataclasses.replace(self.start_trigger_args, ...)are sanctioned (one level into{...}/dict(...); a transformation inside them is still flagged), and removes theDataprocSubmitJobOperatorexemption. The operator itself is unchanged. The same narrowing approach was used for argument-provision checks in #70505.Verified locally:
prek run validate-operators-init --all-filespasses with the change and fails on the six Dataproc reads with the hook change stashed and the exemption removed;uv run --project scripts pytest scripts/tests/ci/prek/test_validate_operators_init.pypasses (53 tests, 4 new cases).related: #70296
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Fable 5.1) following the guidelines
🤖 Generated with Claude Code