Skip to content

Improve detection of tests vs. airflow code for internal API#41030

Merged
potiuk merged 1 commit intoapache:mainfrom
potiuk:improve-internal-api-test-call-detection
Jul 25, 2024
Merged

Improve detection of tests vs. airflow code for internal API#41030
potiuk merged 1 commit intoapache:mainfrom
potiuk:improve-internal-api-test-call-detection

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jul 25, 2024

In case of database isolation, some of our tests are running a number of fixtures that allow for example to create dag runs as setup code - this code, however sometimes runs methods that are used for internal_api calls and those calls are not needed to be run via internal_api.

This PR adds capability of detecting such case - by checking if any of the "callers" of the internal_api are conftest.py which means that this is a test fixture - and in this case, direct method call is used rather than internal API call.

This way we better separate test code from "airflow" code in DB isolation tests - and internal API is only used by the tested methods and not the test code that manages setup/teardown

Also orm_create_dagrun which should only be used in scheduler is unmarked as "internal_api" method - it is heavily used in the fixtures, but neither DAGFileProcessor, Triggerer nor Worker should create new DAGRuns.


^ 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 newsfragments.

In case of database isolation, some of our tests are running
a number of fixtures that allow for example to create dag runs
as setup code - this code, however sometimes runs methods that
are used for internal_api calls and those calls are not needed
to be run via internal_api.

This PR adds capability of detecting such case - by checking if
any of the "callers" of the internal_api are `conftest.py` which
means that this is a test fixture - and in this case, direct
method call is used rather than internal API call.

This way we better separate test code from "airflow" code in
DB isolation tests - and internal API is only used by the tested
methods and not the test code that manages setup/teardown

Also orm_create_dagrun which should only be used in scheduler is
unmarked as "internal_api" method - it is heavily used in the
fixtures, but neither DAGFileProcessor, Triggerer nor Worker
should create new DAGRuns.
@potiuk potiuk requested review from XD-DENG, ashb and kaxil as code owners July 25, 2024 14:28
Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

Really nice!

@potiuk potiuk merged commit ac465e4 into apache:main Jul 25, 2024
@potiuk potiuk deleted the improve-internal-api-test-call-detection branch July 25, 2024 15:20
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jul 31, 2024
@ephraimbuddy ephraimbuddy added this to the Airflow 2.10.0 milestone Jul 31, 2024
@ephraimbuddy ephraimbuddy added the AIP-44 Airflow Internal API label Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AIP-44 Airflow Internal API changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants