You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "seanghaeli (via GitHub)" <gi...@apache.org> on 2026/04/09 22:36:14 UTC
[PR] Add Jinja template rendering and richer context for async deadline callbacks [airflow]
seanghaeli opened a new pull request, #64984:
URL: https://github.com/apache/airflow/pull/64984
Async deadline callbacks (TriggererCallbacks) now receive a full context with dag_id, run_id, logical_date, ds, ts, conf, and other standard template variables. Plain function callbacks get their kwargs rendered with Jinja2 before execution. Notifier classes are skipped since they self-render via __await__.
Replaces the minimal "simple context" from PR #55241 with a richer deadline context.
--
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] Add Jinja template rendering and richer context for async deadline callbacks [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #64984:
URL: https://github.com/apache/airflow/pull/64984#discussion_r3066491797
##########
airflow-core/src/airflow/triggers/callback.py:
##########
@@ -32,6 +33,38 @@
PAYLOAD_BODY_KEY = "body"
+def _is_notifier_class(callback: Any) -> bool:
+ """
+ Check if the callback is a BaseNotifier subclass (not an instance).
+
+ Uses duck-typing (checks for ``async_notify`` and ``template_fields``)
+ to avoid importing ``airflow.sdk`` in core.
+ """
+ return (
+ inspect.isclass(callback)
+ and hasattr(callback, "async_notify")
+ and hasattr(callback, "template_fields")
+ and hasattr(callback, "__await__")
+ )
+
+
+def _render_callback_kwargs(kwargs: dict[str, Any], context: dict) -> dict[str, Any]:
+ """
+ Render Jinja2 templates in callback kwargs using the provided context.
+
+ Uses ``Templater.render_template`` to recursively render all string values
+ in the kwargs dict. Non-string values (int, float, datetime, …) pass
+ through unchanged.
+ """
+ # Use CallbackTrigger (which inherits Templater via BaseTrigger) to access
+ # render_template without importing airflow.sdk directly in core.
+ from jinja2.sandbox import SandboxedEnvironment
+
+ trigger = CallbackTrigger(callback_path="", callback_kwargs={})
+ jinja_env = SandboxedEnvironment(cache_size=0)
+ return trigger.render_template(kwargs, cast("Any", context), jinja_env)
Review Comment:
_render_callback_kwargs() builds a raw `jinja2.sandbox.SandboxedEnvironment`, which bypasses Airflow’s templating environment (custom sandbox behavior, filters like `ds/ts`, extensions, etc.). This can make template rendering for plain function callbacks behave differently than Notifier rendering (which uses `Templater.get_template_env()`). Consider using `CallbackTrigger.get_template_env()` (or importing Airflow’s SDK `SandboxedEnvironment` from `airflow.sdk.definitions._internal.templater`) instead of the raw Jinja environment, and drop the string-based `cast("Any", ...)` in favor of a proper type (or no cast).
##########
airflow-core/src/airflow/models/deadline.py:
##########
@@ -216,28 +216,50 @@ def prune_deadlines(cls, *, session: Session, conditions: dict[Mapped, Any]) ->
def handle_miss(self, session: Session):
"""Handle a missed deadline by queueing the callback."""
- def get_simple_context():
+ def _build_deadline_context():
from airflow.api_fastapi.core_api.datamodels.dag_run import DAGRunResponse
from airflow.models import DagRun
- # TODO: Use the TaskAPI from within Triggerer to fetch full context instead of sending this context
- # from the scheduler
-
- # Fetch the DagRun from the database again to avoid errors when self.dagrun's relationship fields
- # are not in the current session.
+ # Fetch the DagRun from the database again to avoid errors when self.dagrun's
+ # relationship fields are not in the current session.
dagrun = session.get(DagRun, self.dagrun_id)
+ logical_date = dagrun.logical_date
- return {
+ context: dict[str, Any] = {
+ # Full DAGRunResponse as a JSON-serializable dict
"dag_run": DAGRunResponse.model_validate(dagrun).model_dump(mode="json"),
- "deadline": {"id": self.id, "deadline_time": self.deadline_time},
+ # Top-level convenience keys for Jinja templates (match standard context naming)
+ "dag_id": dagrun.dag_id,
+ "run_id": dagrun.run_id,
+ "logical_date": logical_date,
+ "data_interval_start": dagrun.data_interval_start,
+ "data_interval_end": dagrun.data_interval_end,
+ "run_type": dagrun.run_type,
+ "conf": dagrun.conf or {},
+ # Deadline-specific information
+ "deadline": {
+ "id": self.id,
+ "deadline_time": self.deadline_time,
+ "alert_name": self.deadline_alert.name if self.deadline_alert else None,
+ },
Review Comment:
`alert_name` is derived via `self.deadline_alert.name`, but `Deadline.deadline_alert` is not eager-loaded in the scheduler’s deadline query (it currently selectinloads only `callback` and `dagrun`). Since `handle_miss()` is called in a loop, this will trigger a per-deadline lazy-load query (N+1). Either eager-load `Deadline.deadline_alert` in the scheduler query, or avoid relationship access here by fetching the name in bulk/alongside the deadline rows.
##########
airflow-core/tests/unit/models/test_deadline.py:
##########
@@ -234,9 +234,29 @@ def test_handle_miss(self, dagrun, session):
context = callback_kwargs.pop("context")
assert callback_kwargs == TEST_CALLBACK_KWARGS
+ # Verify enriched context — dag_run and deadline info
+ assert context["dag_run"] == DAGRunResponse.model_validate(dagrun).model_dump(mode="json")
assert context["deadline"]["id"] == deadline_orm.id
assert context["deadline"]["deadline_time"].timestamp() == deadline_orm.deadline_time.timestamp()
- assert context["dag_run"] == DAGRunResponse.model_validate(dagrun).model_dump(mode="json")
+ assert context["deadline"]["alert_name"] is None # no deadline_alert in this test
Review Comment:
This test validates the enriched context shape, but it mocks `deadline_orm.callback.queue()`, so it doesn’t exercise the new context through `TriggererCallback.queue()` → `Trigger.from_object()` → `airflow.sdk.serde.serialize()`. Since the enriched context now includes additional types (UUIDs/datetimes/nested dicts), a regression test should ensure the callback can be queued successfully and the trigger kwargs can be serialized/deserialized without error.
--
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] Add Jinja template rendering and richer context for async deadline callbacks [airflow]
Posted by "ramitkataria (via GitHub)" <gi...@apache.org>.
ramitkataria commented on PR #64984:
URL: https://github.com/apache/airflow/pull/64984#issuecomment-4226944083
For context, the reason I previously added the context workaround to add "simple context" was because the triggerrer was not ready for fetching context. Now that #55068 is merged, we can handle context using the same approach that the triggerrer does for AsyncCallbacks and the way it works in the executor for SyncCallbacks. So I think the way I would want to do implement this would involve first reverting #55241 and making changes in triggerer and executor to support context in non-task workloads like callbacks. With the current approach, we are not using the `Context` used by the rest of the codebase so it will not have support for certain fields and everything is being stored in the DB which will cause increased storage usage and slower speed compared to how it is currently handled for task.
--
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] Add Jinja template rendering and richer context for async deadline callbacks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64984:
URL: https://github.com/apache/airflow/pull/64984#issuecomment-4300310241
Quick follow-up to the triage comment above — one clarification on the "Unresolved review comments" item:
Once you believe a thread has been addressed — whether by pushing a fix, or by replying in-thread with an explanation of why the suggestion doesn't apply — **please mark the thread as resolved yourself** by clicking the "Resolve conversation" button at the bottom of each thread. Reviewers don't auto-close their own threads, so an addressed-but-unresolved thread reads as "still waiting on the author" and keeps the PR from moving forward. The author doing the resolve-click is the expected convention on this project.
--
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] Add Jinja template rendering and richer context for async deadline callbacks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed pull request #64984: Add Jinja template rendering and richer context for async deadline callbacks
URL: https://github.com/apache/airflow/pull/64984
--
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] Add Jinja template rendering and richer context for async deadline callbacks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64984:
URL: https://github.com/apache/airflow/pull/64984#issuecomment-4383008098
@seanghaeli This draft PR has been inactive for 13 days since the last triage comment and no response from the author. Closing to keep the queue clean.
You are welcome to reopen this PR when you resume work, or to open a new one addressing the issues previously raised. There is no rush — take your time.
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
--
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] Add Jinja template rendering and richer context for async deadline callbacks [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #64984:
URL: https://github.com/apache/airflow/pull/64984#issuecomment-4225987283
@ramitkataria - I know you had some plans here a while back, does this work with what you have in mind?
--
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] Add Jinja template rendering and richer context for async deadline callbacks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64984:
URL: https://github.com/apache/airflow/pull/64984#issuecomment-4298824559
@seanghaeli This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Merge conflicts**: this branch has conflicts with `main` and cannot be merged as-is. Rebase onto the current `main` to resolve: `git fetch upstream main && git rebase upstream/main`, resolve, force-push.
- :x: **Unresolved review comments** (3 threads): please walk through each unresolved review thread. **Even if a suggestion looks incorrect or irrelevant** — and some of them will be, especially any comments left by automated reviewers like GitHub Copilot — it is still the author's responsibility to respond: apply the fix, reply in-thread with a brief explanation of why the suggestion does not apply, or resolve the thread if the feedback is no longer relevant. Leaving threads unaddressed for weeks blocks the PR from moving forward.
**What to do next:**
- Walk through each unresolved review thread and respond as described above.
- Rebase onto the current `main` to clear the merge conflicts.
- Make sure static checks pass locally: `prek run --from-ref main --stage pre-commit`.
- Mark the PR as "Ready for review" when you're done.
Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
--
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