uranusjr commented on code in PR #42913:
URL: https://github.com/apache/airflow/pull/42913#discussion_r1827469466
##########
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:
I see. Maybe an alternative approach would be to not return None when the
function is passed a non-existing DAG, but raise an exception instead? I am not
sure if that’s a better design.
Without changing the function though, it’s probably easier to understand
code here if it’s written like so
```python
latest_dag_version = DagVersion.get_latest_version(dag_run.dag_id,
session=session)
if latest_dag_version is None:
raise RuntimeError(f"DAG {dag_run.dag_id} does not exist; this should
not happen")
...
```
We can also use `if TYPE_CHECKING` and `assert` instead to minimise runtime
impact.
--
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]
