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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to