shahar1 opened a new issue, #70503:
URL: https://github.com/apache/airflow/issues/70503
### Background
`validate-operators-init` flags every read of a template-field parameter in
`__init__`. A companion PR narrows that: a `field is None` / `field is not
None` check reads whether the argument was *passed*, not its value, and belongs
in the constructor. (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; and raising at construction surfaces a static authoring mistake as
a Dag import error rather than per task instance.)
Replaying both checker versions over every exemption entry removed so far
(31 PRs, 48 entries) shows **8 entries across 5 PRs** where the old rule forced
a change that was not needed. Those PRs are merged and green — nothing is
broken — but the files now carry comments and regression tests asserting the
placement the docs discourage, and contributors working #70296 learn the rule
by reading them.
This issue tracks putting them back. Each is independent; separate PRs are
fine.
### Revert
- [ ] `providers/common/ai/.../operators/agent.py` (#70338) —
`message_history is not None and enable_hitl_review` is pure provision. Restore
to `__init__`, revert
`test_agent.py::test_message_history_with_hitl_review_raises` to
constructor-raises. The comment `# message_history is a template field;
validate the combination after rendering.` states the rule wrongly and must go.
- [ ] `providers/databricks/.../operators/databricks_repos.py` (#70341) —
all four `branch`/`tag`/`repo_id`/`repo_path` checks in
`DatabricksReposCreate/Update/DeleteOperator` are pure provision. Restore to
`__init__`; revert the two tests docstringed *"rejected at execute, after
rendering"*.
- [ ] `providers/neo4j/.../operators/neo4j.py` (#70348) — `if cypher is
None: raise` returns to `__init__`. The deprecation-warning placement is a
separate question; #70373's rationale stands independently.
### Split, don't revert
- [ ] `providers/common/ai/.../operators/document_loader.py` (#70338) — the
`source_path`/`source_bytes` exclusivity is provision and returns to
`__init__`. The third check, `source_bytes is not None and file_type is None`,
backs the downstream `assert self.file_type is not None`; `file_type` is a
template field, so that assumption only holds after rendering and it stays in
`execute()`. Update the comment to say which is which — this is the canonical
example of the boundary.
### Leave alone (listed so nobody reverts them by pattern-match)
- `providers/weaviate/.../operators/weaviate.py` (#70326) — making
`input_data` required is already released; reverting would be a second API
break. The `execute()` check becomes belt-and-braces and is harmless.
- `providers/teradata/.../transfers/teradata_to_teradata.py` (#70392) —
`self.sql_params or {}` at the use site is equivalent. No value in churning it
back.
### Not in scope
The other 40 exemption entries removed so far are genuine value reads
(`ruleset.strip()`, `isinstance(mongo_query, list)`, `source not in
SUPPORTED_SOURCES`, ...). They stay flagged under the narrowed rule and must
not be reverted.
### Verifying a revert
Run the hook on the file after the change — all six classes above are clean
under the narrowed rule, so no exemption entries need re-adding:
```
uv run --script scripts/ci/prek/validate_operators_init.py <path>
```
related: #70296
---
Drafted-by: Claude Code (Opus 5) (no human review before posting)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
