Skip to content

Conversation

@kaxil
Copy link
Member

@kaxil kaxil commented Jan 5, 2026

Introduces a pluggable importer architecture for DAG files. This enables future support for non-Python DAG formats (YAML, etc.) while maintaining full backwards compatibility with existing behavior.

This should make it easier for AIP-85 Extendable DAG parsing controls. So not documenting it purposely until then.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:DAG-processing area:dev-tools backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch labels Jan 5, 2026
@kaxil
Copy link
Member Author

kaxil commented Jan 5, 2026

cc @IKholopov

@kaxil kaxil added full tests needed We need to run full set of tests for this PR to merge and removed area:dev-tools backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch labels Jan 5, 2026
@kaxil kaxil requested a review from Copilot January 5, 2026 15:52
@kaxil kaxil force-pushed the dag-importer-abstraction branch from 0fa63e5 to 925bf9e Compare January 5, 2026 15:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a pluggable DAG importer architecture that enables support for multiple DAG file formats (e.g., YAML) while maintaining backwards compatibility with existing Python DAG files. The implementation follows the AIP-85 Extendable DAG parsing controls proposal.

Key Changes:

  • Introduces an abstract AbstractDagImporter base class and DagImporterRegistry for managing importers
  • Refactors Python DAG import logic into a dedicated PythonDagImporter class
  • Modifies DagBag.process_file() to use the new importer registry instead of directly importing modules

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
airflow-core/src/airflow/dag_processing/importers/__init__.py Exports public API for the new importers module
airflow-core/src/airflow/dag_processing/importers/base.py Defines abstract importer interface, registry singleton, and result/error data structures
airflow-core/src/airflow/dag_processing/importers/python_importer.py Implements Python DAG importer by extracting and refactoring logic from DagBag
airflow-core/src/airflow/dag_processing/dagbag.py Refactored to use the new importer registry pattern instead of direct module loading
airflow-core/tests/unit/dag_processing/importers/__init__.py Test package initialization file
airflow-core/tests/unit/dag_processing/importers/test_registry.py Comprehensive tests for the importer registry functionality
airflow-core/tests/unit/dag_processing/test_dagbag.py Updated mock paths to reflect refactored code structure
airflow-core/.pre-commit-config.yaml Updated pre-commit exclusion patterns for new importer files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kaxil kaxil force-pushed the dag-importer-abstraction branch 4 times, most recently from 55d10bd to 6dee754 Compare January 5, 2026 16:46
Copy link
Contributor

@ikholopov-omni ikholopov-omni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the point of AIP-85 - LGTM as long as we will have the assumption "at most 1 importer per file extension" well documented and handled.

One deviation from AIP-85 is that filepaths are still handled by DagBag (and list_py_file_paths only lists py/zip paths). Do we have a definitive plan on how we want to handle it? Should it be handled by importer (somewhat makes sense, as it knows what extensions to look for) as was proposed in AIP-85 as well or by something else?

@kaxil kaxil force-pushed the dag-importer-abstraction branch 2 times, most recently from 082d59c to a8fc43c Compare January 5, 2026 18:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kaxil kaxil force-pushed the dag-importer-abstraction branch 2 times, most recently from 5c760ef to 7a490bf Compare January 5, 2026 18:37
Introduce a pluggable importer architecture for DAG files:

- Add AbstractDagImporter base class and DagImporterRegistry
- Add PythonDagImporter for .py and .zip files
- Refactor DagBag.process_file() to use importer registry
- Add DagImportResult/Error/Warning dataclasses for structured results

This enables future support for non-Python DAG formats (YAML, etc.)
while maintaining full backwards compatibility with existing behavior.

All existing DagBag tests pass without modification.
@kaxil kaxil force-pushed the dag-importer-abstraction branch from b86cd92 to e1d8848 Compare January 5, 2026 23:23
@kaxil kaxil marked this pull request as ready for review January 6, 2026 00:02
@kaxil kaxil requested review from gopidesupavan and ikholopov-omni and removed request for ikholopov-omni January 6, 2026 00:03
Copy link
Contributor

@ikholopov-omni ikholopov-omni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:DAG-processing full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants