kaxil commented on code in PR #71592:
URL: https://github.com/apache/airflow/pull/71592#discussion_r4139346998
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -2289,7 +2290,10 @@ def _render_map_index(context: Context, ti:
RuntimeTaskInstance, log: Logger) ->
return None
log.debug("Rendering map_index_template", template_length=len(template))
jinja_env = ti.task.dag.get_template_env()
- rendered_map_index = jinja_env.from_string(template).render(context)
+ rendered_map_index =
render_template_as_native(jinja_env.from_string(template), context)
Review Comment:
My earlier suggestion was wrong here, sorry. Routing every render through
`render_template_as_native` fixes the crash on native Dags but changes labels
on default Dags, which never crashed. `native_concat` runs `ast.literal_eval`
on a single string output, and the `str()` cast can't undo that. With
`render_template_as_native_obj=False`, main vs this branch: `3.10` becomes
`3.1`, `0x1f` becomes `31`, `1e3` becomes `1000.0`, `1_000` becomes `1000`,
`'quoted'` loses its quotes, and a label that renders to `None` is dropped. So
a task mapped over Python versions shows `3.10` as `3.1`.
Since the label is always a string, rendering it with a non-native env
avoids both problems:
```python
jinja_env = ti.task.dag.get_template_env(force_sandboxed=True)
rendered_map_index =
render_template_to_string(jinja_env.from_string(template), context)
```
`force_sandboxed=True` gives the same `SandboxedEnvironment` (with the Dag's
macros, filters and undefined) that default Dags already use, so every output
node is a `str` and the join can't hit the `TypeError`. Default Dags render
exactly as on main, native Dags get `"123"` and `"1"`, and the `None` check can
go. The one behavior change is that on native Dags the label template now
renders under the sandbox, which seems fine for a display label.
Could you also parametrize the new test over `render_template_as_native_obj`
in `[False, True]` and add a case like `"{{ '3.10' }}"` -> `"3.10"`? Both
current cases run with native rendering on, and `index-{{ ti.try_number }}`
already passes on main.
--
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]
