Skip to content

Add regression tests for pools API with unlimited (-1) slots#68773

Merged
shahar1 merged 3 commits into
apache:mainfrom
Vamsi-klu:fix/pools-inf-slots-65377
Jun 20, 2026
Merged

Add regression tests for pools API with unlimited (-1) slots#68773
shahar1 merged 3 commits into
apache:mainfrom
Vamsi-klu:fix/pools-inf-slots-65377

Conversation

@Vamsi-klu

Copy link
Copy Markdown
Contributor

Follow-up to #68148, which was closed as already covered by #65466. I was not able to reopen #68148 via the GitHub CLI/API after updating the branch, so I am opening this from the same rebased branch with the gap called out explicitly.

The referenced coverage does not apply to this PR's target:

  • Fix REST API pools: serialize unlimited open_slots as -1 instead of inf #65466 targets v2-11-test / Airflow 2.11 maintenance code.
  • It exercises the legacy Connexion schema path: tests/api_connexion/schemas/test_pool_schemas.py::test_serialize.
  • This PR targets main / Airflow 3.x FastAPI core API routes.
  • api_connexion is not present on main; the relevant production path is _sanitize_open_slots in airflow.api_fastapi.core_api.datamodels.pools, used by PoolResponse and PoolCollectionResponse.

On main, the existing unlimited-slots coverage only exercises the POST/create path (TestPostPool.test_post_pool_allows_unlimited_slots). It does not seed a pre-existing slots=-1 DB row and read it back through the GET routes, which is the read-path regression from #65377.

This test-only PR adds:

  • GET /pools/{name} regression coverage for an unlimited pool returning open_slots == -1 instead of 500.
  • GET /pools regression coverage for PoolCollectionResponse containing an unlimited pool.
  • Datamodel-level coverage for _sanitize_open_slots(float("inf")) == -1 and PoolResponse serialization of a busy unlimited pool with nonzero occupied/running/queued slots.

The list test remains self-seeding and does not touch the shared create_pools() helper, so the existing parametrized list counts/order assertions stay unchanged.

No newsfragment: test-only change.

Local validation:

  • ruff check airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_pools.py airflow-core/tests/unit/api_fastapi/core_api/datamodels/test_pools.py -> passed
  • PYTHONPATH="$PWD/airflow-core/src:$PWD/task-sdk/src:$PWD/airflow-core/tests" /Users/nalam/airflow-1/.venv/bin/python -m pytest airflow-core/tests/unit/api_fastapi/core_api/datamodels/test_pools.py -q -> 2 passed
  • PYTHONPATH="$PWD/airflow-core/src:$PWD/task-sdk/src:$PWD/airflow-core/tests" /Users/nalam/airflow-1/.venv/bin/python -m pytest --with-db-init airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_pools.py::TestGetPool::test_get_unlimited_pool_should_respond_200 airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_pools.py::TestGetPools::test_get_pools_with_unlimited_pool_should_respond_200 -q -> 2 passed

@boring-cyborg boring-cyborg Bot added the area:API Airflow's REST/HTTP API label Jun 19, 2026
@Vamsi-klu

Copy link
Copy Markdown
Contributor Author

Thanks @pierrejeambrun for the review on #68148. I opened this follow-up because GitHub would not let me reopen the original PR after I updated the branch.

I think the referenced coverage from #65466 does not cover this path: that test is for the legacy Connexion API on v2-11-test, while this PR targets main and the FastAPI core_api GET serializers (PoolResponse / PoolCollectionResponse). On main, the existing unlimited-slots test only covers the POST/create path; it does not seed a pre-existing slots=-1 pool and read it back through GET /pools/{name} or GET /pools, which is the regression from #65377.

I also added datamodel-level coverage for _sanitize_open_slots(float("inf")) == -1 and PoolResponse serialization of a busy unlimited pool, so this now covers both the route behavior and the validator behavior.

Local validation passed:

  • ruff check ...
  • datamodel pytest: 2 passed
  • focused GET route pytest with --with-db-init: 2 passed

shahar1
shahar1 previously approved these changes Jun 19, 2026
Comment thread airflow-core/tests/unit/api_fastapi/core_api/datamodels/test_pools.py Outdated
@shahar1 shahar1 dismissed their stale review June 19, 2026 21:06

Accidentally approved - will be approved once datamodels/test_pools.py is removed.

@shahar1 shahar1 merged commit fe3ad5f into apache:main Jun 20, 2026
76 checks passed
@pierrejeambrun

Copy link
Copy Markdown
Member

@Vamsi-klu thanks for the PR, yes I got confused by the other PR targetting 2.x, thanks

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

Labels

area:API Airflow's REST/HTTP API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants