Skip to content

Build Dataproc submit job trigger arguments after template rendering - #71318

Closed
jerryzhou196 wants to merge 1 commit into
apache:mainfrom
jerryzhou196:dataproc-submit-job-start-trigger-args
Closed

Build Dataproc submit job trigger arguments after template rendering#71318
jerryzhou196 wants to merge 1 commit into
apache:mainfrom
jerryzhou196:dataproc-submit-job-start-trigger-args

Conversation

@jerryzhou196

Copy link
Copy Markdown
Contributor

DataprocSubmitJobOperator.__init__ built start_trigger_args from job, project_id,
region, gcp_conn_id, impersonation_chain and request_id — all of them template
fields, so in the constructor they still hold the un-rendered Jinja expression. Those raw
strings were then what the operator advertised (and what got serialized) as the arguments
for starting directly in the triggerer.

Reproducer on main:

op = DataprocSubmitJobOperator(
    task_id="submit",
    region="{{ params.region }}",
    project_id="{{ params.project }}",
    job={"reference": {"job_id": "job-{{ ds }}"}},
    deferrable=True,
    start_from_trigger=True,
)
op.start_trigger_args.trigger_kwargs
# {'job': {'reference': {'job_id': 'job-{{ ds }}'}}, 'project_id': '{{ params.project }}',
#  'region': '{{ params.region }}', ...}

Rendering the fields afterwards does not help — trigger_kwargs keeps the parse-time copy.

start_trigger_args is now built in render_template_fields(), the first method that runs
after rendering, so it holds the rendered values. It is created with dataclasses.replace
so the class-level template is left untouched for the next task built from the operator.

Regression tests are added for the rendered values and for the per-task isolation; the first
fails on main.

Removes the class from scripts/ci/prek/validate_operators_init_exemptions.txt.

related: #70296


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

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

@boring-cyborg boring-cyborg Bot added area:dev-tools area:providers backport-to-v3-3-test Backport to v3-3-test provider:google Google (including GCP) related issues labels Aug 7, 2026
@jerryzhou196
jerryzhou196 force-pushed the dataproc-submit-job-start-trigger-args branch from 067d1e8 to a6b8b30 Compare August 8, 2026 16:50
@jerryzhou196
jerryzhou196 force-pushed the dataproc-submit-job-start-trigger-args branch from a6b8b30 to fe9c096 Compare August 8, 2026 17:23
@jerryzhou196

Copy link
Copy Markdown
Contributor Author

Closing this — the change is wrong.

With `start_from_trigger=True` the operator never runs on a worker, so `render_template_fields()` is never called on it. The scheduler reads `start_trigger_args` straight off the serialized DAG (`serialized_objects.py`, captured at parse time right after `init`) and `TaskInstance.defer_task()` builds the `Trigger` row from `trigger_kwargs`. Building the kwargs in `render_template_fields()` would ship an empty `trigger_kwargs` in the serialized DAG and the trigger would be instantiated without `job`/`region`/`project_id`.

The un-rendered Jinja in `trigger_kwargs` is deliberate: `airflow-core/src/airflow/triggers/base.py` filters the operator's `template_fields` down to the keys present in `start_trigger_args.trigger_kwargs` that also exist on the trigger, and the triggerer calls `BaseTrigger.render_template_fields()` before running it. Rendering for this path happens in the triggerer, not on a worker.

Follow-up PR narrows the `validate-operators-init` hook to sanction this pattern instead.


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

@mihirduvedi

Copy link
Copy Markdown
Contributor

I checked this against triggers/base.py and your reading holds up. It filters self.task.template_fields down to the keys that appear in start_trigger_args.trigger_kwargs and also exist on the trigger (lines 115-137), then renders them in render_template_fields() at 142. The un-rendered Jinja in trigger_kwargs is what that filter keys on, so it has to be captured at parse time.

One thing for the follow-up: narrowing the hook means the diagnostic in _check_constructor_field_logic() needs a third case. I wrote the current wording in #71210 and it only covers two: value-dependent reads and provision checks.


Drafted-by: Claude (Opus 5); reviewed by @mihirduvedi before posting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools area:providers backport-to-v3-3-test Backport to v3-3-test provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants