Skip to content

Commit af6ff13

Browse files
committed
Avoid deactivating configured Dag bundles on transient get bundle errors
Signed-off-by: PoAn Yang <payang@apache.org>
1 parent 211d93d commit af6ff13

3 files changed

Lines changed: 107 additions & 1 deletion

File tree

‎airflow-core/src/airflow/dag_processing/bundles/manager.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,13 +314,18 @@ def _extract_and_sign_template(bundle_name: str) -> tuple[str | None, dict]:
314314
if not team:
315315
raise _bundle_item_exc(f"Team '{config.team_name}' does not exist")
316316

317+
# Pop configured bundles out of ``stored`` before ``_extract_and_sign_template``.
318+
# A transient connection error from that call must not deactivate the bundle,
319+
# which would make all Dags in it non-schedulable.
320+
bundle = stored.pop(name, None)
321+
317322
try:
318323
new_template, new_params = _extract_and_sign_template(name)
319324
except Exception as e:
320325
self.log.exception("Error creating bundle '%s': %s", name, e)
321326
continue
322327

323-
if bundle := stored.pop(name, None):
328+
if bundle:
324329
bundle.active = True
325330
if new_template != bundle.signed_url_template:
326331
bundle.signed_url_template = new_template

‎airflow-core/tests/unit/dag_processing/bundles/test_dag_bundle_manager.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,36 @@ def _get_bundle_names_and_active():
199199
]
200200

201201

202+
@pytest.mark.db_test
203+
@conf_vars({("core", "LOAD_EXAMPLES"): "False"})
204+
def test_sync_bundles_to_db_keeps_configured_bundle_on_creation_error(clear_db, session):
205+
"""A configured bundle failing to instantiate must not be deactivated as removed from config."""
206+
with patch.dict(
207+
os.environ, {"AIRFLOW__DAG_PROCESSOR__DAG_BUNDLE_CONFIG_LIST": json.dumps(BASIC_BUNDLE_CONFIG)}
208+
):
209+
manager = DagBundlesManager()
210+
manager.sync_bundles_to_db()
211+
212+
session.add(
213+
ParseImportError(
214+
bundle_name="my-test-bundle",
215+
filename="some_file.py",
216+
stacktrace="some error",
217+
)
218+
)
219+
session.flush()
220+
221+
manager = DagBundlesManager()
222+
with patch.object(
223+
DagBundlesManager, "get_bundle", autospec=True, side_effect=RuntimeError("transient error")
224+
):
225+
manager.sync_bundles_to_db()
226+
227+
bundle = session.scalar(select(DagBundleModel).where(DagBundleModel.name == "my-test-bundle"))
228+
assert bundle.active is True
229+
assert session.scalar(select(func.count(ParseImportError.id))) == 1
230+
231+
202232
@pytest.mark.db_test
203233
@conf_vars({("core", "LOAD_EXAMPLES"): "False"})
204234
def test_sync_bundles_to_db_does_not_log_removing_none_team(clear_db, caplog):

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
from airflow._shared.timezones import timezone
4646
from airflow.callbacks.callback_requests import DagCallbackRequest
4747
from airflow.dag_processing.bundles.base import BaseDagBundle
48+
from airflow.dag_processing.bundles.local import LocalDagBundle
4849
from airflow.dag_processing.bundles.manager import DagBundlesManager
4950
from airflow.dag_processing.dagbag import DagBag
5051
from airflow.dag_processing.manager import (
@@ -83,6 +84,18 @@
8384

8485
logger = logging.getLogger(__name__)
8586
TEST_DAG_FOLDER = Path(__file__).parents[1].resolve() / "dags"
87+
88+
89+
class _HealthControlledDagBundle(LocalDagBundle):
90+
"""LocalDagBundle that fails to instantiate unless its health file contains "ok"."""
91+
92+
def __init__(self, *, health_file: str, **kwargs) -> None:
93+
state = Path(health_file).read_text().strip()
94+
if state != "ok":
95+
raise RuntimeError(f"simulated {state}")
96+
super().__init__(**kwargs)
97+
98+
8699
DEFAULT_DATE = timezone.datetime(2016, 1, 1)
87100

88101

@@ -1072,6 +1085,64 @@ def test_deactivate_stale_dags_marks_dags_in_inactive_bundles(self, session):
10721085
)
10731086
assert is_stale_by_dag == {"dag_in_inactive_bundle": True, "dag_in_active_bundle": False}
10741087

1088+
def test_bundle_creation_error_keeps_bundle_active_and_dags_fresh(self, tmp_path):
1089+
"""A bundle failing to instantiate at startup must not be deactivated nor its Dags marked stale."""
1090+
dags_dir = tmp_path / "dags"
1091+
dags_dir.mkdir()
1092+
(dags_dir / "good_dag.py").write_text(
1093+
textwrap.dedent(
1094+
"""
1095+
from airflow.providers.standard.operators.empty import EmptyOperator
1096+
from airflow.sdk import DAG
1097+
1098+
with DAG(dag_id="dag_in_flaky_bundle", schedule=None):
1099+
EmptyOperator(task_id="noop")
1100+
"""
1101+
)
1102+
)
1103+
(dags_dir / "broken_dag.py").write_text("an invalid airflow DAG")
1104+
health_file = tmp_path / "health.txt"
1105+
health_file.write_text("ok")
1106+
1107+
bundle_config = [
1108+
{
1109+
"name": "flaky-bundle",
1110+
"classpath": f"{__name__}._HealthControlledDagBundle",
1111+
"kwargs": {
1112+
"path": str(dags_dir),
1113+
"health_file": str(health_file),
1114+
"refresh_interval": 0,
1115+
},
1116+
}
1117+
]
1118+
1119+
def get_db_state():
1120+
with create_session() as session:
1121+
active = session.scalar(
1122+
select(DagBundleModel.active).where(DagBundleModel.name == "flaky-bundle")
1123+
)
1124+
fresh_dags = session.scalar(
1125+
select(func.count())
1126+
.select_from(DagModel)
1127+
.where(DagModel.bundle_name == "flaky-bundle", ~DagModel.is_stale)
1128+
)
1129+
import_errors = session.scalar(select(func.count()).select_from(ParseImportError))
1130+
return active, fresh_dags, import_errors
1131+
1132+
with conf_vars({("dag_processor", "dag_bundle_config_list"): json.dumps(bundle_config)}):
1133+
DagFileProcessorManager(max_runs=1, processor_timeout=365 * 86_400).run()
1134+
assert get_db_state() == (True, 1, 1)
1135+
1136+
# "Restart" the dag-processor while bundle creation fails
1137+
health_file.write_text("transient connection error")
1138+
DagFileProcessorManager(max_runs=1, processor_timeout=365 * 86_400).run()
1139+
assert get_db_state() == (True, 1, 1)
1140+
1141+
# Removing the still-failing bundle from config must remove it from DB
1142+
with conf_vars({("dag_processor", "dag_bundle_config_list"): json.dumps([])}):
1143+
DagFileProcessorManager(max_runs=1, processor_timeout=365 * 86_400).run()
1144+
assert get_db_state() == (False, 0, 0)
1145+
10751146
@mock.patch("airflow.dag_processing.manager.is_lock_not_available_error")
10761147
@pytest.mark.usefixtures("testing_dag_bundle")
10771148
def test_deactivate_stale_dags_handles_lock_timeout(self, mock_is_lock_not_available, session, caplog):

0 commit comments

Comments
 (0)