SamWheating opened a new pull request, #70586:
URL: https://github.com/apache/airflow/pull/70586
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull
request.
Write a good git commit message following this guide:
https://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the
documentation.
For user-facing UI changes, please attach before/after screenshots (or a
short
screen recording) so reviewers can assess the visual impact.
Feel free to ping (in general) for the review if you do not see reaction for
a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we
sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
#### The Issue:
When deactivating DAGs, airflow compares the `DagFileStat` generated by the
manager to a DAG's fileloc in order to identify DAGs which were not seen on the
most recent file parse.
However, when processing zip-packaged DAGs, the DAG's filelog will be the
value of `__file__` reported by zipfile, which specifies a subpath within the
zip archive (ex: `path/to/some/dag_package.zip/my_dag.py`).
The parsing stats used to index the last_parsed dictionary use the path of
the archive file itself (ex: `path/to/some/dag_package.zip`)
This means that for zip-packaged DAGs, the `last_parsed.get()` lookup here
will never return a value:
https://github.com/apache/airflow/blob/a25d8702ca03a40f4f73e0a22d759593bf92ae75/airflow-core/src/airflow/dag_processing/manager.py#L499-L503
And thus zip-packaged DAGs are never properly deactivated.
This issue doesn't surface too often, since most DAGs will also be caught by
the `deactivate_deleted_dags` function which properly evaluates them based on
inner paths. However for files which produce multiple DAGs ("dag factories") or
DAGs which are renamed within a file, this second mechanism won't work either,
since DAGs can be removed without the source file being removed.
As a result, certain DAGs within a zip file will never be removed and
airflow will continue to attempt to schedule and run them. I have reproduced
this issue in breeze on the latest version of `main`.
#### The Fix:
Within the `deactivate_stale_dags` function, we can just remove the inner
part of the fileloc for zip-packaged DAGs to obtain the actual filepath of the
archive itself. I've added this as a private method on the class, but it could
also be added as a property on the `DAG` model itself.
---
##### Was generative AI tooling used to co-author this PR?
No
--
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]
