You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jedcunningham (via GitHub)" <gi...@apache.org> on 2026/09/10 03:40:54 UTC
[PR] Detect Dag subclasses in the Dag version inflation check [airflow]
jedcunningham opened a new pull request, #72861:
URL: https://github.com/apache/airflow/pull/72861
The Dag version inflation check only spotted a Dag constructor when the callable resolved
through a from-import of airflow's `DAG` or `dag`. A Dag built from a subclass, or reached
through an aliased module import, produced no warnings at all — and since task detection keys
off the same predicate, neither did any task inside its `with` block.
Detection now uses the callable's name as well as the import table. The check runs before the
Dag file is imported, so there is no object to test — the name and the file's own imports are
the only evidence available.
Newly caught, both silent before:
```python
# a Dag subclass from a shared library — the task inside it is flagged too
with TeamDAG():
BashOperator(task_id=f"t_{datetime.now()}")
# a module reached through an aliased import: import airflow.sdk as sdk
sdk.DAG(start_date=datetime.now())
```
Three consequences of the heuristic, worth a reviewer's attention:
- A non-Dag named `*Dag` assigned to a variable is treated as a Dag, so real warnings on later
uses of that variable are lost:
```python
stamp = StampDag(datetime.now()) # flagged as a Dag constructor
with DAG():
BashOperator(task_id=f"a_{stamp}") # warned before this change, silent now
```
Tracking the assignment anyway would restore that warning, at the cost of the common path:
```python
dag = DAG(start_date=datetime.now()) # the real problem, already warned
BashOperator(dag=dag) # would start warning, though nothing is wrong
```
Both are a varying-valued call detected as a Dag constructor and assigned to a name, so
nothing downstream separates them — it is a choice of which one to get wrong. Assigning a Dag
and passing `dag=dag` to its tasks is everyday Airflow; naming a non-Dag `*Dag` is not. So
tracking would trade a rare miss for a routine false positive, on the very case the check
exists to catch.
- The suffix is case-sensitive, so `create_DAG()` matches where `create_dag()` does not. A name
cannot tell a Dag subclass from a factory function.
- At `error` level a false positive marks the Dag stale, so it stops scheduling rather than
merely failing to parse. The default is `warning`, so this is opt-in.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
Generated-by: Claude Code (Opus 5)
--
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] Detect Dag subclasses in the Dag version inflation check [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #72861:
URL: https://github.com/apache/airflow/pull/72861#issuecomment-5622214116
### Backport successfully created: v3-3-test
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-3-test</td>
<td><a href="https://github.com/apache/airflow/pull/72898"><img src="https://img.shields.io/badge/PR-72898-blue" alt="PR Link"></a></td>
</tr>
</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] Detect Dag subclasses in the Dag version inflation check [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham merged PR #72861:
URL: https://github.com/apache/airflow/pull/72861
--
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] Detect Dag subclasses in the Dag version inflation check [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #72861:
URL: https://github.com/apache/airflow/pull/72861#discussion_r3976485129
##########
airflow-core/src/airflow/utils/dag_version_inflation_checker.py:
##########
@@ -312,24 +312,35 @@ def __init__(self, from_imports: dict[str, tuple[str, str]]):
def is_dag_constructor(self, node: ast.Call) -> bool:
"""Check if a call is a Dag constructor."""
- # to handle use case "from airflow import sdk" and "with sdk.DAG()"
- if isinstance(node.func, ast.Attribute) and isinstance(node.func.value, ast.Name):
- if node.func.value.id in self.from_imports:
- module, original = self.from_imports[node.func.value.id]
- if (module == "airflow" or module.startswith("airflow.")) and node.func.attr in (
- "DAG",
- "dag",
- ):
+ # The Dag file is not imported yet, so there is no object to test — only names and its imports.
+ # A *DAG/*Dag suffix counts on its own, so a subclass named anything else is missed.
+ # Lowercase "dag" is an ordinary word and needs an import — and only a plain name can
+ # be an imported one, since "from airflow import DAG as D" says nothing about config.D().
+ func = node.func
+
+ # DAG(...), TeamDAG(...), an alias like D(...), or the @dag(...) decorator
+ if isinstance(func, ast.Name):
+ if func.id in self.from_imports:
+ module, original = self.from_imports[func.id]
+ if self._is_airflow_module(module) and original in ("DAG", "dag"):
return True
+ return func.id.endswith(("DAG", "Dag"))
- # to handle use case "from airflow import DAG" form or "from airflow.decorator import dag"
- if isinstance(node.func, ast.Name) and node.func.id in self.from_imports:
- module, original = self.from_imports[node.func.id]
- if (module == "airflow" or module.startswith("airflow.")) and original in ("DAG", "dag"):
- return True
+ # sdk.DAG(...), airflow.sdk.DAG(...), or the @sdk.dag(...) decorator
+ if isinstance(func, ast.Attribute):
+ if func.attr == "dag" and isinstance(func.value, ast.Name):
Review Comment:
Do we want to also support `airflow.sdk.dag(dag_id=......)` with no alias like` import airflow.sdk as sdk`?
(rare style but worth putting it)
--
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] Detect Dag subclasses in the Dag version inflation check [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #72861:
URL: https://github.com/apache/airflow/pull/72861#discussion_r3981414444
##########
airflow-core/src/airflow/utils/dag_version_inflation_checker.py:
##########
@@ -312,24 +312,35 @@ def __init__(self, from_imports: dict[str, tuple[str, str]]):
def is_dag_constructor(self, node: ast.Call) -> bool:
"""Check if a call is a Dag constructor."""
- # to handle use case "from airflow import sdk" and "with sdk.DAG()"
- if isinstance(node.func, ast.Attribute) and isinstance(node.func.value, ast.Name):
- if node.func.value.id in self.from_imports:
- module, original = self.from_imports[node.func.value.id]
- if (module == "airflow" or module.startswith("airflow.")) and node.func.attr in (
- "DAG",
- "dag",
- ):
+ # The Dag file is not imported yet, so there is no object to test — only names and its imports.
+ # A *DAG/*Dag suffix counts on its own, so a subclass named anything else is missed.
+ # Lowercase "dag" is an ordinary word and needs an import — and only a plain name can
+ # be an imported one, since "from airflow import DAG as D" says nothing about config.D().
+ func = node.func
+
+ # DAG(...), TeamDAG(...), an alias like D(...), or the @dag(...) decorator
+ if isinstance(func, ast.Name):
+ if func.id in self.from_imports:
+ module, original = self.from_imports[func.id]
+ if self._is_airflow_module(module) and original in ("DAG", "dag"):
return True
+ return func.id.endswith(("DAG", "Dag"))
- # to handle use case "from airflow import DAG" form or "from airflow.decorator import dag"
- if isinstance(node.func, ast.Name) and node.func.id in self.from_imports:
- module, original = self.from_imports[node.func.id]
- if (module == "airflow" or module.startswith("airflow.")) and original in ("DAG", "dag"):
- return True
+ # sdk.DAG(...), airflow.sdk.DAG(...), or the @sdk.dag(...) decorator
+ if isinstance(func, ast.Attribute):
+ if func.attr == "dag" and isinstance(func.value, ast.Name):
Review Comment:
We could, but I'd rather do it in a follow up since the mechanism to do it is a bit different. I'll look at that soon.
--
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