Skip to content

Move template-field validation/transformation out of operator __init__ (exemption-list burn-down) #70296

Description

@shahar1

Background

Template fields are rendered after an operator's constructor runs. Any logic applied to a template-field parameter's value inside __init__ — validation, type checks, transformation, string interpolation — therefore operates on the un-rendered Jinja expression, not the real value.

This is documented in Creating a custom Operator and contributing-docs/05_pull_requests.rst, and it causes real bugs: #69813 (SSHRemoteJobOperator validated a templated remote_base_dir in __init__, so cleanup validation failed for any custom base) is a recent example.

Important

Checks that only ask whether an argument was passed are the exception and belong in __init__. Read False positives below before fixing an entry.

The validate-operators-init prek hook previously only verified that template fields are assigned verbatim at the top level of __init__; it could not see validation calls, conditionals, or transformations (and it did not cover sensors, AwsBaseOperator[...] subclasses, or aws_template_fields(...)-based classes at all). The hook has been extended to detect any non-sanctioned read of a template field in __init__ (PR to follow).

The exemption ratchet

Existing violations are listed in scripts/ci/prek/validate_operators_init_exemptions.txt as path::ClassName entries so the hook can enforce the rule on new code immediately.

  • A PR that fixes a class must remove its entry in the same PR — the hook fails on stale exemptions, so this cannot be forgotten.
  • New violations cannot be added: only files listed in the exemption file are suppressed.
  • This issue is done when the exemption file is empty.

How to fix a class: move the validation / transformation from __init__ into execute() (or the first method that runs after rendering), keep __init__ down to plain self.field = field assignments (defaulting via field or default is fine). Don't forget the corresponding tests.

Please do not open sub-issues for individual entries — comment here or just open a PR referencing this issue.

False positives

Not everything the hook flags should move. A check that only asks whether an argument was passed — the usual "exactly one of a or b" guard — belongs in __init__ and must not be moved:

  • The constructor is 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.
  • Raising at construction surfaces a static authoring mistake as a Dag import error, instead of on a worker once per task instance and per retry.

Fix these by rewriting in place, not by moving. Use the is not None polarity — if field: is a truthiness test on the un-rendered string and asks a third question that matches neither:

# in __init__ — correct
if not exactly_one(command is not None, powershell is not None, cmdlet is not None):
    raise ValueError("Must provide exactly one of 'command', 'powershell', or 'cmdlet'")

Do not write exactly_one(command is None, ...). It happens to agree for two arguments but is wrong for three or more: it rejects the valid single-argument case and accepts two arguments at once.

Anything that inspects the value — a range check, a format check, an in test against allowed values, .strip(), isinstance(...) — still moves to execute(). And a provision check in __init__ guarantees nothing about the rendered value: code in execute() that needs the field set still needs its own guard.

The hook did not know this distinction until it was narrowed. These five entries were flagged only for a provision check and clear with no operator code change — they are removed from the exemption file by the narrowing PR, so please don't pick them up:

class file the flagged check
EmrAddStepsOperator emr.py exactly_one(job_flow_id is None, job_flow_name is None)
GCSDeleteObjectsOperator gcs.py objects / prefix exclusivity
GCSToLocalFilesystemOperator gcs_to_local.py filename / store_to_xcom_key exclusivity
OracleToAzureDataLakeOperator oracle_to_azure_data_lake.py sql_params default
OracleToOracleOperator oracle_to_oracle.py source_sql_params default

Five already-merged fixes moved a provision check that should have stayed. Nothing is broken, but those files now demonstrate the discouraged pattern — reverting them is tracked in #70503. Please don't use them as a model.

S3DeleteObjectsOperator needs care and is worth reading before you touch it. Its check is a pure provision check, but it hides the comparisons in a comprehension (all(var is None for var in [...])) that the hook does not recognise, so the entry stays even after the narrowing. It also already exists twice — in __init__ and in execute() — and both copies must stay:

  • The execute() copy is load-bearing. keys = self.keys or self.hook.list_keys(prefix=..., from_datetime=..., to_datetime=...) lists the whole bucket when every filter is None, and the next line deletes it. A templated keys that renders to None (native rendering) reaches that state past a correct __init__ check.
  • The __init__ copy still catches the static authoring mistake at Dag parse time rather than on a worker.

To clear the entry, unroll the __init__ copy so every read is a direct comparison — don't delete it, and don't touch execute():

by_scan = prefix is not None or from_datetime is not None or to_datetime is not None
if not exactly_one(keys is not None, by_scan):
    raise ValueError("Either keys or at least one of prefix, from_datetime, to_datetime should be set.")

That is semantically identical to the current condition on every input combination (including keys=[]) and clears the hook.

Current exemption list (snapshot)

The authoritative list is the exemptions file; this checklist is the snapshot at the time the check was introduced. 83 classes across 22 providers, of which the 5 listed under False positives above need no code change.

amazon

anthropic

  • AnthropicAgentSessionOperatoragent.py — logic in __init__

apache/hive

apache/kafka

  • ProduceToTopicOperatorproduce.py — logic in __init__

apache/spark

cncf/kubernetes

  • KubernetesInstallKueueOperatorkueue.py — logic in __init__
  • KubernetesPodOperatorpod.py — logic in __init__
  • KubernetesResourceBaseOperatorresource.py — logic in __init__

cohere

  • CohereEmbeddingOperatorembedding.py — logic in __init__

common/ai

databricks

dbt/cloud

  • DbtCloudGetJobRunArtifactOperatordbt.py — logic in __init__

docker

  • DockerOperatordocker.py — logic in __init__

google

microsoft/azure

microsoft/psrp

neo4j

  • Neo4jOperatorneo4j.py — logic in __init__

papermill

snowflake

ssh

standard

  • BashOperatorbash.py — logic in __init__
  • DateTimeSensordate_time.py — logic in __init__, missing assignment
  • HITLOperatorhitl.py — logic in __init__
  • TriggerDagRunOperatortrigger_dagrun.py — logic in __init__

teradata

weaviate

  • WeaviateIngestOperatorweaviate.py — logic in __init__

Drafted-by: Claude Code (Opus 4.8) (no human review before posting)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions