ephraimbuddy commented on PR #71425:
URL: https://github.com/apache/airflow/pull/71425#issuecomment-5291236896

   Thanks for raising this. I agree that the name and documentation of 
`created_dag_version_id` no longer match its behavior, and that should be 
resolved.
   
   I don’t think that should block this PR, though. #71425 does not introduce 
the mutation for queued/running Dag runs: `main` already updates 
`created_dag_version_id` and `bundle_version` in that path following #65835, 
which was backported by #66901. This PR makes the existing behavior consistent 
by also moving a running TI that becomes `RESTARTING`, calling 
`verify_integrity`, and applying the update when `dag_run_state=False`.
   
   The #71455 repro also does not reproduce on the finished-run path as 
written. That path calls `verify_integrity`, which creates the TI for task C 
during the first clear, so the later `only_new` result is correctly empty. 
There is a reachable queued/running variant because `main` currently moves the 
run pointer without `verify_integrity`; #71425 fixes that inconsistency.
   
   I agree #71454 is valid. `DagRun.dag_versions` should report versions 
represented by the run’s TI/TIH rows rather than assume that a versioned run 
only contains its current pointer. Likewise, `_get_new_task_ids` should 
determine missing tasks from the run’s actual TI rows.
   
   On the broader design question, `run_on_latest_version` is now a released 
user-facing capability across the clear UI/API, backfills, configuration, and 
the Dag-level parameter. The API still marks it experimental, but removing it 
from versioned bundles would nevertheless be a separate user-visible design 
decision. Disabling versioning is not equivalent: users can reasonably want 
runs pinned by default while deliberately rerunning a failed task against fixed 
code.
   
   From the consumers I checked, the execution paths use 
`created_dag_version_id` as the version the run currently resolves to. I have 
not found a consumer that requires immutable creation-time provenance. Adding 
another column would also have unclear upgrade semantics because 
`_update_dagrun_to_latest_version` can rewrite all current TI version IDs 
without creating TI history, meaning the original value may already be 
unrecoverable.
   
   My preference is therefore to land #71425 to restore the existing 
invariants, fix #71454 and `_get_new_task_ids` from the TI/TIH source of truth, 
and use #71453 to settle the documentation and possible physical rename 
separately.
   
   ---
   Drafted-by: Codex (5.6 Sol); reviewed by @ephraimbuddy before posting
   


-- 
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]

Reply via email to