potiuk commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-5464457372
Picking this up after too long a silence — apologies, @Vamsi-klu's question
has been sitting unanswered since 19 July.
First, to close out my original review: @moomindani's analysis settles it,
and against this PR. `_launch_job` is never reached from a
`DatabricksWorkflowTaskGroup` member — the workflow branch of `execute()` only
pulls and monitors the launch task's run metadata, and the workflow JSON is
built earlier by the auto-added launch task via
`_CreateDatabricksWorkflowOperator`. `render_template_fields` does run for
every member, but inside that member's own task instance, which starts after
the job already exists. So the render pass added here cannot affect the
reported scenario. @jroachgolf84 reached the same conclusion when he moved this
back to draft.
### On the context semantics
> The member task instance does not exist when the launch task builds the
workflow, so task-scoped expressions such as `{{ ti }}` would otherwise resolve
against the launch task.
Launch-task context is the right contract, and I think it is the only one
the architecture permits.
One Databricks job is created for the whole group before any member task
instance exists, so task-scoped expressions (`{{ ti }}`, `{{ task_instance }}`,
and anything derived from them such as XCom pulls) have no member TI to resolve
against. There is no correct value available at that point — only a misleading
one taken from the launch task.
Run-scoped expressions are a different matter: `{{ ds }}`, `{{ dag_run }}`,
`{{ params }}`, `{{ data_interval_start }}` and friends are fully defined when
the launch task runs, and are identical for every member of the group. Those
cover the case in #46563, where the fields in question are `application_args`
and `spark_conf`.
So the contract I would like to see implemented and documented:
- Render each member's declared `template_fields` against the **launch
task's** context while building the workflow JSON.
- **Document** that task-scoped expressions are not supported for
workflow-group members, rather than silently resolving them against the launch
task. A user who writes `{{ ti.xcom_pull(...) }}` in a member field should find
out from the docs, not from a value that is quietly wrong.
If a later change needs genuine per-member rendering, that requires
deferring JSON construction until the member TIs exist, which is a much larger
change to how the task group launches. Not something to solve here.
### Carrying it forward
@Vamsi-klu — please go ahead with the replacement patch on that basis: drop
the whole-payload render from `_launch_job`, render each child's declared
`template_fields` while building the workflow JSON, and add the
`DatabricksWorkflowTaskGroup` regression test. A fresh PR from your fork is
cleaner than granting access to this branch, since the approach differs from
what is here.
@jroachgolf84 — thank you for the original investigation and for converting
this to draft rather than pushing it through; the analysis in this thread is
what made the real path findable. Would you be happy for @Vamsi-klu to carry
it, with this PR closed once the replacement is open?
Please link the new PR back to this one and to #46563 so the reasoning above
does not have to be rediscovered a third time.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk 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]
