ephraimbuddy commented on code in PR #42913:
URL: https://github.com/apache/airflow/pull/42913#discussion_r1827459601
##########
airflow/jobs/scheduler_job_runner.py:
##########
@@ -1760,18 +1760,19 @@ def _verify_integrity_if_dag_changed(self, dag_run:
DagRun, session: Session) ->
Return True if we determine that DAG still exists.
"""
- latest_version =
SerializedDagModel.get_latest_version_hash(dag_run.dag_id, session=session)
- if dag_run.dag_hash == latest_version:
+ latest_dag_version = DagVersion.get_latest_version(dag_run.dag_id,
session=session)
+ latest_dag_version_id = latest_dag_version.id if latest_dag_version
else None
+ if dag_run.dag_version_id == latest_dag_version_id:
Review Comment:
`dag_run.dag_version_id` cannot be none. This check for whether
`latest_dag_version.id` is None is not really necessary, but it was done
because of type checking.
>Maybe all these questions can be summed into a more fundamental thought,
can all DAGs just always have a version (and latest_dag_version is never None
here) instead of having a special “no version” state?
Yes, all dags must have a version. The typing for `get_latest_version` was
made to return dag_version or none for public usage, where someone can give it
a non-existing DAG_ID, but I can convert it to a private method if we prefer
that. WDYT?
--
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]
