Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]

["potiuk (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org