[GitHub] [airflow] luos-fc commented on a diff in pull request #30608: Deactivate DAGs deleted from within zipfiles

["luos-fc (via GitHub)" <gi...@apache.org>]

luos-fc commented on code in PR #30608:
URL: https://github.com/apache/airflow/pull/30608#discussion_r1164662986


##########
airflow/models/dag.py:
##########
@@ -3375,9 +3374,8 @@ def deactivate_deleted_dags(cls, alive_dag_filelocs: list[str], session=NEW_SESS
 
         dag_models = session.query(cls).all()
         for dag_model in dag_models:
-            if dag_model.fileloc is not None:
-                if correct_maybe_zipped(dag_model.fileloc) not in alive_dag_filelocs:
-                    dag_model.is_active = False
+            if dag_model.fileloc is not None and dag_model.fileloc not in alive_dag_filelocs:
+                dag_model.is_active = False

Review Comment:
   I've added an assertion to an existing test which fails if this change is not also added.
   
   The zipped paths in `alive_dag_filelocs` previously just contained the zip files themselves, e.g. `/opt/airflow/tests/dags/test_zip.zip`. Now they contain the full path, e.g. `/opt/airflow/tests/dags/test_zip.zip/test_zip.py`.
   
   `dag_model.fileloc` has the full path within the zip, so previously `correct_maybe_zipped(dag_model.fileloc)` would match with `/opt/airflow/tests/dags/test_zip.zip` within `alive_dag_filelocs`. Now that `alive_dag_filelocs` contains the full path, we can compare with `dag_model.fileloc` directly.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org