dkling-it opened a new pull request, #72391:
URL: https://github.com/apache/airflow/pull/72391
related: #45227
### The problem
`clear_orphaned_import_errors` needs the full set of files in a bundle, so
it runs only
after a bundle has been refreshed. While a bundle is inside its
`refresh_interval`,
`should_skip_refresh` returns `True` and the loop continues before reaching
that cleanup.
A Dag file deleted between two refreshes therefore keeps showing an import
error in the UI
for up to `[dag_processor] refresh_interval` seconds, 300 by default.
The permanent form of #45227 reported against 2.x is already fixed by #58242
and #63617.
What is left is this window, in which the UI reports an error for a file
that is gone.
### Why the tests did not catch it
Every bundle fixture pins the interval to zero:
```python
# airflow-core/tests/conftest.py
"kwargs": {"path": str(path), "refresh_interval": 0},
```
With that setting the refresh is never skipped, so
`test_remove_file_clears_import_error`
always takes the refreshed path. The new test configures a bundle with the
production
default of 300 and spies on `clear_orphaned_import_errors` to confirm the
post-refresh
cleanup really did not run.
### The change
On the skip path, check only the paths that already have a stored import
error instead of
listing the bundle. There are normally none, so this costs one query and no
filesystem
access. The `refresh_interval` throttle is untouched, so nothing is
refreshed or scanned
more often than before.
Entries inside an archive are left for the next refresh. They are detected
with the
existing `correct_maybe_zipped`, which keeps the guarantee from #63617 that
zip import
errors are not cleared while their archive is still present.
Only import errors are reconciled here, not `deactivate_deleted_dags`.
Stored import
errors are normally zero rows, whereas checking Dag rows the same way would
mean a
`stat()` per Dag on every pass. Deleted Dags are still deactivated by the
next refresh.
On query volume: `_refresh_dag_bundles` already calls `get_bundle_state`
once per bundle
per check, before the skip test, so this goes from one round trip to two
rather than
adding one where there were none.
If a path is briefly unreadable, say while a bundle checkout is being
replaced, the error
is cleared and then recreated by the next parse of the still failing file.
### Alternatives
Lifting the orphan cleanup out of the skip, as suggested on the issue, needs
a full bundle
listing on every check. `bundle_refresh_check_interval` is 5s, so that is
60x more
directory walks at the default interval, which is the cost @uranusjr raised
there.
A CLI command to clear import errors was suggested when `airflow dags` was
still the
primary surface. `dag_list_import_errors` is now decorated
`@deprecated_for_airflowctl`
and the `airflowctl` side would need a new REST endpoint, so that is no
longer a small
change.
### Tests
```
uv run --project airflow-core pytest airflow-core/tests/unit/dag_processing/
-q
```
`test_remove_file_clears_import_error_with_non_zero_refresh_interval` fails
with the
`manager.py` change reverted and passes with it.
`test_clear_import_errors_for_missing_files_keeps_zip_inner_file_errors`
covers the
archive case.
---
##### Was generative AI tooling used to co-author this PR?
- [ ] Yes (please specify the tool below)
--
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]
