uranusjr commented on code in PR #61019:
URL: https://github.com/apache/airflow/pull/61019#discussion_r2731245045
##########
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:
There’s one potential advantage to keeping null and change the runtime
query; the `dags-folder` bundle name can be changed, and setting all null to
that string could be confusing if the user changed that bundle name prior to
migration.
However, this is admittedly a very edgy edge case, and even if it happens,
it’s only a one-time issue that can be fixed either by a new bundle parse (for
up-to-date dags) and a manual database update (for stale ones, which shouldn’t
matter either in the first place).
I don’t have a strong opinion either way, but if you’re going with option 1
(using migrations), please also change the database schema so the bundle_name
column is non-nullable to reflect it.
--
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]
