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, "unmapping" is
achieved by mutating the ti in the context dict, and it relies on the
assumption that the TI in the context dict is the same object as the one that
is created locally and being run, which isn't true when the context comes from
RPC.
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
