You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "spider-man-tm (via GitHub)" <gi...@apache.org> on 2026/06/21 04:42:38 UTC
[PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
spider-man-tm opened a new pull request, #68791:
URL: https://github.com/apache/airflow/pull/68791
This is a follow-up to #68678, which proposed raising an `ImportError` when the same `dag_id` is found in two different files.
@uranusjr pointed out a valid concern with that approach: when a Dag file is simply renamed or moved, the old entry still exists in the DB until `stale_dag_threshold` passes, so the import error would fire transiently on every legitimate file move — not just on accidental duplicates.
This PR takes a different approach that avoids that problem entirely: instead of blocking the parse, it records a`DagWarning` using the existing infrastructure and surfaces it in the UI.
### What this PR does
- Adds `DagWarningType.DUPLICATE_DAG_ID` to the `DagWarningType` enum.
- Adds `_build_duplicate_dag_id_warnings()` in `dag_processing/collection.py` that queries `DagModel` for active (non-stale) rows with the same `dag_id` but a different `(bundle_name, relative_fileloc)` pair, and returns `DagWarning` objects for each conflict found.
- Calls this function inside `update_dag_parsing_results_in_db()` so warnings are persisted alongside every normal parse result.
- The warning message clearly shows both file names and the `dag_id`, helping users distinguish between an accidental duplicate and a routine file rename:
> dag_id 'hoge' is now served from 'file_b.py' (bundle: 'default'),
> previously registered from 'file_a.py' (bundle: 'default').
> If 'file_a.py' was renamed or moved, this notice will clear on the
> next parse cycle once the old file is no longer observed.
> If both files coexist with the same dag_id, rename one of them to
> avoid non-deterministic behavior.
### How it handles the file-move case (addresses uranusjr's concern)
The "last write wins" behaviour is **preserved** — no parse is blocked. When a file is renamed (e.g. `file_a.py` → `file_b.py`):
1. First parse of `file_b.py`: a `DagWarning` is recorded because `file_a.py` is still active in `DagModel`.
2. Once `file_a.py` stops being observed and is marked stale, the next parse of `file_b.py` finds no conflicting active row → no warning is generated → `_update_dag_warnings` deletes the previous entry.
So for file moves, the warning is **transient and self-clearing**. For real duplicates (both files still present with the same `dag_id`), the warning **persists** until one of the files is removed or the `dag_id` is changed.
### Emits a metric
`dag_processing.duplicate_dag_id` (tags: `dag_id`) is incremented each time a conflict is detected, allowing operators to alert on true duplicates.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Sonnet 4.6)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3510812640
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
Review Comment:
Let’s remove this, DagWarning is not imported at the top.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3509533011
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -325,6 +325,59 @@ def _sync_dag_perms(dag: LazyDeserializedDAG, session: Session):
security_manager.sync_perm_for_dag(dag_id, dag.access_control)
+def _build_duplicate_dag_id_warnings(
+ dags: Collection[LazyDeserializedDAG],
+ bundle_name: str,
+ session: Session,
+) -> set[DagWarning]:
+ """
+ Detect dag_ids that are defined in multiple files and return DagWarning objects for each.
+
+ A warning is emitted whenever the incoming file differs from the file already recorded in
+ DagModel. This covers both accidental duplicates and file renames while leaving the final
+ interpretation to the user.
+ """
+ dag_by_id = {dag.dag_id: dag for dag in dags}
+ if not dag_by_id:
+ return set()
+
+ existing_rows = session.execute(
+ select(DagModel.dag_id, DagModel.bundle_name, DagModel.relative_fileloc).where(
+ DagModel.dag_id.in_(dag_by_id),
+ DagModel.is_stale == false(),
+ )
+ )
+
+ warnings: set[DagWarning] = set()
+ for existing in existing_rows:
+ dag = dag_by_id[existing.dag_id]
+ if bundle_name == existing.bundle_name and dag.relative_fileloc == existing.relative_fileloc:
+ continue
+
+ message = (
+ f"dag_id '{dag.dag_id}' is now served from '{dag.relative_fileloc}' "
+ f"(bundle: '{bundle_name}'), previously registered from '{existing.relative_fileloc}' "
+ f"(bundle: '{existing.bundle_name}'). "
+ f"If '{existing.relative_fileloc}' was renamed or moved, this notice will clear on "
+ "the next parse cycle once the old file is no longer observed. "
+ f"If both files coexist with the same dag_id, rename one of them to avoid "
+ "non-deterministic behavior."
+ )
Review Comment:
`dag.dag_id` is already on the DagWarning model, so it does not need to appear again here. The message can also be a lot shorter since `DagWarningType.DUPLICATE_DAG_ID` already carries a lot of context to the warning. If you want to provide detailed instructions, it’s better to add a section in documentation instead.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "takayoshi-makabe (via GitHub)" <gi...@apache.org>.
takayoshi-makabe commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3510289197
##########
shared/observability/src/airflow_shared/observability/metrics/metrics_template.yaml:
##########
@@ -399,6 +399,12 @@ metrics:
legacy_name: "-"
name_variables: []
+ - name: "dag_processing.duplicate_dag_id"
+ description: "Number of Dags skipped because their dag_id is already registered from another file"
Review Comment:
I decided that there was no need to track this as a metric in the first place, so I removed it.
commit https://github.com/apache/airflow/pull/68791/commits/352ac1122942453efb291547d070a1af62136542
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3509515952
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -325,6 +325,59 @@ def _sync_dag_perms(dag: LazyDeserializedDAG, session: Session):
security_manager.sync_perm_for_dag(dag_id, dag.access_control)
+def _build_duplicate_dag_id_warnings(
+ dags: Collection[LazyDeserializedDAG],
+ bundle_name: str,
+ session: Session,
+) -> set[DagWarning]:
+ """
+ Detect dag_ids that are defined in multiple files and return DagWarning objects for each.
+
+ A warning is emitted whenever the incoming file differs from the file already recorded in
+ DagModel. This covers both accidental duplicates and file renames while leaving the final
+ interpretation to the user.
+ """
+ dag_by_id = {dag.dag_id: dag for dag in dags}
+ if not dag_by_id:
+ return set()
+
+ existing_rows = session.execute(
+ select(DagModel.dag_id, DagModel.bundle_name, DagModel.relative_fileloc).where(
+ DagModel.dag_id.in_(dag_by_id),
+ DagModel.is_stale == false(),
+ )
+ )
+
+ warnings: set[DagWarning] = set()
+ for existing in existing_rows:
+ dag = dag_by_id[existing.dag_id]
+ if bundle_name == existing.bundle_name and dag.relative_fileloc == existing.relative_fileloc:
+ continue
+
+ message = (
+ f"dag_id '{dag.dag_id}' is now served from '{dag.relative_fileloc}' "
+ f"(bundle: '{bundle_name}'), previously registered from '{existing.relative_fileloc}' "
+ f"(bundle: '{existing.bundle_name}'). "
+ f"If '{existing.relative_fileloc}' was renamed or moved, this notice will clear on "
+ "the next parse cycle once the old file is no longer observed. "
+ f"If both files coexist with the same dag_id, rename one of them to avoid "
+ "non-deterministic behavior."
+ )
+ log.warning(
+ "Duplicate dag_id '%s' detected: incoming file '%s' (bundle '%s') conflicts with "
+ "existing file '%s' (bundle '%s')",
+ dag.dag_id,
+ dag.relative_fileloc,
+ bundle_name,
+ existing.relative_fileloc,
+ existing.bundle_name,
+ )
Review Comment:
We use structlog, so instead of `%s` format strings, simply provide a short message with additional metadata with kwargs.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "takayoshi-makabe (via GitHub)" <gi...@apache.org>.
takayoshi-makabe commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3510243227
##########
shared/observability/src/airflow_shared/observability/metrics/metrics_template.yaml:
##########
@@ -399,6 +399,12 @@ metrics:
legacy_name: "-"
name_variables: []
+ - name: "dag_processing.duplicate_dag_id"
+ description: "Number of Dags skipped because their dag_id is already registered from another file"
Review Comment:
fix https://github.com/apache/airflow/pull/68791/commits/e35a56871dfd2801b0d6df169bf85da0e6fca8a4
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "spider-man-tm (via GitHub)" <gi...@apache.org>.
spider-man-tm commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3447939103
##########
airflow-ctl/src/airflowctl/api/datamodels/generated.py:
##########
Review Comment:
Auto-regenerated by `datamodel-codegen` to reflect the new `DUPLICATE_DAG_ID` value added to `DagWarningType`.
##########
airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts:
##########
Review Comment:
Auto-regenerated by `pnpm codegen` to reflect the new `duplicate dag id` value added to `DagWarningType`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "takayoshi-makabe (via GitHub)" <gi...@apache.org>.
takayoshi-makabe commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3510249569
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -325,6 +325,59 @@ def _sync_dag_perms(dag: LazyDeserializedDAG, session: Session):
security_manager.sync_perm_for_dag(dag_id, dag.access_control)
+def _build_duplicate_dag_id_warnings(
+ dags: Collection[LazyDeserializedDAG],
+ bundle_name: str,
+ session: Session,
+) -> set[DagWarning]:
+ """
+ Detect dag_ids that are defined in multiple files and return DagWarning objects for each.
+
+ A warning is emitted whenever the incoming file differs from the file already recorded in
+ DagModel. This covers both accidental duplicates and file renames while leaving the final
+ interpretation to the user.
+ """
+ dag_by_id = {dag.dag_id: dag for dag in dags}
+ if not dag_by_id:
+ return set()
+
+ existing_rows = session.execute(
+ select(DagModel.dag_id, DagModel.bundle_name, DagModel.relative_fileloc).where(
+ DagModel.dag_id.in_(dag_by_id),
+ DagModel.is_stale == false(),
+ )
+ )
+
+ warnings: set[DagWarning] = set()
+ for existing in existing_rows:
+ dag = dag_by_id[existing.dag_id]
+ if bundle_name == existing.bundle_name and dag.relative_fileloc == existing.relative_fileloc:
+ continue
+
+ message = (
+ f"dag_id '{dag.dag_id}' is now served from '{dag.relative_fileloc}' "
+ f"(bundle: '{bundle_name}'), previously registered from '{existing.relative_fileloc}' "
+ f"(bundle: '{existing.bundle_name}'). "
+ f"If '{existing.relative_fileloc}' was renamed or moved, this notice will clear on "
+ "the next parse cycle once the old file is no longer observed. "
+ f"If both files coexist with the same dag_id, rename one of them to avoid "
+ "non-deterministic behavior."
+ )
+ log.warning(
+ "Duplicate dag_id '%s' detected: incoming file '%s' (bundle '%s') conflicts with "
+ "existing file '%s' (bundle '%s')",
+ dag.dag_id,
+ dag.relative_fileloc,
+ bundle_name,
+ existing.relative_fileloc,
+ existing.bundle_name,
+ )
Review Comment:
fix
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "spider-man-tm (via GitHub)" <gi...@apache.org>.
spider-man-tm commented on PR #68791:
URL: https://github.com/apache/airflow/pull/68791#issuecomment-4795626163
Hi @potiuk,
I have fixed the issue that caused the previous CI failure. Could you please approve and trigger the tests again when you have a moment? Thank you!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "takayoshi-makabe (via GitHub)" <gi...@apache.org>.
takayoshi-makabe commented on PR #68791:
URL: https://github.com/apache/airflow/pull/68791#issuecomment-4861891708
@uranusjr
Thank you for the review. I've made some revisions based on your comments.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3509518120
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -325,6 +325,59 @@ def _sync_dag_perms(dag: LazyDeserializedDAG, session: Session):
security_manager.sync_perm_for_dag(dag_id, dag.access_control)
+def _build_duplicate_dag_id_warnings(
+ dags: Collection[LazyDeserializedDAG],
+ bundle_name: str,
+ session: Session,
+) -> set[DagWarning]:
+ """
+ Detect dag_ids that are defined in multiple files and return DagWarning objects for each.
+
+ A warning is emitted whenever the incoming file differs from the file already recorded in
+ DagModel. This covers both accidental duplicates and file renames while leaving the final
+ interpretation to the user.
+ """
+ dag_by_id = {dag.dag_id: dag for dag in dags}
+ if not dag_by_id:
+ return set()
+
+ existing_rows = session.execute(
+ select(DagModel.dag_id, DagModel.bundle_name, DagModel.relative_fileloc).where(
+ DagModel.dag_id.in_(dag_by_id),
+ DagModel.is_stale == false(),
+ )
+ )
+
+ warnings: set[DagWarning] = set()
+ for existing in existing_rows:
+ dag = dag_by_id[existing.dag_id]
+ if bundle_name == existing.bundle_name and dag.relative_fileloc == existing.relative_fileloc:
+ continue
+
+ message = (
+ f"dag_id '{dag.dag_id}' is now served from '{dag.relative_fileloc}' "
+ f"(bundle: '{bundle_name}'), previously registered from '{existing.relative_fileloc}' "
+ f"(bundle: '{existing.bundle_name}'). "
+ f"If '{existing.relative_fileloc}' was renamed or moved, this notice will clear on "
+ "the next parse cycle once the old file is no longer observed. "
+ f"If both files coexist with the same dag_id, rename one of them to avoid "
+ "non-deterministic behavior."
+ )
+ log.warning(
+ "Duplicate dag_id '%s' detected: incoming file '%s' (bundle '%s') conflicts with "
+ "existing file '%s' (bundle '%s')",
+ dag.dag_id,
+ dag.relative_fileloc,
+ bundle_name,
+ existing.relative_fileloc,
+ existing.bundle_name,
+ )
+ stats.incr("dag_processing.duplicate_dag_id", tags={"dag_id": dag.dag_id})
+ warnings.add(DagWarning(dag.dag_id, DagWarningType.DUPLICATE_DAG_ID, message))
Review Comment:
I don’t think we need this—Warnings in Airflow simply go to stderr, but we already have a log above.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "takayoshi-makabe (via GitHub)" <gi...@apache.org>.
takayoshi-makabe commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3510253022
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -325,6 +325,59 @@ def _sync_dag_perms(dag: LazyDeserializedDAG, session: Session):
security_manager.sync_perm_for_dag(dag_id, dag.access_control)
+def _build_duplicate_dag_id_warnings(
+ dags: Collection[LazyDeserializedDAG],
+ bundle_name: str,
+ session: Session,
+) -> set[DagWarning]:
+ """
+ Detect dag_ids that are defined in multiple files and return DagWarning objects for each.
+
+ A warning is emitted whenever the incoming file differs from the file already recorded in
+ DagModel. This covers both accidental duplicates and file renames while leaving the final
+ interpretation to the user.
+ """
+ dag_by_id = {dag.dag_id: dag for dag in dags}
+ if not dag_by_id:
+ return set()
+
+ existing_rows = session.execute(
+ select(DagModel.dag_id, DagModel.bundle_name, DagModel.relative_fileloc).where(
+ DagModel.dag_id.in_(dag_by_id),
+ DagModel.is_stale == false(),
+ )
+ )
+
+ warnings: set[DagWarning] = set()
+ for existing in existing_rows:
+ dag = dag_by_id[existing.dag_id]
+ if bundle_name == existing.bundle_name and dag.relative_fileloc == existing.relative_fileloc:
+ continue
+
+ message = (
+ f"dag_id '{dag.dag_id}' is now served from '{dag.relative_fileloc}' "
+ f"(bundle: '{bundle_name}'), previously registered from '{existing.relative_fileloc}' "
+ f"(bundle: '{existing.bundle_name}'). "
+ f"If '{existing.relative_fileloc}' was renamed or moved, this notice will clear on "
+ "the next parse cycle once the old file is no longer observed. "
+ f"If both files coexist with the same dag_id, rename one of them to avoid "
+ "non-deterministic behavior."
+ )
Review Comment:
fix https://github.com/apache/airflow/pull/68791/commits/6120028321a27bb9ec7e982114df2d328a83d3ff
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "spider-man-tm (via GitHub)" <gi...@apache.org>.
spider-man-tm commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3447939103
##########
airflow-ctl/src/airflowctl/api/datamodels/generated.py:
##########
Review Comment:
Auto-regenerated by `datamodel-codegen` to reflect the new `DUPLICATE_DAG_ID` value added to `DagWarningType`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "spider-man-tm (via GitHub)" <gi...@apache.org>.
spider-man-tm commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3447936044
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
Review Comment:
Auto-regenerated by `pnpm codegen` to reflect the new `duplicate dag id` value added to `DagWarningType`.
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml:
##########
Review Comment:
Auto-regenerated by `scripts/in_container/run_generate_openapi_spec.py` to reflect the new `DUPLICATE_DAG_ID` value added to `DagWarningType`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "spider-man-tm (via GitHub)" <gi...@apache.org>.
spider-man-tm commented on PR #68791:
URL: https://github.com/apache/airflow/pull/68791#issuecomment-4767255959
The CI failure was in CI image checks / Static checks — the check-metrics-synced-with-registry hook requires every metric used in code to be registered in the metrics registry YAML, but dag_processing.duplicate_dag_id was missing.
Fixed by adding the metric to
shared/observability/src/airflow_shared/observability/metrics/metrics_template.yaml.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi merged PR #68791:
URL: https://github.com/apache/airflow/pull/68791
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "spider-man-tm (via GitHub)" <gi...@apache.org>.
spider-man-tm commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3447936131
##########
airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts:
##########
Review Comment:
Auto-regenerated by `pnpm codegen` to reflect the new `duplicate dag id` value added to `DagWarningType`.
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dag_warning.py:
##########
Review Comment:
Update expected Pydantic validation error message to include the newly added 'duplicate dag id' enum value.
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml:
##########
Review Comment:
Auto-regenerated by `scripts/in_container/run_generate_openapi_spec.py` to reflect the new `DUPLICATE_DAG_ID` value added to `DagWarningType`.
##########
airflow-core/src/airflow/models/dagwarning.py:
##########
Review Comment:
Add DUPLICATE_DAG_ID to DagWarningType to allow recording duplicate dag_id warnings in the dag_warning table.
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
Review Comment:
Auto-regenerated by `pnpm codegen` to reflect the new `duplicate dag id` value added to `DagWarningType`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "spider-man-tm (via GitHub)" <gi...@apache.org>.
spider-man-tm commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3447938000
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dag_warning.py:
##########
Review Comment:
Update expected Pydantic validation error message to include the newly added 'duplicate dag id' enum value.
##########
airflow-core/src/airflow/models/dagwarning.py:
##########
Review Comment:
Add DUPLICATE_DAG_ID to DagWarningType to allow recording duplicate dag_id warnings in the dag_warning table.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3509518120
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -325,6 +325,59 @@ def _sync_dag_perms(dag: LazyDeserializedDAG, session: Session):
security_manager.sync_perm_for_dag(dag_id, dag.access_control)
+def _build_duplicate_dag_id_warnings(
+ dags: Collection[LazyDeserializedDAG],
+ bundle_name: str,
+ session: Session,
+) -> set[DagWarning]:
+ """
+ Detect dag_ids that are defined in multiple files and return DagWarning objects for each.
+
+ A warning is emitted whenever the incoming file differs from the file already recorded in
+ DagModel. This covers both accidental duplicates and file renames while leaving the final
+ interpretation to the user.
+ """
+ dag_by_id = {dag.dag_id: dag for dag in dags}
+ if not dag_by_id:
+ return set()
+
+ existing_rows = session.execute(
+ select(DagModel.dag_id, DagModel.bundle_name, DagModel.relative_fileloc).where(
+ DagModel.dag_id.in_(dag_by_id),
+ DagModel.is_stale == false(),
+ )
+ )
+
+ warnings: set[DagWarning] = set()
+ for existing in existing_rows:
+ dag = dag_by_id[existing.dag_id]
+ if bundle_name == existing.bundle_name and dag.relative_fileloc == existing.relative_fileloc:
+ continue
+
+ message = (
+ f"dag_id '{dag.dag_id}' is now served from '{dag.relative_fileloc}' "
+ f"(bundle: '{bundle_name}'), previously registered from '{existing.relative_fileloc}' "
+ f"(bundle: '{existing.bundle_name}'). "
+ f"If '{existing.relative_fileloc}' was renamed or moved, this notice will clear on "
+ "the next parse cycle once the old file is no longer observed. "
+ f"If both files coexist with the same dag_id, rename one of them to avoid "
+ "non-deterministic behavior."
+ )
+ log.warning(
+ "Duplicate dag_id '%s' detected: incoming file '%s' (bundle '%s') conflicts with "
+ "existing file '%s' (bundle '%s')",
+ dag.dag_id,
+ dag.relative_fileloc,
+ bundle_name,
+ existing.relative_fileloc,
+ existing.bundle_name,
+ )
+ stats.incr("dag_processing.duplicate_dag_id", tags={"dag_id": dag.dag_id})
+ warnings.add(DagWarning(dag.dag_id, DagWarningType.DUPLICATE_DAG_ID, message))
Review Comment:
I don’t think we need this—Warnings in Airflow simply go to stderr, but we already have a log above.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Warn in UI when the same dag_id is registered from multiple files [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #68791:
URL: https://github.com/apache/airflow/pull/68791#discussion_r3509512218
##########
shared/observability/src/airflow_shared/observability/metrics/metrics_template.yaml:
##########
@@ -399,6 +399,12 @@ metrics:
legacy_name: "-"
name_variables: []
+ - name: "dag_processing.duplicate_dag_id"
+ description: "Number of Dags skipped because their dag_id is already registered from another file"
Review Comment:
_Skipped_ is misleading; nothing is skipped, both dags (from different files) exist in the db at some point, but only one is visible after the parsing round. Maybe it’s better to say something like _shadowed_ or _overwritten_ instead.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org