Skip to content

Allow argument-provision checks of templated fields in operator __init__ - #70505

Open
shahar1 wants to merge 1 commit into
apache:mainfrom
shahar1:validate-init-provision-checks
Open

Allow argument-provision checks of templated fields in operator __init__#70505
shahar1 wants to merge 1 commit into
apache:mainfrom
shahar1:validate-init-provision-checks

Conversation

@shahar1

@shahar1 shahar1 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

related: #70296, #70347, #70297

Human Summary

As it turns out, the check I've introduced in #70297 might have been too strict, and we should allow some specific checks to still run in the constructor. This PR relaxes the requirements and removes false positives (according to the new definition) from the exemptions list.

Created a separate issue (#70503) to track reverts.

AI Summary

Click here

validate-operators-init flags every read of a template-field parameter in __init__. That is right for reads of the value, but wrong for reads of provision — "did the author pass this argument?" — which is what mutually-exclusive-argument checks ask.

The constructor is in fact the only place that can answer it. With render_template_as_native_obj=True a provided field renders to None, so the same check in execute() reports a supplied argument as missing. Forcing the move also turns a static authoring mistake into a per-task-instance runtime failure, and — because a rendered value can legitimately be falsy — invites truthiness tests that silently accept conflicting arguments.

This adds one sanctioned pattern: field is None / field is not None on a template-field parameter (or self.<field>) is allowed in __init__. Everything else — truthiness, in, .startswith, isinstance, transformations — stays flagged.

Measured against the burn-down. Replaying both checkers over every exemption entry ever removed (31 PRs, 48 entries): 8 entries across 5 PRs would have needed no operator code change at all — #70341, #70338, #70348, #70392, #70326 moved pure is None exclusivity checks into execute() verbatim. Two cost more than churn: #70348 broke the Rendered Templates view and needed follow-up #70373, and #70326 made a released operator argument required. Five currently-listed entries clear with no code change and are removed here; two of them are already claimed by contributors on #70296.

The remaining 40 entries stay flagged, correctly — they are genuine value reads (ruleset.strip(), isinstance(mongo_query, list), source not in SUPPORTED_SOURCES).

Second half of the change: template-field assignments are now walked recursively. Sanctioning the is None read would otherwise let if field is None: self.field = derive(other) through, since the assignment check only looked at top-level statements. That also closes a pre-existing hole with an unrelated guard.

Testing Done
  • scripts/tests/ci/prek/test_validate_operators_init.py: 35 passed. 8 of the 10 new/changed cases fail against the pre-change checker; the 2 that don't are guards against over-sanctioning (truthiness and foo.get('x') is None must stay flagged).
  • Hook run over all 541 files matching its files: regex: exit 0, no stale exemptions.
  • prek run --from-ref main --stage pre-commit: clean (includes mypy for scripts).
Reviewer note: reconciling merged precedent

Five merged burn-down PRs moved pure provision checks into execute() and left comments and regression tests asserting that placement. Nothing breaks — the new rule permits is None in __init__, it does not require it — but those files now demonstrate the pattern the docs discourage, and contributors on #70296 learn the rule by reading them. Reverting them is tracked separately so this PR stays reviewable.

No newsfragment: prek hook, contributing docs, and a clarification to an existing limitations section. Not user-facing behaviour.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5)

Generated-by: Claude Code (Opus 5) following the guidelines

Which arguments a Dag author passed is a question only the constructor can
answer: with render_template_as_native_obj=True a provided field can render to
None, so the same check in execute() reports a supplied argument as missing.
Forcing those checks out of __init__ also turns a static authoring mistake into
a per-task-instance runtime failure and, because a rendered value can
legitimately be falsy, invites truthiness tests that silently accept
conflicting arguments.

Assignments are now walked recursively so a branch cannot hide a derived
write to a template field behind a provision guard.
@shahar1
shahar1 force-pushed the validate-init-provision-checks branch from adfa421 to 0026dec Compare July 27, 2026 08:58
mitre88 added a commit to mitre88/airflow that referenced this pull request Jul 27, 2026
The provision checks in GCSDeleteObjectsOperator are false positives
under the relaxed validator definition proposed in apache#70505, so they stay
in the constructor and the class keeps its exemption entry.
@shahar1
shahar1 requested a review from vincbeck July 27, 2026 16:22
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