Repository navigation
Explicitly convert map_index to str - #71592
Conversation
jroachgolf84
left a comment
There was a problem hiding this comment.
LGTM, I like the fix.
|
Hi @amoghrajesh Can you help with the review for this fix. Thank you. |
1fdbb9a to
37e9fef
Compare
|
Hi everyone, I’m not really convinced that #71586 is actually an issue. |
|
@dondaum I'd say it's definitely an issue, since we're at least two who've ran into it. I haven't had time to test your workaround to quote the value, but I'd expect it to work, however that's still only a workaround. Even if it was included in the docs it's not what I as a DAG-author would expect, and therefore would probably miss. Much better to have a fix that makes this behave as you'd expect (producing a string under any condition, since that's all it ever needs to be). |
37e9fef to
1f4d4c8
Compare
Wrapping the template expression within single quotes could work, but the rendering result may differ depending on |
|
Thanks, LGTM. Could you update the title and description before merge? They still describe the earlier One user-visible change is worth a line in the description: on Dags with |
Backport successfully created: v3-4-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
Previous: Dynamic task map index was using jinja's native env, so any numeric string like "123" was being coverted to int and subsequently failing.
Fix: Use jinja native render_template_as_native() function to case types correctly followed by an explicit conversion to str if it is not None (as suggested by Kaxil in the PR comment).
closes: #71586
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.