dstandish commented on code in PR #39259:
URL: https://github.com/apache/airflow/pull/39259#discussion_r1580450719
##########
airflow/models/taskinstance.py:
##########
@@ -3167,6 +3188,8 @@ def render_templates(
# MappedOperator is useless for template rendering, and we need to be
# able to access the unmapped task instead.
original_task.render_template_fields(context, jinja_env)
+ if isinstance(self.task, MappedOperator):
+ self.task = context["ti"].task
Review Comment:
that make sense @uranusjr ?
so with normal task, `self.task` is the task that is created locally, and
there is no need to override it from the one in context dict. and if you _did_
that then you'd take a task object that isn't quite complete, essentially
because we don't have proper serialization of Task since there's no real Task
entity and no TaskPydantic. But generally it's not a problem because most of
the time we don't need to serialize a task object.
in the mappedoperator case though, as we saw last night, the "unmap"
operation mutates the ti in the context dict.
if searching for alternatives, we could look at not relying on the context
dict for this "unmapping". e.g. we could forword the "original" ti object to
the thing doing the unmapping so we don't need to mutate what's in context.
another option would be, upon receiving a fresh context dict over RPC, we
could replace the TIs in the context with the local TIPydantic object -- or
something to this effect. then perhaps we could keep the context["ti"]
mutation approach for unmapping.
--
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]
