SameerMesiah97 commented on code in PR #61019:
URL: https://github.com/apache/airflow/pull/61019#discussion_r2725930057
##########
airflow-core/src/airflow/dag_processing/manager.py:
##########
@@ -318,7 +318,10 @@ def deactivate_stale_dags(
DagModel.fileloc,
DagModel.last_parsed_time,
DagModel.relative_fileloc,
- ).where(~DagModel.is_stale, DagModel.bundle_name.in_(bundle_names))
+ ).where(
+ ~DagModel.is_stale,
+ (DagModel.bundle_name.in_(bundle_names)) |
(DagModel.bundle_name.is_(None)),
Review Comment:
You narrowed down the root cause (missing bundle name for 2.x) but I am
uncomfortable with widening the query like this. This would be treating
bundle-less DAGs as if they belong to any/all bundles, which works fine in most
scenarios. But what about situations where bundle resolution fails? Would it
not be masking the bug? I understand that it would still be a silent error here
but non-deactivation of stale bundle-less DAGs would at least give us a
breadcrumb to follow.
I think it might be better to apply the same migration to 2.x upgrades (i.e.
set NULL to ‘dags-folder’ for `bundle_name`) as 3.0.x to 3.1.
--
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]
