Skip to content

Commit bc85f67

Browse files
ulixius9claude
authored andcommitted
fix(airflow-apis): avoid DagContext autoregister race in build_dag (#28500) (#28744)
Pipeline deploy intermittently failed with `KeyError: 'unusual_prefix_<hash>_<other dag>'` on Airflow 3.x. `common.build_dag` used `with DAG(...) as dag:`, which relies on Airflow 3.x's process-global `DagContext.current_autoregister_module_name`. On `DAG.__exit__` -> `DagContext.pop()` it dereferences `sys.modules[current_autoregister_module_name]`. When multiple DAG files parse concurrently in the same api-server process (a deploy racing the DAG-processor scan / status polls / other deploys), that global is clobbered and may point to a module already evicted from `sys.modules`, raising the KeyError on a *different* DAG than the one being deployed. Build the DAG and attach the task with an explicit `dag=` reference instead of the context manager. Airflow's BaseOperator only consults DagContext when `dag is None`, so this bypasses autoregister entirely. The DAG is already registered into module globals by `WorkflowFactory.register_dag`, so autoregister was never needed. The change is version-agnostic (Airflow 2.x and 3.x). Adds tests/unit/.../test_build_dag.py: a deterministic regression that poisons DagContext.current_autoregister_module_name and asserts build_dag still succeeds (fails with the KeyError before this fix). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 079761f)
1 parent 5dd5866 commit bc85f67

2 files changed

Lines changed: 170 additions & 22 deletions

File tree

