Skip to content

Fix SQLAlchemy mapper init failure when DB tests are skipped #65127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wjddn279 wants to merge 1 commit into
base: main
Choose a base branch
from
+6 −0
Open

Fix SQLAlchemy mapper init failure when DB tests are skipped #65127

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions airflow-core/src/airflow/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,12 @@ def configure_orm(disable_connection_pool=False, pool_class=None):
# Skip DB initialization in unit tests, if DB tests are skipped
Session = SkipDBTestsSession
engine = None

# Ensure all models are imported so SQLAlchemy can resolve string-based relationships in tests.
from airflow.models import import_all_models

import_all_models()
Copy link
Copy Markdown
Contributor

@holmuk holmuk Apr 13, 2026

Choose a reason for hiding this comment

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

That's a workaround, but not really a solution - you suggest to perform costly ORM initialization in non-DB mode. The root problem is that ORM-objects are created during initialization even in non-DB mode right here:

airflow/airflow-core/tests/unit/serialization/test_serialized_objects.py

Lines 227 to 250 in bb5a744

TI = TaskInstance(
task=create_scheduler_operator(EmptyOperator(task_id="test-task")),
run_id="fake_run",
state=State.RUNNING,
dag_version_id=uuid7(),
)
TI_WITH_START_DAY = TaskInstance(
task=create_scheduler_operator(EmptyOperator(task_id="test-task")),
run_id="fake_run",
state=State.RUNNING,
dag_version_id=uuid7(),
)
TI_WITH_START_DAY.start_date = timezone.datetime(2020, 1, 1, 0, 0, 0)
DAG_RUN = DagRun(
dag_id="test_dag_id",
run_id="test_dag_run_id",
run_type=DagRunType.MANUAL,
logical_date=timezone.utcnow(),
start_date=timezone.utcnow(),
state=DagRunState.SUCCESS,
)
DAG_RUN.id = 1

The better solution would be to create these objects in tests via fixtures.

Copy link
Copy Markdown
Contributor Author

@wjddn279 wjddn279 Apr 13, 2026

Choose a reason for hiding this comment

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

I actually think the opposite. If we add fixtures to individual tests, we'll need to keep adding more fixtures every time a new issue arises. Python's module import system is complex, making it unpredictable which module will be imported in what order. Whether a fixture is needed or not would become a black box that you can only figure out by running the tests. I believe this kind of non-determinism in tests is risky.

Regarding the performance concern you raised — for non-DB tests to run correctly, all airflow.models need to be imported eventually anyway (that's exactly why we're having this issue in the first place). The only difference is that we import them explicitly and all at once at startup — nothing more.

Copy link
Copy Markdown
Contributor

@holmuk holmuk Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Importing airflow.model or calling any ORM-related code is not expected in non-DB tests. Anyway, I noticed the following:

  • Global TI object only used in test_serialize_deserialize as an argument in one of test suites.
  • TI_WITH_START_DAY and DAG_RUN are unused.

I think the better compromise is:

  • Remove TI, TI_WITH_START_DAY and DAG_RUN from the code so no global objects are used by tests.
  • In test_serialize_deserialize arguments, replace ti=TI with ti=make_callback_ti(), where make_callback_ti() creates a datamodel object:
from airflow.api_fastapi.execution_api.datamodels import taskinstance as ti_datamodel

def make_callback_ti() -> ti_datamodel.TaskInstance:
    return ti_datamodel.TaskInstance(
        id=uuid7(),
        task_id="test-task",
        dag_id="test-dag",
        run_id="fake_run",
        try_number=1,
        dag_version_id=uuid7(),
        map_index=-1,
    )

This way we keep the code architecturally clean and without explicit ORM where ORM is not expected.

Copy link
Copy Markdown
Contributor Author

@wjddn279 wjddn279 Apr 13, 2026

Choose a reason for hiding this comment

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

I tried the approach you suggested, but the same error I encountered still occurs.
Would you like to try it yourself?

run into shell by executing

breeze shell --skip-db-tests --backend none

and do test what i did

airflow-core/tests/unit/serialization --verbosity=0 --strict-markers --durations=100 --maxfail=50 --color=yes --junitxml=/files/test_result-all-none.xml --timeouts-order=moi --setup-timeout=60 --execution-timeout=60 --teardown-timeout=60 --disable-warnings -rfEX --skip-db-tests --ignore-glob=*/tests/system/* --ignore-glob=airflow-core/tests/integration/* --ignore-glob=providers/apache/cassandra/tests/integration/* --ignore-glob=providers/apache/drill/tests/integration/* --ignore-glob=providers/apache/hive/tests/integration/* --ignore-glob=providers/apache/kafka/tests/integration/* --ignore-glob=providers/apache/pinot/tests/integration/* --ignore-glob=providers/apache/tinkerpop/tests/integration/* --ignore-glob=providers/celery/tests/integration/* --ignore-glob=providers/elasticsearch/tests/integration/* --ignore-glob=providers/google/tests/integration/* --ignore-glob=providers/microsoft/mssql/tests/integration/* --ignore-glob=providers/mongo/tests/integration/* --ignore-glob=providers/openlineage/tests/integration/* --ignore-glob=providers/opensearch/tests/integration/* --ignore-glob=providers/qdrant/tests/integration/* --ignore-glob=providers/redis/tests/integration/* --ignore-glob=providers/trino/tests/integration/* --ignore-glob=providers/ydb/tests/integration/* --warning-output-path=/files/warnings-all-none.txt --ignore=helm-tests --with-db-init --ignore=providers/apache/beam/tests -n 4 --no-cov --no-db-cleanup

Copy link
Copy Markdown
Contributor Author

@wjddn279 wjddn279 Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, we could probably fix it by tweaking other things as well. But should we really have to run and fix each case one by one? That approach risks breaking DB tests along the way, and it could make maintaining compatibility even harder.

Copy link
Copy Markdown
Contributor

@holmuk holmuk Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Please check #65206
This PR doesn't introduce new ORM-related calls to --skip-db-tests mode, but rearrange the code a bit to allow serialization tests to run in this mode. I also checked that all unit-tests in airflow-core work with --skip-db-tests, the problem is only with serialization tests.


return
log.debug("Setting up DB connection pool (PID %s)", os.getpid())
engine_args = prepare_engine_args(disable_connection_pool, pool_class)
Expand Down
Loading