Skip to content

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

Open
wjddn279 wants to merge 1 commit intoapache:mainfrom
wjddn279:fix-non-db-test-sqlalchemy-mapper-error
Open

Fix SQLAlchemy mapper init failure when DB tests are skipped#65127
wjddn279 wants to merge 1 commit intoapache:mainfrom
wjddn279:fix-non-db-test-sqlalchemy-mapper-error

Conversation

@wjddn279
Copy link
Copy Markdown
Contributor

test fail: https://github.com/apache/airflow/actions/runs/24328439584/job/71030412976?pr=63871

Problem

When running non-DB tests, the following error occurs.

E   sqlalchemy.exc.InvalidRequestError: When initializing mapper Mapper[DagVersion(dag_version)], expression 'DagCode' failed to locate a name ('DagCode'). If this is a class name, consider adding this relationship() to the <class 'airflow.models.dag_version.DagVersion'> class after both dependent classes have been defined.

This also reproduces on main branch when executing the tests with those pytest arguments:

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

same in test, only testing airflow-core/tests/unit/serializationwith skip-db-tests

Cause

The root cause is that when DB initialization is skipped, Airflow models that require SQLAlchemy relations are never imported. In a typical non-DB test run, multiple modules are executed together, and the models get imported as a side effect during that process. However, when only airflow-core/tests/unit/serialization is tested in isolation — as is the case for certain changes — these models are never imported, causing the failure.

Solution

Import all airflow.module using import_all_models when testing with --skip-db-tests. It makes non-db test deterministic from sqlalchemy mapper


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@wjddn279 wjddn279 force-pushed the fix-non-db-test-sqlalchemy-mapper-error branch from 0714510 to 4376703 Compare April 13, 2026 08:17
# 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

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:

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

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

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

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

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

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.

@eladkal eladkal requested a review from Dev-iL April 14, 2026 10:24
@Dev-iL Dev-iL added the full tests needed We need to run full set of tests for this PR to merge label Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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