openmetadata-airflow-apis/openmetadata_managed_apis/workflows/ingestion/common.py

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -425,26 +425,34 @@ def build_dag(
425425
:return: DAG
426426
"""
427427

428-
with DAG(**build_dag_configs(ingestion_pipeline)) as dag:
429-
# Initialize with random UUID4. Will be used by the callback instead of
430-
# generating it inside the Workflow itself.
431-
workflow_config.pipelineRunId = Uuid(uuid.uuid4())
432-
433-
CustomPythonOperator(
434-
task_id=task_name,
435-
python_callable=workflow_fn,
436-
op_kwargs={
437-
"workflow_config": workflow_config,
438-
},
439-
# There's no need to retry if we have had an error. Wait until the next schedule or manual rerun.
440-
retries=ingestion_pipeline.airflowConfig.retries or 0,
441-
# each DAG will call its own OpenMetadataWorkflowConfig
442-
on_failure_callback=partial(send_failed_status_callback, workflow_config),
443-
# Add tag and ownership to easily identify DAGs generated by OM
444-
owner=ingestion_pipeline.owners.root[0].name
445-
if (ingestion_pipeline.owners and ingestion_pipeline.owners.root)
446-
else "openmetadata",
447-
params=params,
448-
)
428+
# Build the DAG and attach the task with an explicit `dag=` reference instead of
429+
# the `with DAG(...) as dag:` context manager. The context manager relies on
430+
# Airflow 3.x's process-global DagContext autoregister, which races when multiple
431+
# DAG files are parsed concurrently in the same process and raises a KeyError on
432+
# __exit__ (see issue #28500). The DAG is registered into the module globals
433+
# explicitly by WorkflowFactory.register_dag, so autoregister is not needed here.
434+
dag = DAG(**build_dag_configs(ingestion_pipeline))
435+
436+
# Initialize with random UUID4. Will be used by the callback instead of
437+
# generating it inside the Workflow itself.
438+
workflow_config.pipelineRunId = Uuid(uuid.uuid4())
439+
440+
CustomPythonOperator(
441+
task_id=task_name,
442+
python_callable=workflow_fn,
443+
op_kwargs={
444+
"workflow_config": workflow_config,
445+
},
446+
# There's no need to retry if we have had an error. Wait until the next schedule or manual rerun.
447+
retries=ingestion_pipeline.airflowConfig.retries or 0,
448+
# each DAG will call its own OpenMetadataWorkflowConfig
449+
on_failure_callback=partial(send_failed_status_callback, workflow_config),
450+
# Add tag and ownership to easily identify DAGs generated by OM
451+
owner=ingestion_pipeline.owners.root[0].name
452+
if (ingestion_pipeline.owners and ingestion_pipeline.owners.root)
453+
else "openmetadata",
454+
params=params,
455+
dag=dag,
456+
)
449457

450-
return dag
458+
return dag
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# Copyright 2025 Collate
2+
# Licensed under the Collate Community License, Version 1.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# https://github.com/open-metadata/OpenMetadata/blob/main/ingestion/LICENSE
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
11+
"""
12+
Regression tests for ``build_dag`` (issue #28500).
13+
14+
``build_dag`` must NOT rely on Airflow 3.x's ``DagContext`` autoregister context
15+
manager. That context manager keeps a process-global
16+
``current_autoregister_module_name`` and, on ``DAG.__exit__`` -> ``DagContext.pop()``,
17+
dereferences ``sys.modules[current_autoregister_module_name]``. When multiple DAG
18+
files are parsed concurrently in the same process that global gets clobbered and the
19+
referenced module may already be evicted from ``sys.modules``, raising a ``KeyError``.
20+
21+
Building the DAG with an explicit ``dag=`` reference (instead of ``with DAG(...)``)
22+
bypasses ``DagContext`` entirely, so these tests assert both that the task attaches
23+
correctly and that a poisoned ``DagContext`` no longer breaks the build.
24+
"""
25+
26+
import uuid
27+
28+
import pytest
29+
30+
from metadata.generated.schema.entity.services.connections.metadata.openMetadataConnection import (
31+
OpenMetadataConnection,
32+
)
33+
from metadata.generated.schema.entity.services.ingestionPipelines.ingestionPipeline import (
34+
AirflowConfig,
35+
IngestionPipeline,
36+
PipelineType,
37+
)
38+
from metadata.generated.schema.metadataIngestion.databaseServiceMetadataPipeline import (
39+
DatabaseServiceMetadataPipeline,
40+
)
41+
from metadata.generated.schema.metadataIngestion.workflow import (
42+
OpenMetadataWorkflowConfig,
43+
Sink,
44+
Source,
45+
SourceConfig,
46+
WorkflowConfig,
47+
)
48+
from metadata.generated.schema.security.client.openMetadataJWTClientConfig import (
49+
OpenMetadataJWTClientConfig,
50+
)
51+
from metadata.generated.schema.type.entityReference import EntityReference
52+
from openmetadata_managed_apis.workflows.ingestion.common import build_dag
53+
54+
TASK_NAME = "ingestion_task"
55+
56+
57+
def _noop_workflow_fn(*_, **__):
58+
return None
59+
60+
61+
def _server_config() -> OpenMetadataConnection:
62+
return OpenMetadataConnection(
63+
hostPort="http://localhost:8585/api",
64+
authProvider="openmetadata",
65+
securityConfig=OpenMetadataJWTClientConfig(jwtToken="x.y.z"),
66+
)
67+
68+
69+
def _ingestion_pipeline(name: str) -> IngestionPipeline:
70+
return IngestionPipeline(
71+
name=name,
72+
pipelineType=PipelineType.metadata,
73+
fullyQualifiedName=f"svc.{name}",
74+
sourceConfig=SourceConfig(config=DatabaseServiceMetadataPipeline()),
75+
openMetadataServerConnection=_server_config(),
76+
airflowConfig=AirflowConfig(),
77+
service=EntityReference(id=str(uuid.uuid4()), type="databaseService", name="svc"),
78+
)
79+
80+
81+
def _workflow_config(name: str) -> OpenMetadataWorkflowConfig:
82+
return OpenMetadataWorkflowConfig(
83+
source=Source(
84+
type="mysql",
85+
serviceName="svc",
86+
sourceConfig=SourceConfig(config=DatabaseServiceMetadataPipeline()),
87+
),
88+
sink=Sink(type="metadata-rest", config={}),
89+
workflowConfig=WorkflowConfig(openMetadataServerConfig=_server_config()),
90+
ingestionPipelineFQN=f"svc.{name}",
91+
)
92+
93+
94+
def _build(name: str):
95+
return build_dag(
96+
task_name=TASK_NAME,
97+
ingestion_pipeline=_ingestion_pipeline(name),
98+
workflow_config=_workflow_config(name),
99+
workflow_fn=_noop_workflow_fn,
100+
)
101+
102+
103+
def test_build_dag_attaches_task():
104+
dag = _build("attach_dag")
105+
106+
assert dag.dag_id == "attach_dag"
107+
assert len(dag.tasks) == 1
108+
assert TASK_NAME in dag.task_dict
109+
assert dag.task_dict[TASK_NAME].dag is dag
110+
111+
112+
def test_build_dag_survives_poisoned_dag_context():
113+
"""
114+
Simulate the concurrent-parse race deterministically: point the process-global
115+
autoregister module name at a module that is not in ``sys.modules``. Before the
116+
fix this raised ``KeyError`` on ``with DAG(...) as dag:`` __exit__; the explicit
117+
``dag=`` build must be immune to it.
118+
"""
119+
dag_context = pytest.importorskip("airflow.sdk.definitions._internal.contextmanager").DagContext
120+
121+
if not hasattr(dag_context, "current_autoregister_module_name"):
122+
pytest.skip("DagContext autoregister not present on this Airflow version")
123+
124+
original = dag_context.current_autoregister_module_name
125+
dag_context.current_autoregister_module_name = f"missing_module_{uuid.uuid4().hex}"
126+
try:
127+
dag = _build("poisoned_dag")
128+
finally:
129+
dag_context.current_autoregister_module_name = original
130+
131+
assert dag.dag_id == "poisoned_dag"
132+
assert TASK_NAME in dag.task_dict
133+
134+
135+
def test_build_dag_does_not_use_dag_context_stack():
136+
dag_context = pytest.importorskip("airflow.sdk.definitions._internal.contextmanager").DagContext
137+
138+
_build("stack_dag")
139+
140+
assert dag_context.get_current_dag() is None

0 commit comments

Comments
 (0)