Do not require dag_version_id in workload TI model#55717
Do not require dag_version_id in workload TI model#55717ephraimbuddy wants to merge 4 commits intoapache:mainfrom
Conversation
ephraimbuddy
requested review from
XD-DENG,
ashb,
hussein-awala,
o-nikolas and
pierrejeambrun
as code owners
boring-cyborg
bot
added
the
area:Executors-core
o-nikolas
left a comment
o-nikolas
left a comment
There was a problem hiding this comment.
Looks like API spec needs some updating, also should we have some tests added or at least updated to test this edgecase?
Older TIs won't have the dag_version_id, so we shouldn't require it in workload's TI model.
ephraimbuddy
force-pushed
the
nullify-dagversion-in-ti-workloads
branch
from
c14b250 to
fac4f4a
Compare
Thanks, updated |
| """Schema for TaskInstance with minimal required fields needed for Executors and Task SDK.""" | ||
|
|
||
| id: uuid.UUID | ||
| dag_version_id: uuid.UUID |
There was a problem hiding this comment.
How can we execute a TI without a dag version? For it to be executed, doesn't it have to still exist, and this get reparsed to include a dag version?
There was a problem hiding this comment.
We can execute a TI without a dag_version since the RuntimeTaskInstance model doesn't include the dag_version_id:
. I hope my understanding is correct? Main purpose of the dag_version_id is for display not for executionThere was a problem hiding this comment.
Isn't that dag_version_id the one that the (git, etc) dag bundle backend is told to checkout?
There was a problem hiding this comment.
Ah no, bundle_info is what the dag bundle info is.
I still wonder how it's actually possible to get to an executor with no dag_version set?
There was a problem hiding this comment.
No. dag bundle backend checks out dag bundle version:
There was a problem hiding this comment.
And it's more about the dagrun than task instance
There was a problem hiding this comment.
Still -- aren't "all" DagRuns in airflow 3 meant to have a version? What is the case by which we end up without a dag version but the task in the executor?
There was a problem hiding this comment.
I will experiment with this by reproducing this with a running TI
There was a problem hiding this comment.
Alternative to this that's less disruptive: #55884
|
Closing as the alternative has been merged |
7 participants
Older TIs won't have the dag_version_id, so we shouldn't require it in workload's TI model.
closes: #55713