You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "purnachandergit (via GitHub)" <gi...@apache.org> on 2025/04/26 08:53:13 UTC
[PR] map index rendered on task completion #49224 [airflow]
purnachandergit opened a new pull request, #49809:
URL: https://github.com/apache/airflow/pull/49809
Fixes #49224
--
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] map index rendered on task completion #49224 [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2066006911
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -826,7 +829,17 @@ def run(
state = TerminalTIState.FAILED
return state, msg, error
- result = _execute_task(context, ti, log)
+ try:
+ result = _execute_task(context, ti, log)
+ except Exception:
+ import jinja2
+
+ # If the task failed, swallow rendering error so it doesn't mask the main error.
+ with contextlib.suppress(jinja2.TemplateSyntaxError, jinja2.UndefinedError):
+ ti.rendered_map_index = _render_map_index(context, ti=ti, log=log)
+ raise
+ else: # If the task succeeded, render normally to let rendering error bubble up.
+ ti.rendered_map_index = _render_map_index(context, ti=ti, log=log)
Review Comment:
This and `_render_map_index` need tests.
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -826,7 +829,17 @@ def run(
state = TerminalTIState.FAILED
return state, msg, error
- result = _execute_task(context, ti, log)
+ try:
+ result = _execute_task(context, ti, log)
+ except Exception:
+ import jinja2
+
+ # If the task failed, swallow rendering error so it doesn't mask the main error.
+ with contextlib.suppress(jinja2.TemplateSyntaxError, jinja2.UndefinedError):
+ ti.rendered_map_index = _render_map_index(context, ti=ti, log=log)
+ raise
+ else: # If the task succeeded, render normally to let rendering error bubble up.
+ ti.rendered_map_index = _render_map_index(context, ti=ti, log=log)
Review Comment:
There is a bug in `_render_map_index(`
--
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] map index rendered on task completion #49224 [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2066007139
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -826,7 +829,17 @@ def run(
state = TerminalTIState.FAILED
return state, msg, error
- result = _execute_task(context, ti, log)
+ try:
+ result = _execute_task(context, ti, log)
+ except Exception:
+ import jinja2
+
+ # If the task failed, swallow rendering error so it doesn't mask the main error.
+ with contextlib.suppress(jinja2.TemplateSyntaxError, jinja2.UndefinedError):
+ ti.rendered_map_index = _render_map_index(context, ti=ti, log=log)
+ raise
+ else: # If the task succeeded, render normally to let rendering error bubble up.
+ ti.rendered_map_index = _render_map_index(context, ti=ti, log=log)
Review Comment:
There is a bug in `_render_map_index(` func definition -- will add a comment there
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2072832143
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -983,21 +987,26 @@ def _handle_request(self, msg: ToSupervisor, log: FilteringBoundLogger):
if isinstance(msg, TaskState):
self._terminal_state = msg.state
self._task_end_time_monotonic = time.monotonic()
+ self._rendered_map_index = getattr(msg, "rendered_map_index", None)
Review Comment:
If there’s an unexpected case, it should be treated as a bug and emit an error loudly. I don’t think this getattr is wise.
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "purnachandergit (via GitHub)" <gi...@apache.org>.
purnachandergit commented on PR #49809:
URL: https://github.com/apache/airflow/pull/49809#issuecomment-2842670860
@kaxil , Got stuck with some other work
Working on test cases, will raise the pr in 2 days
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on PR #49809:
URL: https://github.com/apache/airflow/pull/49809#issuecomment-2841700799
how are you getting along @purnachandergit ? Would like to get this merged soon, since I am planning to cur rc1 in next few days
--
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] map index rendered on task completion #49224 [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2062571671
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1095,6 +1125,16 @@ def _execute_task(context: Context, ti: RuntimeTaskInstance, log: Logger):
return result
+def _render_map_index(context: Context, *, jinja_env: jinja2.Environment | None, log: Logger) -> str | None:
+ """Render named map index if the DAG author defined map_index_template at the task level."""
+ if jinja_env is None or (template := context.get("map_index_template")) is None:
+ return None
+ rendered_map_index = jinja_env.from_string(template).render(context)
+ log.info("Map index rendered as %s", rendered_map_index)
+ print("Map index rendered as " + rendered_map_index)
Review Comment:
This could be removed since there is a log statement above.
--
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] map index rendered on task completion #49224 [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2061591709
##########
airflow-core/src/airflow/api_fastapi/execution_api/datamodels/taskinstance.py:
##########
@@ -75,6 +75,7 @@ class TITerminalStatePayload(StrictBaseModel):
end_date: UtcDateTime
"""When the task completed executing"""
+ rendered_map_index: Annotated[str | None, Field(default_factory=str)]
Review Comment:
Shouldn't these all default to none?
```suggestion
rendered_map_index: str | None = None
```
So that we can tell the difference between not passed and empty string?
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -33,6 +34,7 @@
import aiologic
import attrs
+import jinja2
Review Comment:
Please don't import this at the top level - it can be a surprisingly expensive module to import
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1095,6 +1125,16 @@ def _execute_task(context: Context, ti: RuntimeTaskInstance, log: Logger):
return result
+def _render_map_index(context: Context, *, jinja_env: jinja2.Environment | None, log: Logger) -> str | None:
+ """Render named map index if the DAG author defined map_index_template at the task level."""
+ if jinja_env is None or (template := context.get("map_index_template")) is None:
Review Comment:
A) how would jinja env ever be None?
B) don't eagerly create (nor pass in!) the jinja env, delay it until we actually know we need it and create it in this fn
##########
task-sdk/tests/task_sdk/execution_time/test_supervisor.py:
##########
@@ -1196,7 +1196,7 @@ def watched_subprocess(self, mocker):
b"",
"task_instances.retry",
(),
- {"id": TI_ID, "end_date": timezone.parse("2024-10-31T12:00:00Z")},
+ {"id": TI_ID, "end_date": timezone.parse("2024-10-31T12:00:00Z"), "rendered_map_index": None},
Review Comment:
I'd rather we didn't even send it if the task isn't mapped, and not send it when there isn't a template specified
--
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] map index rendered on task completion #49224 [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2065995277
##########
airflow-core/src/airflow/api_fastapi/execution_api/datamodels/taskinstance.py:
##########
@@ -136,6 +138,7 @@ class TIDeferredStatePayload(StrictBaseModel):
Both forms will be passed along to the TaskSDK upon resume, the server will not handle either.
"""
+ rendered_map_index: str | None = None
Review Comment:
We should add some tests to `class TestTIUpdateState`:
https://github.com/apache/airflow/blob/main/airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_task_instances.py#L424
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "purnachandergit (via GitHub)" <gi...@apache.org>.
purnachandergit commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2066105888
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1099,6 +1130,19 @@ def _execute_task(context: Context, ti: RuntimeTaskInstance, log: Logger):
return result
+def _render_map_index(context: Context, ti: RuntimeTaskInstance, log: Logger) -> str | None:
+ """Render named map index if the DAG author defined map_index_template at the task level."""
+ if template := context.get("map_index_template") is None:
+ return None
Review Comment:
@kaxil yes, i was facing issue with that only. I have figured it out.
Thanks for pointing it out
--
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] map index rendered on task completion #49224 [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2066010142
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1099,6 +1130,19 @@ def _execute_task(context: Context, ti: RuntimeTaskInstance, log: Logger):
return result
+def _render_map_index(context: Context, ti: RuntimeTaskInstance, log: Logger) -> str | None:
+ """Render named map index if the DAG author defined map_index_template at the task level."""
+ if template := context.get("map_index_template") is None:
+ return None
Review Comment:
```suggestion
if (template := context.get("map_index_template")) is None:
return None
```
Otherwise, `template` will always be a "bool" and you will see following error:
<img width="972" alt="image" src="https://github.com/user-attachments/assets/63e0beac-a977-47d4-b95d-4c26a11c470f" />
--
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] map index rendered on task completion #49224 [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2066005214
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -983,21 +987,26 @@ def _handle_request(self, msg: ToSupervisor, log: FilteringBoundLogger):
if isinstance(msg, TaskState):
self._terminal_state = msg.state
self._task_end_time_monotonic = time.monotonic()
+ self._rendered_map_index = getattr(msg, "rendered_map_index", None)
Review Comment:
When can this attribute not be present? (Same question for all calls below)
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr merged PR #49809:
URL: https://github.com/apache/airflow/pull/49809
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #49809:
URL: https://github.com/apache/airflow/pull/49809#issuecomment-2850413298
I don’t think this (alone) fixes the issue. The value needs to be calculated in the task worker, and then passed to the server via the execution API. This PR adds the field (and passes the value when the worker calculates it), but when you set the task state manually, there’s no worker involved and thus no value calculated.
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "raphaelauv (via GitHub)" <gi...@apache.org>.
raphaelauv commented on PR #49809:
URL: https://github.com/apache/airflow/pull/49809#issuecomment-2850366254
@purnachandergit does it also fix this issue https://github.com/apache/airflow/issues/39092 ? thaks
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #49809:
URL: https://github.com/apache/airflow/pull/49809#issuecomment-2850338666
### Backport successfully created: v3-0-test
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-0-test</td>
<td><a href="https://github.com/apache/airflow/pull/50194"><img src="https://img.shields.io/badge/PR-50194-blue" alt="PR Link"></a></td>
</tr>
</table>
--
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] Render custom `map_index_template` on task completion [airflow]
Posted by "purnachandergit (via GitHub)" <gi...@apache.org>.
purnachandergit commented on code in PR #49809:
URL: https://github.com/apache/airflow/pull/49809#discussion_r2071513546
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -983,21 +987,26 @@ def _handle_request(self, msg: ToSupervisor, log: FilteringBoundLogger):
if isinstance(msg, TaskState):
self._terminal_state = msg.state
self._task_end_time_monotonic = time.monotonic()
+ self._rendered_map_index = getattr(msg, "rendered_map_index", None)
Review Comment:
@uranusjr i have added that piece of code to handle rendered_map_index in the msg object.
Ideally it should be present in the respective msg objects.
Added the code to handle any edge cases if any
--
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