You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jroachgolf84 (via GitHub)" <gi...@apache.org> on 2026/04/26 12:38:56 UTC
[PR] issue-46563: Render `template_fields` on job launch [airflow]
jroachgolf84 opened a new pull request, #65869:
URL: https://github.com/apache/airflow/pull/65869
## Description
Per #46563:
> While creating workflow in Databricks using DatabricksWorkflowTaskGroup, I have a situation to pass some parameters to spark submit which are passed as jinja template.
The user had created a custom Operator that inherited from the `DatabricksBaseTaskOperator`. They specified templated fields that were NOT being rendered when the job launched, but WERE being rendered when executed.
This PR updates the `_launch_job` to render template fields before submitting the run (launching the operation in Databricks).
closes: #46563
##### Was generative AI tooling used to co-author this PR?
No, generative AI tooling was not used to co-author this PR. Keeping it old-fashioned :)
--
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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-4346012215
@eladkal - do you mind taking a look at this?
--
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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-5003439339
Converting back to a draft, as this is not ready to go yet.
--
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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-5464465880
@potiuk, yes, that sounds good to me. Thanks for the follow up!
--
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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-6026072811
Closing this PR for right now.
--
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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-4348717296
@moomindani - thanks for the feedback. I think you're right, we may need to dig into this a bit more. The example in issue #46563 was actually inheriting from the `DatabricksTaskBaseOperator`, despite having `Workflow` in the class name. This is was the sample DAG that I tested with (and was included in the unit-tests). Here's that DAG below.
```python
class CustomDatabricksWorkflowTaskOperator(DatabricksTaskBaseOperator):
"""Custom operator wrapping DatabricksTaskOperator"""
template_fields = (
"application_args",
"spark_conf",
"driver_jvm_props",
)
def __init__(
self,
*args,
class_name: str = "",
spark_conf: dict[str, str] | str = None,
application_args: List[str] | str | None = None,
driver_jvm_props: dict[str, str] | None = None,
application_conf_file: str = "test.conf",
libraries: List[dict[str, str]] = None,
**kwargs,
):
self.class_name = class_name
self.application_conf_file = application_conf_file
self.library = libraries if libraries is not None else _get_default_libraries()
self.application_args = application_args
super().__init__(*args, **kwargs)
self.driver_jvm_props = driver_jvm_props
self.spark_conf = spark_cone
def _get_task_base_json(self) -> dict[str, Any]:
return {
"run_if": self.databricks_trigger_rule,
'spark_jar_task': {
'main_class_name': self.class_name,
parameters": [
self.application_args,
"{{ get_test_template_value(ti,'test_key') }}" // I have hard-coded here for ref. In ideal case, this comes as part of self.application_args which has been added in template_fields
]
},
'libraries': self.library,
}
def execute(self, context):
super().execute(context)
```
Regardless if tasks are "standalone" or if they are part of a Workflow TaskGroup, Airflow will ensure that `render_template_fields(context)` is called on them. So for the use-case that you're mentioning, we should be covered. `_launch_job` could still be called by these tasks in the `DatabricksWorkflowTaskGroup`, and this PR covers that.
As I type this out, I think we're covered. But let me review a bit more tomorrow.
--
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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-4387287213
> As I type this out, I think we're covered. But let me review a bit more tomorrow.
ping me when ready to revisit
--
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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "Vamsi-klu (via GitHub)" <gi...@apache.org>.
Vamsi-klu commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-5014062861
I rechecked this against current `main`. I can prepare a replacement patch that removes the whole-payload render from `_launch_job`, renders each child's declared `template_fields` while building workflow JSON, and adds a real `DatabricksWorkflowTaskGroup` regression test.
Before preparing the final patch, could we settle the remaining 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. If launch-context rendering is the intended contract, I can provide a cherry-pickable commit from my fork or update this branch if access is granted.
---
Drafted-by: Codex (GPT-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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "moomindani (via GitHub)" <gi...@apache.org>.
moomindani commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-4863233269
Following up since the "I think we're covered" question was left open — I dug into the code paths, and unfortunately I don't think it is:
1. `_launch_job` is never reached from a `DatabricksWorkflowTaskGroup` member. In `DatabricksTaskBaseOperator.execute()`, the workflow-group branch only pulls the launch task's run metadata and monitors it (`operators/databricks.py:1917-1935`); `_launch_job` sits in the `else` (standalone) branch.
2. `render_template_fields` does run for every member task — but only in that task's own task instance, which starts *after* the job has already been created. The workflow JSON is built by the auto-added `launch` task via `_CreateDatabricksWorkflowOperator.create_workflow_json()`, which iterates the member operator objects and calls `_convert_to_databricks_workflow_task` on them (`databricks_workflow.py:187-192`) — at that point those objects are still unrendered. That is the actual #46563 bug.
3. For a standalone operator, fields are rendered before `execute()`, so the re-render added in `_launch_job` is a no-op there — consistent with what the CHANGES_REQUESTED review found, including why `test_launch_job` only passes because it calls `_launch_job` directly without the render step.
So the fix likely needs to live in the workflow-group JSON-building path (`_convert_to_databricks_workflow_task`). The design question to answer there is context: rendering member fields at that point would use the launch task's context rather than the member task's, so expressions like `{{ ti }}` or task-scoped params would resolve against the wrong task instance. Settling how #46563 should behave for those cases is probably the prerequisite for the right fix.
---
Drafted-by: Claude Code (Fable 5); reviewed by @moomindani 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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #65869:
URL: https://github.com/apache/airflow/pull/65869#issuecomment-4346775233
cc @moomindani can you also take a look?
--
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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "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
Re: [PR] issue-46563: Render `template_fields` on job launch [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 closed pull request #65869: issue-46563: Render `template_fields` on job launch
URL: https://github.com/apache/airflow/pull/65869
--
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