Skip to content

Commit 3cccb23

Browse files
committed
Warn only about bindings of Dags missing from the parse result
A Dag rejected for another team's plugin class already gets an import error on every parse, so warning about its dropped bindings as well was noise that buried the case worth a warning: a binding for a Dag the parse never produced, which points at a bug in the parse.
1 parent 36e5a7a commit 3cccb23

2 files changed

Lines changed: 32 additions & 7 deletions

File tree

‎airflow-core/src/airflow/dag_processing/collection.py‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,7 @@ def _reconcile_task_handler_bindings(
730730
bindings: Iterable[TaskHandlerBinding],
731731
*,
732732
dag_ids: Collection[str],
733+
rejected_dag_ids: Collection[str],
733734
artifact_bundle_names: Collection[str],
734735
dag_bundle_name: str,
735736
dag_relative_fileloc: str,
@@ -742,18 +743,19 @@ def _reconcile_task_handler_bindings(
742743
that moved to another file, which that file's parse reconciles. A Dag bound to an artifact outside
743744
*artifact_bundle_names*, or to one with no recorded row, keeps its rows as they are. Artifact rows are
744745
only read: the probed-artifact write records them, and the Dag processor's orphan sweep deletes them.
746+
Bindings of *rejected_dag_ids*, Dags the parse produced but that are not persisted, are dropped
747+
quietly: their import error explains why.
745748
"""
746749
by_task: dict[tuple[str, str], list[TaskHandlerBinding]] = defaultdict(list)
747-
ignored_dag_ids: set[str] = set()
750+
unknown_dag_ids: set[str] = set()
748751
for binding in bindings:
749752
if binding.dag_id in dag_ids:
750753
by_task[(binding.dag_id, binding.task_id)].append(binding)
751-
else:
752-
ignored_dag_ids.add(binding.dag_id)
753-
if ignored_dag_ids:
754+
elif binding.dag_id not in rejected_dag_ids:
755+
unknown_dag_ids.add(binding.dag_id)
756+
if unknown_dag_ids:
754757
log.warning(
755-
"Ignoring task handler bindings of Dags this parse does not persist",
756-
dag_ids=sorted(ignored_dag_ids),
758+
"Ignoring task handler bindings of Dags not in the parse result", dag_ids=sorted(unknown_dag_ids)
757759
)
758760
# The parse reports a task bound twice as an import error; this only keeps the rows as they are.
759761
if conflicting := {dag_id for (dag_id, _), group in by_task.items() if len(group) > 1}:
@@ -848,6 +850,7 @@ def update_dag_parsing_results_in_db(
848850
if task_handler_bindings is not None and task_handler_artifact_bundles is None:
849851
raise ValueError("task_handler_artifact_bundles is required with task_handler_bindings")
850852
accepted = _reject_other_teams_plugin_classes(bundle_name, dags, import_errors, session=session)
853+
rejected_ids: set[str] = set()
851854
if len(accepted) != len(dags):
852855
# A rejected Dag may have no ``dag`` row yet, and dag_warning has a foreign key to it.
853856
rejected_ids = {dag.dag_id for dag in dags} - {dag.dag_id for dag in accepted}
@@ -891,6 +894,7 @@ def update_dag_parsing_results_in_db(
891894
_reconcile_task_handler_bindings(
892895
task_handler_bindings,
893896
dag_ids={dag.dag_id for dag in dags},
897+
rejected_dag_ids=rejected_ids,
894898
artifact_bundle_names=task_handler_artifact_bundles,
895899
dag_bundle_name=bundle_name,
896900
dag_relative_fileloc=relative_fileloc,

‎airflow-core/tests/unit/dag_processing/test_collection.py‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1961,6 +1961,27 @@ def test_team_class_is_only_stored_for_its_team(
19611961
assert list(errors) == [(bundle_name, "team_dag.py")]
19621962
assert f"belonging to {plugin_team}" in errors[(bundle_name, "team_dag.py")]
19631963

1964+
@conf_vars({("core", "multi_team"): "True"})
1965+
def test_task_handler_bindings_of_a_rejected_dag_are_dropped_quietly(self, bundle, session, caplog):
1966+
bundle_name = bundle(True)
1967+
_record(session, "etl.jar")
1968+
with mock_plugin_manager(plugins=[self._plugin("other_team", "timetables", AfterWorkdayTimetable)]):
1969+
update_dag_parsing_results_in_db(
1970+
bundle_name=bundle_name,
1971+
bundle_version=None,
1972+
dags=[self._serialized(schedule=AfterWorkdayTimetable()), self._serialized("plain_dag")],
1973+
import_errors={},
1974+
parse_duration=None,
1975+
warnings=set(),
1976+
session=session,
1977+
relative_fileloc="team_dag.py",
1978+
task_handler_bindings=[_make_binding("team_dag"), _make_binding("plain_dag")],
1979+
task_handler_artifact_bundles={ARTIFACT_BUNDLE},
1980+
)
1981+
1982+
assert session.scalars(select(LangSDKTaskHandler.dag_id)).all() == ["plain_dag"]
1983+
assert not any("task handler bindings" in entry["event"] for entry in caplog.entries)
1984+
19641985
@conf_vars({("core", "multi_team"): "True"})
19651986
@pytest.mark.parametrize(
19661987
"weight_rule",
@@ -2478,7 +2499,7 @@ def test_task_handler_bindings_for_dags_outside_the_result_are_dropped(self, ses
24782499

24792500
assert _get_recorded_handlers(session) == {("etl", "extract", "etl.jar")}
24802501
assert {
2481-
"event": "Ignoring task handler bindings of Dags this parse does not persist",
2502+
"event": "Ignoring task handler bindings of Dags not in the parse result",
24822503
"dag_ids": ["ghost"],
24832504
} in caplog
24842505

0 commit comments

Comments
 (0)